-
Notifications
You must be signed in to change notification settings - Fork 382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes to build analyzer for PS7 and PS6 and ship in separate directories and bump version to 1.19.0 #1425
Changes from 7 commits
c485f58
0ec56c1
d9e7138
c15e809
9a0cb55
23d19f8
cb178e1
c3b2a4c
255b70d
147700b
3178e74
64577dd
94c0ecd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,17 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<VersionPrefix>1.18.3</VersionPrefix> | ||
<TargetFrameworks>netstandard2.0;net452</TargetFrameworks> | ||
<VersionPrefix>1.19.0</VersionPrefix> | ||
<TargetFrameworks>netcoreapp3.1;netstandard2.0;net452</TargetFrameworks> | ||
<AssemblyName>Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules</AssemblyName> | ||
<PackageId>Rules</PackageId> | ||
<RootNamespace>Microsoft.Windows.PowerShell.ScriptAnalyzer</RootNamespace> <!-- Namespace needs to match Assembly name for ressource binding --> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\Engine\Engine.csproj" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is common amongst all compilations and is therefore duplicated. Please move it into its own ItemGroup There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure |
||
<PackageReference Include="Newtonsoft.Json" Version="12.0.2" /> | ||
<PackageReference Include="Microsoft.Management.Infrastructure" Version="1.0.0" /> | ||
<PackageReference Include="Newtonsoft.Json" Version="12.0.3" /> | ||
bergmeister marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<PackageReference Include="Microsoft.Management.Infrastructure" Version="2.0.0" /> | ||
rjmholt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</ItemGroup> | ||
|
||
<ItemGroup Condition=" '$(TargetFramework)' == 'net452' "> | ||
|
@@ -42,7 +42,7 @@ | |
</EmbeddedResource> | ||
</ItemGroup> | ||
|
||
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'"> | ||
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0' OR '$(TargetFramework)' == 'netcoreapp3.1'"> | ||
<PackageReference Include="System.Reflection.TypeExtensions" Version="4.5.1" /> | ||
<Compile Remove="UseSingularNouns.cs" /> | ||
</ItemGroup> | ||
|
@@ -67,4 +67,30 @@ | |
<DefineConstants>$(DefineConstants);PSV3;PSV4</DefineConstants> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1' AND '$(Configuration)' == 'PSV7Release'"> | ||
<DefineConstants>$(DefineConstants);PSV7;CORECLR</DefineConstants> | ||
</PropertyGroup> | ||
<ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1' AND '$(Configuration)' == 'PSV7Release'"> | ||
<PackageReference Include="Microsoft.PowerShell.SDK" Version="7.0.0" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the SDK and not SMA? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. strictly speaking, SMA should have never been used, but the SDK did not exist. Using the SDK is the guidance we're providing. |
||
</ItemGroup> | ||
<PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0' AND '$(Configuration)' == 'PSV6Release'"> | ||
<DefineConstants>$(DefineConstants);PSV6;CORECLR</DefineConstants> | ||
</PropertyGroup> | ||
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0' AND '$(Configuration)' == 'PSV6Release'"> | ||
<PackageReference Include="System.Management.Automation" Version="6.1.0" /> | ||
</ItemGroup> | ||
|
||
<PropertyGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1' AND '$(Configuration)' == 'PSV7Debug'"> | ||
<DefineConstants>$(DefineConstants);PSV7;CORECLR</DefineConstants> | ||
</PropertyGroup> | ||
<ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1' AND '$(Configuration)' == 'PSV7Debug'"> | ||
<PackageReference Include="Microsoft.PowerShell.SDK" Version="7.0.0" /> | ||
</ItemGroup> | ||
<PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0' AND '$(Configuration)' == 'PSV6Debug'"> | ||
<DefineConstants>$(DefineConstants);PSV6;CORECLR</DefineConstants> | ||
</PropertyGroup> | ||
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0' AND '$(Configuration)' == 'PSV6Debug'"> | ||
<PackageReference Include="System.Management.Automation" Version="6.1.0" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,10 +37,10 @@ param( | |
[switch] $Bootstrap | ||
) | ||
BEGIN { | ||
if ($PSVersion -gt 6) { | ||
# due to netstandard2.0 we do not need to treat PS version 7 differently | ||
$PSVersion = 6 | ||
} | ||
#if ($PSVersion -gt 6) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove commented out code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do |
||
# # due to netstandard2.0 we do not need to treat PS version 7 differently | ||
# $PSVersion = 6 | ||
#} | ||
} | ||
|
||
END { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,9 +144,7 @@ function Start-ScriptAnalyzerBuild | |
param ( | ||
[switch]$All, | ||
|
||
# Note that 6 should also be chosen for PowerShell7 as both implement netstandard2.0 | ||
# and we do not use features from netstandard2.1 | ||
[ValidateRange(3, 6)] | ||
[ValidateRange(3, 7)] | ||
[int]$PSVersion = $PSVersionTable.PSVersion.Major, | ||
|
||
[ValidateSet("Debug", "Release")] | ||
|
@@ -178,7 +176,7 @@ function Start-ScriptAnalyzerBuild | |
if ( $All ) | ||
{ | ||
# Build all the versions of the analyzer | ||
foreach($psVersion in 3..6) { | ||
foreach($psVersion in 3..7) { | ||
Start-ScriptAnalyzerBuild -Configuration $Configuration -PSVersion $psVersion | ||
} | ||
return | ||
|
@@ -191,15 +189,18 @@ function Start-ScriptAnalyzerBuild | |
Set-Variable -Name profilesCopied -Value $true -Scope 1 | ||
} | ||
|
||
if ($PSVersion -ge 6) { | ||
if ($PSVersion -eq 7) { | ||
$framework = 'netcoreapp3.1' | ||
} | ||
elseif ($PSVersion -eq 6) { | ||
$framework = 'netstandard2.0' | ||
} | ||
else { | ||
$framework = "net452" | ||
} | ||
|
||
# build the appropriate assembly | ||
if ($PSVersion -match "[34]" -and $Framework -eq "core") | ||
if ($PSVersion -match "[34]" -and $Framework -ne "net452") | ||
{ | ||
throw ("ScriptAnalyzer for PS version '{0}' is not applicable to {1} framework" -f $PSVersion,$Framework) | ||
} | ||
|
@@ -231,7 +232,11 @@ function Start-ScriptAnalyzerBuild | |
} | ||
6 | ||
{ | ||
$destinationDirBinaries = "$script:destinationDir\coreclr" | ||
$destinationDirBinaries = "$script:destinationDir\PSv6" | ||
} | ||
7 | ||
{ | ||
$destinationDirBinaries = "$script:destinationDir\PSv7" | ||
} | ||
default | ||
{ | ||
|
@@ -240,7 +245,7 @@ function Start-ScriptAnalyzerBuild | |
} | ||
|
||
$buildConfiguration = $Configuration | ||
if ((3, 4) -contains $PSVersion) { | ||
if ((3, 4, 6, 7) -contains $PSVersion) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because only 7 is special, wouldn't code be simpler if we didn't add 6 so that we would only need the PSVersion7 constant in the csproj file but not PSVersion6? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. possibly, but i thought it would be better to be more complete and treat 5 as the only special one. Since assemblies are put in special directories, i thought it was best to provide constants that could be used |
||
$buildConfiguration = "PSV${PSVersion}${Configuration}" | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
{ | ||
"sdk": { | ||
"version": "3.1.102" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reason for this change? This seems to be a downgrade There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Jim mentioned on Slack it didn't work with 102 but only 101. Def something that would be interesting to know but I am not too worried about being one patch behind since PSSA is not a self-contained app There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it was a vsts build problem - for some reason the VSTS build machine couldn't download 102 - i didn't spend the time to find out why since it was just a single patch (for the SDK) |
||
"version": "3.1.101" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
# escape=` | ||
#0.3.6 (no powershell 6) | ||
# FROM microsoft/windowsservercore | ||
FROM microsoft/dotnet-framework:4.7.1 | ||
FROM mcr.microsoft.com/dotnet/framework/sdk:4.8 | ||
LABEL maintainer='PowerShell Team <[email protected]>' | ||
LABEL description="This Dockerfile for Windows Server Core with git installed via chocolatey." | ||
|
||
|
@@ -14,19 +14,20 @@ COPY dockerInstall.psm1 containerFiles/dockerInstall.psm1 | |
RUN Import-Module PackageManagement; ` | ||
Install-PackageProvider -Name NuGet -MinimumVersion 2.8.5.201 -Force; ` | ||
Import-Module ./containerFiles/dockerInstall.psm1; ` | ||
[System.Net.ServicePointManager]::SecurityProtocol = [System.Net.ServicePointManager]::SecurityProtocol -bor [System.Net.SecurityProtocolType]::Tls12; ` | ||
Install-ChocolateyPackage -PackageName git -Executable git.exe; ` | ||
Install-ChocolateyPackage -PackageName nuget.commandline -Executable nuget.exe -Cleanup; ` | ||
Install-Module -Force -Name platyPS -Repository PSGallery; ` | ||
Invoke-WebRequest -Uri https://raw.githubusercontent.com/dotnet/cli/master/scripts/obtain/dotnet-install.ps1 -outfile C:/dotnet-install.ps1; ` | ||
C:/dotnet-install.ps1 -Channel Release -Version 2.1.4; ` | ||
Add-Path C:/Users/ContainerAdministrator/AppData/Local/Microsoft/dotnet; | ||
|
||
RUN Import-Module ./containerFiles/dockerInstall.psm1; ` | ||
#RUN Import-Module ./containerFiles/dockerInstall.psm1; ` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth removing this entirely if we're no longer using this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure |
||
# git clone https://Github.com/PowerShell/PSScriptAnalyzer; ` | ||
Install-ChocolateyPackage -PackageName dotnet4.5; | ||
# Install-ChocolateyPackage -PackageName dotnet4.5; | ||
|
||
RUN Import-Module ./containerFiles/dockerInstall.psm1; ` | ||
Install-ChocolateyPackage -PackageName netfx-4.5.2-devpack; | ||
#RUN Import-Module ./containerFiles/dockerInstall.psm1; ` | ||
# Install-ChocolateyPackage -PackageName netfx-4.5.2-devpack; | ||
|
||
COPY buildPSSA.ps1 containerFiles/buildPSSA.ps1 | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please cleanup this commented out line