/PowerShell

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

Add Windows Compat Pack 2.0.0 to PSCore6 and update package references to final versions including dotnetcore 2.1 #6958

Merged
merged 11 commits into from Jun 4, 2018

Conversation

Reviewers

@iSazonov iSazonov

@daxian-dbw daxian-dbw

@TravisEz13 TravisEz13

@adityapatwardhan adityapatwardhan

Assignees

@daxian-dbw daxian-dbw

Labels
None yet
Projects
None yet
Milestone
No milestone
7 participants
@SteveL-MSFT @daxian-dbw @markekraus @bergmeister @iSazonov @TravisEz13 @adityapatwardhan
@SteveL-MSFT
Member

SteveL-MSFT commented May 30, 2018 •
edited

PR Summary

To support PowerShell Modules built with .Net Windows Compatibility Pack, we decided that it was best to ship the WCP assemblies with PSCore6. This also adds many new .Net apis be default while only adding ~3.5 MB additional disk footprint (to a ~137 MB install currently).

Added ref to WCP 2.0.0 to SDK csproj.

Updated all refs of 4.5.0-RC1 to 4.5.0 final.

Update dotnetcore2.1 to final build

Update WebListener to aspnetcore.app 2.1.0 final
(Microsoft.VisualStudio.Web.CodeGeneration.Tools is not final yet, so no change)

PR Checklist

@SteveL-MSFT SteveL-MSFT requested review from adityapatwardhan , daxian-dbw and TravisEz13 as code owners May 30, 2018

@SteveL-MSFT SteveL-MSFT assigned daxian-dbw May 30, 2018

@daxian-dbw

daxian-dbw reviewed May 30, 2018
View changes

src/powershell-win-core/powershell-win-core.csproj Outdated
@@ -38,6 +38,7 @@
<PackageReference Include="PSDesiredStateConfiguration" Version="6.0.0-beta.8" />
<PackageReference Include="PowerShellHelpFiles" Version="1.0.0-*" />
<PackageReference Include="psrp.windows" Version="6.1.0-*" />
<PackageReference Include="Microsoft.Windows.Compatibility" Version="2.0.0" />

This comment has been minimized.

Sign in to view
Copy link
@daxian-dbw

daxian-dbw May 30, 2018

Member

I think this reference should be put in SDK.csproj with a Condition attribute, like <PackageReference Include="Microsoft.Windows.Compatibility" Version="2.0.0" Condition=" '$(IsWindows)' != 'true' " />. In this way, application that refernce to SDK nuget package will get those extended pack too.

IsWindows is defined in https://github.com/PowerShell/PowerShell/blob/master/PowerShell.Common.props#L113

This comment has been minimized.

Sign in to view
Copy link
@daxian-dbw

daxian-dbw May 30, 2018

Member

/cc @adityapatwardhan, in case I'm wrong about the SDK reference part.

This comment has been minimized.

Sign in to view
Copy link
@daxian-dbw

daxian-dbw May 30, 2018

Member

Actually, I don't know if we should even have the condition part. Some of the compat assemblies work on Unix platforms too.

This comment has been minimized.

Sign in to view
Copy link
@adityapatwardhan

adityapatwardhan May 30, 2018

Member

Yes, this should be part of SDK, so apps hosting PowerShell can use the modules with WCP.

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT May 30, 2018

Member

Perhaps we don't need the condition as @daxian-dbw is correct that some of the assemblies from the WCP meta-package work on non-Windows so we should make those available. I'll make the change.
@SteveL-MSFT
moved ref of WCP to SDK 
updated all refs of 4.5.0-RC1 to 4.5.0 final
Loading status checks…
f4b146f
@daxian-dbw

This comment has been minimized.

Sign in to view
Member

daxian-dbw commented May 30, 2018 •
edited

Ha, it looks dotnet core 2.1 is officially out: https://www.nuget.org/packages/Microsoft.NETCore.App/2.1.0
Shall we update our build to use the official 2.1?
@SteveL-MSFT
update dotnetcore to 2.1.300 RTM
Loading status checks…
24c5bc8

@SteveL-MSFT SteveL-MSFT changed the title Add Windows Compat Pack 2.0.0 to PSCore6 Add Windows Compat Pack 2.0.0 to PSCore6 and update package references to final versions including dotnetcore 2.1 May 30, 2018

@markekraus

This comment has been minimized.

Sign in to view
Collaborator

markekraus commented May 30, 2018

@SteveL-MSFT Can you push a commit with [feature] to ensure that the version bump didn't cause any regressions?
@bergmeister

This comment has been minimized.

Sign in to view
Contributor

bergmeister commented May 30, 2018 •
edited

