Expose platform specific assemblies which are already exposed in the runtime pack in the targeting pack

[ViktorHofer]

There are five assemblies which are currently not exposed in the .NETCoreApp targeting pack because they are platform specific (Windows):

runtime/src/libraries/NetCoreAppLibrary.props

Lines 148 to 154 in 54fdd26

	<NetCoreAppLibraryNoReference>
	Microsoft.Win32.Registry;
	System.IO.FileSystem.AccessControl;
	System.IO.Pipes.AccessControl;
	System.Security.AccessControl;
	System.Security.Principal.Windows;
	</NetCoreAppLibraryNoReference>

As the platform analyzers warns for usages of APIs which aren't available on specific platforms, it should be considered to expose the few missing reference assemblies which are already part of the runtime pack. The big benefit of doing so is that customers don't need to reference the corresponding packages to target these APIs and that their packages can be dead-ended as their APIs are de-facto frozen.

Discussed this with @ericstj and @terrajobst and they supportive. Ideally we would like to get this into Preview 6 which means we would want to reach consensus sooner than later. Thoughts on this?

cc @adamsitnik @carlossanlop @jozkee @jeffhandley for System.IO.FileSystem.AccessControl and System.IO.Pipes.AccessControl
cc @bartonjs @GrabYourPitchforks @jeffhandley for System.Security.AccessControl and System.Security.Principal.Windows
cc @Anipik and myself for Microsoft.Win32.Registry

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

---

There are five assemblies which are currently not exposed in the .NETCoreApp targeting pack because they are platform specific (Windows):

runtime/src/libraries/NetCoreAppLibrary.props

Lines 148 to 154 in 54fdd26

	<NetCoreAppLibraryNoReference>
	Microsoft.Win32.Registry;
	System.IO.FileSystem.AccessControl;
	System.IO.Pipes.AccessControl;
	System.Security.AccessControl;
	System.Security.Principal.Windows;
	</NetCoreAppLibraryNoReference>

.

As the platform analyzers warns for usages of APIs which aren't available on specific platforms, it should be considered to expose the few missing reference assemblies which are already part of the runtime pack. The big benefit of doing so is that customers don't need to reference the corresponding packages to target these APIs and that their packages can be dead-ended as their APIs are de-facto frozen.

Discussed this with @ericstj and @terrajobst and they supportive. Ideally we would like to get this into Preview 6 which means we would want to reach consensus sooner than later. Thoughts on this?

cc @adamsitnik @carlossanlop @jozkee @jeffhandley for System.IO.FileSystem.AccessControl and System.IO.Pipes.AccessControl
cc @bartonjs @GrabYourPitchforks @jeffhandley for System.Security.AccessControl and System.Security.Principal.Windows
cc @Anipik and myself for Microsoft.Win32.Registry

Author:	ViktorHofer
Assignees:	-
Labels:	area-Infrastructure-libraries
Milestone:	6.0.0

[jkotas]

their packages can be dead-ended as their APIs are de-facto frozen.

Why is the API of these packages de-facto frozen?

[jkotas]

The big benefit of doing so is that customers don't need to reference the corresponding packages to target these APIs

It is benefit if you are targeting Windows. If you are targeting cross-plat, you just get more useless cruft in intelinse.

[ViktorHofer]

Why is the API of these packages de-facto frozen?

We don't want to accidently ship new APIs in a .NETStandard or .NETFramework ref in a package that is already inbox on any platform. For .NETCoreApp, the APIs can only evolve together with the runtime as their runtime assets are inbox.

The customer benefit of exposing these platform specific assemblies in the targeting pack are:

