Upgrade 'System.Automation.Management' NuGet package of version 6.0.0-alpha13 to official version 6.0.2 from powershell-core feed, which requires upgrade to netstandard2.0 #919

bergmeister · 2018-03-03T22:39:23Z

PR Summary

Closes #879

At the moment we use an unofficial package from Nuget.org, which is a self-contained version of System.Management.Automation (including dependencies) from 6.0.0-alpha13, which contains the Parser that PSSA uses.

For coreclr:

This required the upgrade from netstandard1.6 to netstandard2.0, which required some more NuGet packages: Microsoft.CSharp, Microsoft.Management.Infrastructure (for DSC rules, this is only available as a pre-release) and System.Reflection.TypeExtensions
I tested in on Windows (10) and a Ubuntu VM and I only had the same 9 local test failures that I currently also get for the development branch

For fullclr:

We are now using the reference assemblies instead. Because the rule UseIdenticalMandatoryParametersDSC compiles only in PSv4 I had to add a new PSV4Release build option.
In order to thoroughly test the fullclr change, I created 2 azure machines with WMF3 (WinServer2012) and WMF4 (WinServer2012R2) to run the test. The outcome was that for WMF3, the current release (or the development branch) have an awful lot (245) failures. This PR reduces it to only around 8, which is good and an improvement. For WMF4, there were no test failures for the development branch but 1 test in this PR. This test failure could be looked into but it seems to be minor because I can see from the test failure already that it is just a string that is slightly different (additional quote in an expected string) and therefore I think this might be negligible.

Old unused csproj files were also removed

The reason for the PowerShell Core Appveyor failure is because the Windows image is still using pwsh 6.0.0 but the system.management,automation reference is 6.0.2. I opened an issue at appveyor/ci#2230 and this will get fixed in the next 3 weeks, therefore a step was added to install pwsh 6.0.2 on the Windows image if necessary

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets. Please mark anything not applicable to this PR NA.

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
[NA] User facing documentation needed
Change is not breaking: The test results on Ubuntu indicate breaking changes
[NA] Make sure you've added a new test if existing tests do not effectively test the code changed
This PR is ready to merge and is not work in progress
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

…h required upgrade from netstandard 1.6 to netstandard2.0,which included some breaking changes in netstandard2.0 that required some more nuget packges.

bergmeister · 2018-03-03T22:42:09Z

Engine/ScriptAnalyzer.cs