@SteveL-MSFT I believe you also need to update the installer for .Net Core 2.1 RTM and cleanup comments, see my commit here
@SteveL-MSFT
[feature] 
update cache names for CI
update files.wxs to reflect new versions
Loading status checks…
625f106
@iSazonov

This comment has been minimized.

Sign in to view
Collaborator

iSazonov commented May 31, 2018

We have a issue with System.Text.Encoding.CodePages. Maybe not still published.
@iSazonov

This comment has been minimized.

Sign in to view
Collaborator

iSazonov commented May 31, 2018

We could use Version="4.5.*" for packages.
@iSazonov

This comment has been minimized.

Sign in to view
Collaborator

iSazonov commented May 31, 2018

we decided that it was best to ship the WCP assemblies with PSCore6

Can we use it directly - to access registry, perfcounters and so on?
@SteveL-MSFT

This comment has been minimized.

Sign in to view
Member

SteveL-MSFT commented May 31, 2018

@iSazonov System.Text.Encoding.CodePages 4.5.0 is published: https://www.nuget.org/packages/System.Text.Encoding.CodePages/
@SteveL-MSFT
[feature] 
update pwrshplugin.dll trusted assembly list
Loading status checks…
ce61eb9

@SteveL-MSFT SteveL-MSFT force-pushed the SteveL-MSFT:add-wcp branch to ce61eb9 May 31, 2018

@daxian-dbw

daxian-dbw reviewed May 31, 2018
View changes

src/powershell-native/nativemsh/pwrshcommon/pwrshcommon.cpp Outdated
"Microsoft.CodeAnalysis.CSharp",
"Microsoft.CodeAnalysis",
"Microsoft.CodeAnalysis.VisualBasic",

This comment has been minimized.

Sign in to view
Copy link
@daxian-dbw

daxian-dbw May 31, 2018

Member

Do we want to include this one? I think we will remove VB support before 6.1 RC. If we include it here now, we will have to update the windows.psrp package again when removing the VB support.

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Jun 1, 2018

Member

Will remove. We can always add it back later if needed.
@daxian-dbw

daxian-dbw reviewed May 31, 2018
View changes

src/Microsoft.PowerShell.SDK/Microsoft.PowerShell.SDK.csproj Outdated
@@ -15,20 +15,21 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Windows.Compatibility" Version="2.0.0" />

This comment has been minimized.

Sign in to view
Copy link
@daxian-dbw

daxian-dbw May 31, 2018

Member

If we don't know the source location of this package, maybe put it at the end, in the section with the comment the source could not be found for the following package(s), along with Microsoft.NETCore.Windows.ApiSets

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Jun 1, 2018

Member

Ok
@SteveL-MSFT
[feature] 
address Dongbo's feedback
Loading status checks…
0ee8d62
@SteveL-MSFT

This comment has been minimized.

Sign in to view
Member

SteveL-MSFT commented Jun 1, 2018

Can't tell why System.Text.Encoding.CodePages is failing to load as the signature matches the assembly.
@iSazonov

This comment has been minimized.

Sign in to view
Collaborator

iSazonov commented Jun 1, 2018

I suspect this is a publishing error. Could you ask .Net team?
@SteveL-MSFT

This comment has been minimized.

Sign in to view
Member

SteveL-MSFT commented Jun 1, 2018

@iSazonov that xunit test passes fine on my desktop machine, just not in AppVeyor though
@daxian-dbw

This comment has been minimized.

Sign in to view
Member

daxian-dbw commented Jun 1, 2018

I see the following from Start-PSxUnit:

            $requiredDependencies = @(
                $nativeLib,
                "$Content/Microsoft.Management.Infrastructure.dll",
                "$Content/System.Text.Encoding.CodePages.dll"
            )

The codepages.dll line is suspicious to me. But I haven't spent time digging further yet.
@SteveL-MSFT
[feature] 
Removed unncessary dep ref for psxunit on System.Text.Encoding.CodePages.dll
Updated packaging nupkg dep assembly versions
Loading status checks…
fdf87e4
@iSazonov

This comment has been minimized.

Sign in to view
Collaborator

iSazonov commented Jun 1, 2018

Errors in Appveyor tests:

[00:21:47]   Describing CmsMessage cmdlets thorough tests
[00:21:48] +++++++++++++++++++?+
[00:21:48] 
[00:21:48] New-Item : The directory specified, 'Modules', is not a subdirectory of 'C:\'.
[00:21:48] Parameter name: path
[00:21:48] At C:\projects\powershell\test\powershell\Modules\Microsoft.PowerShell.Security\ConstrainedLanguageRestriction.Tests.ps1:87 char:21
[00:21:48] + ...          $null = New-Item -ItemType Directory $moduleDirectory -Force
[00:21:48] +                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[00:21:48] + CategoryInfo          : InvalidArgument: (C:\Modules:String) [New-Item], ArgumentException
[00:21:48] + FullyQualifiedErrorId : CreateDirectoryArgumentError,Microsoft.PowerShell.Commands.NewItemCommand

