Bring back System.IO.Pipes.AccessControl package and make it available in .NET 5.0

[carlossanlop]

Fixes: #38123

Pending making sure we don't regress this: #30813

cc @joperezr @safern @ericstj @Jozkee

[ghost]

Tagging subscribers to this area: @safern, @ViktorHofer
Notify danmosemsft if you want to be subscribed.

[safern]

You need to add the new PackageVersion here and map it to the AssemblyVersion:
https://github.com/dotnet/runtime/blob/master/src/libraries/pkg/baseline/packageIndex.json#L3384

[carlossanlop]

@ericstj What is it exactly that you want me to test to make sure we don't regress the wpf scenario? @joperezr and I did try running the commands specified on that issue using my new package and it does still flag a conflict on that assembly, but Joe and I believe that this problem is caused by the netcoreapp3.0 targeting pack desktop facades which still forward to specific versions (as opposed to just forwarding to 0.0.0.0). My new package is not fixing this as I am just harvesting for everything except .net5 but it is also not making it worse, we basically have the same behavior than the previous package except that we now have an extra .NET 5 asset.

[ericstj]

@ericstj What is it exactly that you want me to test to make sure we don't regress the wpf scenario?

We should find where WindowsDesktop consumes the old package and make sure it is set up to subscribe to the new package we build. Here's what I found:
https://github.com/dotnet/windowsdesktop/blob/3285e1b58507eb88a1cd7be4ee03fe08cbbe6b12/pkg/windowsdesktop/src/windowsdesktop.depproj#L18

[ericstj]

Also we should make sure we undo the stable version reference from Windows.Compatibility:

runtime/src/libraries/pkg/Microsoft.Windows.Compatibility/Microsoft.Windows.Compatibility.pkgproj

Lines 51 to 54 in 54625f3

	<!-- Packages we don't build in master anymore -->
	<LibraryPackage Include="System.IO.Pipes.AccessControl">
	<Version>4.5.1</Version>
	</LibraryPackage>

[safern]

@ericstj believes that we should be able to get away without setting this and using 5.0.0.0

[joperezr]

I don't think so, because we are still harvesting runtimes/win/lib/netcoreapp2.1 asset that is 4.0.3.0, so if we make lib/netcoreapp2.1 be 5.0.0.0 our packaging validation will fail saying that the compile time asset was found to support 5.0.0.0 but the runtime asset was found to support 4.0.3.0. If we wanted to do this, we would have to go and bring back the netcoreapp2.1-Windows_NT live build.

[safern]

Ah that's right.

[ericstj]

I thought we only needed to build net5.0 assets since that was the only place we could add surface area. @carlossanlop?

[safern]

This is needed in order to avoid bringing the refs. This asset will only be used for compile time; but I guess that's a good point.

We would need to exclude this APIs for older targets so that they're not available there.

[safern]

See here: #38177 (comment)

[ericstj]

Actually, maybe the goal of avoiding using refs here might have been bad advice on my part. This is a full-facade in src, but contains typedefs in ref. By only shipping the src assembly we're shipping dangling type-forwards to the compiler 🤦‍♂️

[safern]

Ah that's right... dang it. No worries, none of use caught it either 😄

[carlossanlop]

Also we should make sure we undo the stable version reference from Windows.Compatibility:

runtime/src/libraries/pkg/Microsoft.Windows.Compatibility/Microsoft.Windows.Compatibility.pkgproj

Lines 51 to 54 in 54625f3

	<!-- Packages we don't build in master anymore -->
	<LibraryPackage Include="System.IO.Pipes.AccessControl">
	<Version>4.5.1</Version>
	</LibraryPackage>

Addressed in the latest commit, @ericstj. PTAL.

[joperezr]

@ericstj we needed to add the netcoreapp2.1 config in order to workaround the fact that we were missing two declared dependencies in NCA2.1 given that it would take now the NS2.0 assembly as the compile asset. This workaround is less than ideal, but likely preferable to add back the refs to the package, do you agree with that

[safern]

I chatted with him on my 1:1 with regards on this, and he said that he prefers doing this than bringing back the ref assemblies and that seems like the best workaround.

He also suggested that we could exclude the compile asset from the package and so it would only be used to calculate dependencies but that either way was fine.

[ericstj]

I'm a little surprised we didn't copy the PackageDependencies from netstandard2.0 to netcoreapp2.1 here: https://github.com/dotnet/arcade/blob/2fd7a99a9bc16b04312b816e3ff0aade830efbeb/src/Microsoft.DotNet.Build.Tasks.Packaging/src/build/Packaging.targets#L680

Barring that another hack we could use here would be to copy the netstandard2.0 harvested asset into lib/netcoreapp2.1 as well.

[carlossanlop]

Test failure is unrelated to this change:
https://helix.dot.net/api/2019-06-17/jobs/ad7459ad-b88a-45eb-8f18-e1ddfd7d7154/workitems/System.Threading.Channels.Tests/console