@@ -1141,7 +1141,7 @@ private List<ExternalRule> GetExternalRule(string[] moduleNames)
                            {
                                dynamic description = helpContent[0].Properties["Description"];

-                                if (null != description && null != description.Value && description.Value.GetType().IsArray)
+                                if (description != null && description.Value != null && description.Value.GetType().IsArray)


This is just to highlight why I needed the NuGet package for Microsoft.CSharp, which was included in netstandard1.6 but not netstandard2.0

yup - I found this as well when I was looking at building against PowerShellStandard.Library

bergmeister · 2018-03-03T22:46:12Z

Rules/Rules.csproj

+
+  <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
+    <PackageReference Include="System.Management.Automation" Version="6.0.1.1" />
+    <PackageReference Include="Microsoft.Management.Infrastructure" Version="1.0.0-alpha08" />


This is a good example to show that the old System.Management.Automation package was self-contained and now we also need its other dependency Microsoft.Management.Infrastructure.

iSazonov · 2018-03-05T17:05:51Z

I wonder - Could PSSA use current PowerShell installation? - there is SMA.dll with all dependencies.

bergmeister · 2018-03-05T17:33:24Z

@iSazonov Can you please clarify what you mean by PowerShell installation? The issue is that currently there is a missing feature in the PowerShell Parser and I expect the change to come in about 1 month and therefore want to consume the parser via the system.management.automation nuget package in order for the update to work on any old or even current version of PowerShell such as e.g. Windows PowerShell that will not receive the parser update in the near future.

iSazonov · 2018-03-06T05:32:16Z

My quesion is could PSSA load needed dlls from c:\program file\powershell or C:\Windows\System32\WindowsPowerShell\v1.0?

bergmeister · 2018-03-06T08:09:45Z

It probably could but we don't wan't to. The whole point of this PR is being able to consume the latest version of system.management.automation with the latest required features of the parser.

…nalyzer into Parser # Conflicts: # Rules/Rules.csproj # appveyor.yml

…nalyzer into Parser

bergmeister · 2018-04-03T21:46:54Z

The reason for the PowerShell Core Appveyor failure is because the Windows image is still using pwsh 6.0.0 but the system.management,automation reference is 6.0.1. I opened an issue at appveyor/ci#2230 and this will get fixed in the next 3 weeks, therefore it is probably best to disable the PowerShell Core build on Windows for the moment (the Ubuntu build should give us enough coverage in the meantime). Just using the 6.0.0 package instead is not a good either because the 6.0.0 package only works for netcore2.0 but only the 6.0.2 packages were fixed to work with netstandard2.0

…d by the new system.management.automation package.

JamesWTruher

just one real question down below. Otherwise, this looks fine.

JamesWTruher · 2018-04-03T23:48:18Z

Engine/ScriptAnalyzer.cs

@@ -1141,7 +1141,7 @@ private List<ExternalRule> GetExternalRule(string[] moduleNames)
                            {
                                dynamic description = helpContent[0].Properties["Description"];

-                                if (null != description && null != description.Value && description.Value.GetType().IsArray)
+                                if (description != null && description.Value != null && description.Value.GetType().IsArray)


yup - I found this as well when I was looking at building against PowerShellStandard.Library

JamesWTruher · 2018-04-03T23:52:27Z

Rules/Rules.csproj

+
+  <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
+    <PackageReference Include="System.Management.Automation" Version="6.0.2" />
+    <PackageReference Include="Microsoft.Management.Infrastructure" Version="1.0.0-alpha08" />


could this just be Version="1.0.0-alpha* (that's what it is in PowerShell repo)

Yes, I think it is reasonable to always take the latest version of a pre-release package.

JamesWTruher · 2018-04-03T23:59:59Z

Engine/Engine.csproj

+  </ItemGroup>
+
+  <ItemGroup Condition="'$(TargetFramework)' == 'net451'">
+    <PackageReference Include="System.Management.Automation" Version="6.0.0-alpha13" />


since this is really about full CLR should we still point at this? Would Microsoft.PowerShell.5.ReferenceAssemblies be better? (or Microsoft.PowerShell.3.ReferenceAssemblies) rather than pointing off to that alpha13 thing?

At the moment this PR changes it only for the coreclr but I can look into how to do it for full CLR. I first wanted to rather start with the simpler task but maybe it is better to do both in one go.

…s for Ilya for the help to get that working

This reverts commit d31a55b.

This reverts commit 52d8521.

…nalyzer into Parser

…roducing PSV4Release Remove old csproj files that are not needed any more

… (i.e. latest instead of specific)

JamesWTruher · 2018-04-05T18:20:55Z

I think we're ready to go - go ahead and merge

bergmeister added 2 commits March 3, 2018 21:11

Upgrade system.automation.management to 6.0.1.1 for netstandard, whic…

a17699b

…h required upgrade from netstandard 1.6 to netstandard2.0,which included some breaking changes in netstandard2.0 that required some more nuget packges.

use release version of TypeExtensions instead of pre-release

a48573c

bergmeister added the Area - Build label Mar 3, 2018

bergmeister self-assigned this Mar 3, 2018

bergmeister added the in progress label Mar 3, 2018

bergmeister requested a review from JamesWTruher March 3, 2018 22:39

bergmeister commented Mar 3, 2018

View reviewed changes

bergmeister added 3 commits March 23, 2018 19:40

Merge branch 'development' of https://github.com/PowerShell/PSScriptA…

83fcb7b

…nalyzer into Parser # Conflicts: # Rules/Rules.csproj # appveyor.yml

Update System.Management.Automation from 6.0.1.1 to 6.0.2

7403578

Merge branch 'development' of https://github.com/PowerShell/PSScriptA…

81bdc15

…nalyzer into Parser

run PowerShell Core tests only if image has at least 6.0.2 as require…

511216c

…d by the new system.management.automation package.

JamesWTruher reviewed Apr 4, 2018

View reviewed changes

bergmeister added 9 commits April 4, 2018 07:52

install pwsh for windows if required instead of skipping tests. Thank…

4767772

…s for Ilya for the help to get that working

upgrade to net 461 and use in full clr as well

52d8521

fix other references to net451 with net461

d31a55b

Revert "fix other references to net451 with net461"

041c2a2

This reverts commit d31a55b.

Revert "upgrade to net 461 and use in full clr as well"

5e73dd9

This reverts commit 52d8521.

use v3/5 reference assemblies

e59178c

Merge branch 'development' of https://github.com/PowerShell/PSScriptA…

09b8822

…nalyzer into Parser

Make UseIdenticalMandatoryParametersDSC rule compilable for v4 by int…

5bb6fed

…roducing PSV4Release Remove old csproj files that are not needed any more

fix wmf4 build

c13f87b

Use 1.0.0-alpha* of Microsoft.Management.Infrastructure nuget package…

0c59367

… (i.e. latest instead of specific)

bergmeister added Area - Engine and removed in progress labels Apr 4, 2018

bergmeister merged commit 7304362 into PowerShell:development Apr 5, 2018

bergmeister mentioned this pull request May 11, 2018

Fix New-Release cmdlet by also building also for v4 and misc. improvements #996

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade 'System.Automation.Management' NuGet package of version 6.0.0-alpha13 to official version 6.0.2 from powershell-core feed, which requires upgrade to netstandard2.0 #919

Upgrade 'System.Automation.Management' NuGet package of version 6.0.0-alpha13 to official version 6.0.2 from powershell-core feed, which requires upgrade to netstandard2.0 #919

bergmeister commented Mar 3, 2018 •

edited

Loading

bergmeister Mar 3, 2018

JamesWTruher Apr 3, 2018

bergmeister Mar 3, 2018

iSazonov commented Mar 5, 2018

bergmeister commented Mar 5, 2018

iSazonov commented Mar 6, 2018

bergmeister commented Mar 6, 2018

bergmeister commented Apr 3, 2018 •

edited

Loading

JamesWTruher left a comment

JamesWTruher Apr 3, 2018

JamesWTruher Apr 3, 2018

bergmeister Apr 4, 2018

JamesWTruher Apr 3, 2018

bergmeister Apr 4, 2018 •

edited

Loading

JamesWTruher commented Apr 5, 2018

Upgrade 'System.Automation.Management' NuGet package of version 6.0.0-alpha13 to official version 6.0.2 from powershell-core feed, which requires upgrade to netstandard2.0 #919

Upgrade 'System.Automation.Management' NuGet package of version 6.0.0-alpha13 to official version 6.0.2 from powershell-core feed, which requires upgrade to netstandard2.0 #919

Conversation

bergmeister commented Mar 3, 2018 • edited Loading

PR Summary

PR Checklist

bergmeister Mar 3, 2018

Choose a reason for hiding this comment

JamesWTruher Apr 3, 2018

Choose a reason for hiding this comment

bergmeister Mar 3, 2018

Choose a reason for hiding this comment

iSazonov commented Mar 5, 2018

bergmeister commented Mar 5, 2018

iSazonov commented Mar 6, 2018

bergmeister commented Mar 6, 2018

bergmeister commented Apr 3, 2018 • edited Loading

JamesWTruher left a comment

Choose a reason for hiding this comment

JamesWTruher Apr 3, 2018

Choose a reason for hiding this comment

JamesWTruher Apr 3, 2018

Choose a reason for hiding this comment

bergmeister Apr 4, 2018

Choose a reason for hiding this comment

JamesWTruher Apr 3, 2018

Choose a reason for hiding this comment

bergmeister Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

JamesWTruher commented Apr 5, 2018

bergmeister commented Mar 3, 2018 •

edited

Loading

bergmeister commented Apr 3, 2018 •

edited

Loading

bergmeister Apr 4, 2018 •

edited

Loading