[00:22:22]   Describing Invoke-Item basic tests
[00:22:22] 
[00:22:22]     Context Invoke a text file on Unix
[00:22:22] !!
[00:22:22] Get-Process : Cannot find a process with the name "notepad". Verify the process name and call the cmdlet again.
[00:22:22] At C:\projects\powershell\test\powershell\Modules\Microsoft.PowerShell.Utility\Invoke-Item.Tests.ps1:70 char:17
[00:22:22] +                 Get-Process -Name $notepadProcessName | Stop-Process  ...
[00:22:22] +                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[00:22:22] + CategoryInfo          : ObjectNotFound: (notepad:String) [Get-Process], ProcessCommandException
[00:22:22] + FullyQualifiedErrorId : NoProcessFoundForGivenName,Microsoft.PowerShell.Commands.GetProcessCommand

[00:37:57] new file {$(env.ProductSourcePath)\Microsoft.Win32.SystemEvents.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.CodeDom.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.ComponentModel.Composition.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Configuration.ConfigurationManager.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Data.DataSetExtensions.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Data.Odbc.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Diagnostics.PerformanceCounter.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.DirectoryServices.AccountManagement.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.DirectoryServices.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.DirectoryServices.Protocols.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Drawing.Common.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.IO.Ports.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Management.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Runtime.Caching.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Security.Cryptography.ProtectedData.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.Security.Cryptography.Xml.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57] new file {$(env.ProductSourcePath)\System.ServiceModel.Syndication.dll} need to be added to {C:\projects\powershell\tools\packaging\..\..\assets\Files.wxs}
[00:37:57]
@SteveL-MSFT

This comment has been minimized.

Sign in to view
Member

SteveL-MSFT commented Jun 1, 2018 •
edited

Ok, figured out the problem with System.Text.Encoding.CodePages.dll. The nupkg doesn't contain unix runtime only Windows. Older version has both. Created dotnet/corefx#30059