• No need to reference a package to be able to use these APIs
• Platform analyzer support that warns when these APIs are being used on an unsupported platform instead of just a PNSE assembly throwing on runtime. Even though the APIs appear on Intellisense, the analyzer warns when they are being used.
• All of these assemblies are already exposed on upstream targeting packs (that's the list from 5.0.0):

  runtime/src/libraries/NetCoreAppLibrary.props

  Lines 156 to 225 in 3b47bf8

  	<AspNetCoreAppLibrary>
  	Microsoft.Extensions.Caching.Abstractions;
  	Microsoft.Extensions.Caching.Memory;
  	Microsoft.Extensions.Configuration;
  	Microsoft.Extensions.Configuration.Abstractions;
  	Microsoft.Extensions.Configuration.Binder;
  	Microsoft.Extensions.Configuration.CommandLine;
  	Microsoft.Extensions.Configuration.EnvironmentVariables;
  	Microsoft.Extensions.Configuration.FileExtensions;
  	Microsoft.Extensions.Configuration.Ini;
  	Microsoft.Extensions.Configuration.Json;
  	Microsoft.Extensions.Configuration.UserSecrets;
  	Microsoft.Extensions.Configuration.Xml;
  	Microsoft.Extensions.DependencyInjection;
  	Microsoft.Extensions.DependencyInjection.Abstractions;
  	Microsoft.Extensions.FileProviders.Abstractions;
  	Microsoft.Extensions.FileProviders.Composite;
  	Microsoft.Extensions.FileProviders.Embedded;
  	Microsoft.Extensions.FileProviders.Physical;
  	Microsoft.Extensions.FileSystemGlobbing;
  	Microsoft.Extensions.Hosting;
  	Microsoft.Extensions.Hosting.Abstractions;
  	Microsoft.Extensions.Http;
  	Microsoft.Extensions.Logging;
  	Microsoft.Extensions.Logging.Abstractions;
  	Microsoft.Extensions.Logging.Configuration;
  	Microsoft.Extensions.Logging.Console;
  	Microsoft.Extensions.Logging.Debug;
  	Microsoft.Extensions.Logging.EventLog;
  	Microsoft.Extensions.Logging.EventSource;
  	Microsoft.Extensions.Logging.TraceSource;
  	Microsoft.Extensions.Options;
  	Microsoft.Extensions.Options.ConfigurationExtensions;
  	Microsoft.Extensions.Options.DataAnnotations;
  	Microsoft.Extensions.Primitives;
  	Microsoft.Extensions.WebEncoders;
  	Microsoft.Win32.Registry;
  	System.Diagnostics.EventLog;
  	System.IO.Pipelines;
  	System.Security.AccessControl;
  	System.Security.Cryptography.Cng;
  	System.Security.Cryptography.Xml;
  	System.Security.Permissions;
  	System.Security.Principal.Windows;
  	System.Windows.Extensions;
  	</AspNetCoreAppLibrary>
  	<WindowsDesktopCoreAppLibrary>
  	Microsoft.Win32.Registry;
  	Microsoft.Win32.Registry.AccessControl;
  	Microsoft.Win32.SystemEvents;
  	System.CodeDom;
  	System.Configuration.ConfigurationManager;
  	System.Diagnostics.EventLog;
  	System.Diagnostics.PerformanceCounter;
  	System.DirectoryServices;
  	System.Drawing.Common;
  	System.IO.FileSystem.AccessControl;
  	System.IO.Packaging;
  	System.IO.Pipes.AccessControl;
  	System.Resources.Extensions;
  	System.Security.AccessControl;
  	System.Security.Cryptography.Cng;
  	System.Security.Cryptography.Pkcs;
  	System.Security.Cryptography.ProtectedData;
  	System.Security.Cryptography.Xml;
  	System.Security.Permissions;
  	System.Security.Principal.Windows;
  	System.Threading.AccessControl;
  	System.Windows.Extensions;
  	</WindowsDesktopCoreAppLibrary>

  . Adding these to the Microsoft.NETCore.App.Ref targeting pack as well makes targeting these APIs more consistent across our products (.NETCoreApp, AspNetCore, WindowsDesktop).

The benefits we get out doing this are:

• We can dead end the existing packages which don't change much as the APIs are already available inbox. S.S.Principal.Windows which carries along a functional .NETStandard asset will be trimmed out by conflict resolution anyway as its asset is already available inbox.
• By dead-ending the package, we don't have to support and build them anymore which lowers the complexity of the projects.
• Upstream targeting packs can remove these reference assemblies from their targeting packs and can also eliminate the package references.
• Supporting references which are exposed in packages and their runtime asset is available in the framework add complexity to the build and test infrastructure in dotnet/runtime. A recent example of that is the package testing infrastructure where the VerifyClosure task can't handle the omission of the reference assembly which would come in transitively via System.Management but as it's marked as Exclude="compile", the build doesn't see it: https://github.com/dotnet/runtime/blob/3855df9f0b7825e5da7ce144ca742309d55950a7/src/libraries/pkg/test/packageSettings/System.Management/net6.0/disableVerifyClosure.targets.

It is benefit if you are targeting Windows. If you are targeting cross-plat, you just get more useless cruft in intelinse.

The proposal consists of adding these APIs to the Microsoft.NETCore.App.Ref targeting pack:

• https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Win32.Registry/ref/Microsoft.Win32.Registry.cs
• https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.FileSystem.AccessControl/ref/System.IO.FileSystem.AccessControl.cs
• https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.Pipes.AccessControl/ref/System.IO.Pipes.AccessControl.cs
• https://github.com/dotnet/runtime/blob/main/src/libraries/System.Security.AccessControl/ref/System.Security.AccessControl.cs
• https://github.com/dotnet/runtime/blob/main/src/libraries/System.Security.Principal.Windows/ref/System.Security.Principal.Windows.cs

Trying to quantify the addition, we would bring in 82 types (13+7+4+46+12) and most of those are already exposed in the aspnetcore targeting pack which is cross-plat. It would be interesting to explore if Intellisense could hide unsupported types when targeting a specific platform.

Does that make sense? If we want to go into more detail, we could meet with Immo to further discuss the addition.

[jkotas]

We don't want to accidently ship new APIs in a .NETStandard or .NETFramework ref in a package that is already inbox on any platform. For .NETCoreApp, the APIs can only evolve together with the runtime as their runtime assets are inbox.

I do not think that this is correct statement. This restriction only applies to packages that are implemented via CoreLib forwarder inbox. It does not apply to pure packages that just happen to be included in the platform, like the packages discussed here. Other examples include System.Text.Json, System.Collections.Immutable or System.Reflection.Metadata. These packages are inbox, and we have added and we will likely add more API to the .NETStandard ref in these packages. Or is there some fundamental difference between the packages discussed here and S.T.Json, S.C.Immutable or S.R.Metadata?

I do not have a problem with doing this if we have good reasons. I have commented on this issue since the reasons stated in the top post did not look valid to me.

[ViktorHofer]

Or is there some fundamental difference between the packages discussed here and S.T.Json, S.C.Immutable or S.R.Metadata?

I agree that my statement wasn't precise. These package differ in that their reference assembly isn't exposed in the targeting pack even though their implementation assemblies are. That's probably the main difference to the packages that you mentioned.

I do not have a problem with doing this if we have good reasons. I have commented on this issue since the reasons stated in the top post did not look valid to me.

Thanks for raising your concern, I should have gone into more detail in the top post. I believe that the reasons are valid and justify the change. Assigning this to myself to make sure this gets in sooner than later so that partner teams can react to it earlier in the cycle than later.