Moving back to v4.3.0 fixes that issue. Will update files.wxs and submit new commit. (v4.4.0 also doesn't have unix runtime)
@SteveL-MSFT
[feature] 
revert ref to System.Text.Encoding.CodePages to 4.3.0 as newer versions don't have Unix runtime
updated files.wxs
Loading status checks…
6c0fa8b

@SteveL-MSFT SteveL-MSFT force-pushed the SteveL-MSFT:add-wcp branch to 6c0fa8b Jun 1, 2018

SteveL-MSFT and others added some commits Jun 1, 2018

@SteveL-MSFT
[feature] 
revert CodePages back to 4.5.0-rc1
Loading status checks…
39628b7
@daxian-dbw
[Feature] Update xunit packages
Loading status checks…
40d3425
@daxian-dbw

This comment has been minimized.

Sign in to view
Member

daxian-dbw commented Jun 2, 2018

PSXunit tests passed on my machine (consistently failing previously) after I update the xunit packages. So push a change to see if that also fixes the issue in CI.
@SteveL-MSFT

This comment has been minimized.

Sign in to view
Member

SteveL-MSFT commented Jun 2, 2018

AppVeyor failure is due to recent PR to migrate the install docs, the aka.ms link now returned 404. I updated the aka.ms link so restarting AppVeyor.
@daxian-dbw

daxian-dbw approved these changes Jun 2, 2018
View changes

LGTM

@iSazonov

iSazonov approved these changes Jun 2, 2018
View changes

Leave a comment

src/System.Management.Automation/System.Management.Automation.csproj
<PackageReference Include="System.Security.AccessControl" Version="4.5.0" />
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="4.5.0" />
<PackageReference Include="System.Security.Permissions" Version="4.5.0" />
<PackageReference Include="System.Text.Encoding.CodePages" Version="4.5.0" />
<!-- the following package(s) are from the powershell org -->
<PackageReference Include="Microsoft.Management.Infrastructure" Version="1.0.0-alpha*" />
<PackageReference Include="PowerShell.Core.Instrumentation" Version="6.0.0-beta*" />

This comment has been minimized.

Sign in to view
Copy link
@iSazonov

iSazonov Jun 2, 2018

Collaborator

Have we this APIs in WCP?

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Jun 2, 2018

Member

These are pkgs PowerShell Team publishes

This comment has been minimized.

Sign in to view
Copy link
@iSazonov

iSazonov Jun 3, 2018 •
edited

Collaborator

Should we migrate to WCP assemblies if we have them on long time?

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Jun 3, 2018

Member

You mean include them in WCP? Probably not.
src/powershell-native/nativemsh/pwrshcommon/pwrshcommon.cpp
"Microsoft.CodeAnalysis.CSharp",
"Microsoft.CodeAnalysis",

This comment has been minimized.

Sign in to view
Copy link
@iSazonov

iSazonov Jun 2, 2018

Collaborator

Why was the order changed?

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Jun 2, 2018

Member

The list was generated rather than trying to insert a large number of new assemblies into the list. Sort-Object seems to prefer the period over end of string.
test/csharp/csharp.tests.csproj
<!-- DotNetCliToolReference element specifies the CLI tool that the user wants to restore in the context of the project. -->
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" />
<PackageReference Include="Xunit.SkippableFact" Version="1.3.3" />
<DotNetCliToolReference Include="dotnet-xunit" Version="2.4.0-beta.1.build3958" />

This comment has been minimized.

Sign in to view
Copy link
@iSazonov

iSazonov Jun 2, 2018

Collaborator

I think we should open new tracking issue to update the beta to release.
And Microsoft.VisualStudio.Web.CodeGeneration.Tools too.

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Jun 2, 2018

Member

Created #6977
tools/packaging/packaging.psm1
@@ -1409,12 +1409,12 @@ function New-UnifiedNugetPackage

'Microsoft.PowerShell.Commands.Management' {
$deps.Add([tuple]::Create([tuple]::Create('id', 'Microsoft.PowerShell.Security'), [tuple]::Create('version', $PackageVersion))) > $null
$deps.Add([tuple]::Create([tuple]::Create('id', 'System.ServiceProcess.ServiceController'), [tuple]::Create('version', '4.4.1'))) > $null
$deps.Add([tuple]::Create([tuple]::Create('id', 'System.ServiceProcess.ServiceController'), [tuple]::Create('version', '4.5.0'))) > $null

This comment has been minimized.

Sign in to view
Copy link
@iSazonov

iSazonov Jun 2, 2018

Collaborator

Not related the PR - why we set the versions manually and do not take from csproj files?

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Jun 2, 2018

Member

#6978
assets/files.wxs
@@ -4,6 +4,57 @@
<?define FileArchitecture = "$(env.FileArchitecture)" ?>
<Fragment>
<DirectoryRef Id="$(var.ProductDirectoryName)">
<Component Id="cmpCBE3857586454EB3B634B118E8EBD1D1" Guid="{CBE38575-8645-4EB3-B634-B118E8EBD1D1}">
<File Id="filCBE3857586454EB3B634B118E8EBD1D1" KeyPath="yes" Source="$(env.ProductSourcePath)\Microsoft.Win32.SystemEvents.dll" />
</Component>

This comment has been minimized.

Sign in to view
Copy link
@iSazonov

iSazonov Jun 2, 2018

Collaborator

I lost understanding - why do we update the file manually? Can we generate it automatically?

This comment has been minimized.

Sign in to view
Copy link
@SteveL-MSFT

SteveL-MSFT Jun 2, 2018

Member

#6978

This comment has been minimized.

Sign in to view
Copy link
@TravisEz13

TravisEz13 Jun 5, 2018

Member

The WiX tools will break patching. When you build, I added errors to tell you what to add or remove, but didn't fully automate the update. I updated the issue with the requirements of the update.
@iSazonov

This comment has been minimized.

Sign in to view
Collaborator

iSazonov commented Jun 2, 2018

Newtonsoft.Json latest version is 11.0.1
https://github.com/JamesNK/Newtonsoft.Json/releases

NJsonSchema latest version is 9.10.50
https://www.nuget.org/packages/NJsonSchema/

@SteveL-MSFT SteveL-MSFT referenced this pull request Jun 2, 2018

Closed

Update PackageReferences to GA releases #6977

@SteveL-MSFT
[feature] 
update Newtonsoft.Json and NJsonSchema to latest versions
Loading status checks…
ceb25f0

@SteveL-MSFT SteveL-MSFT force-pushed the SteveL-MSFT:add-wcp branch to ceb25f0 Jun 2, 2018

@daxian-dbw daxian-dbw merged commit 5dc7a6c into PowerShell:master Jun 4, 2018
5 checks passed

5 checks passed

CodeFactor No issues found.
Details
WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details

@SteveL-MSFT SteveL-MSFT deleted the SteveL-MSFT:add-wcp branch Jun 5, 2018

@mi-hol mi-hol referenced this pull request Jun 20, 2018

Closed

Question: has it been tested with PowerShell Core 6.0.0? #65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment