Skip to content

Add netstandard2.0 to Microsoft.CodeAnalysis.Common? #26299

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

Closed
natemcmaster opened this issue Apr 20, 2018 · 3 comments
Closed

Add netstandard2.0 to Microsoft.CodeAnalysis.Common? #26299

natemcmaster opened this issue Apr 20, 2018 · 3 comments
Assignees
Milestone

Comments

@natemcmaster
Copy link

natemcmaster commented Apr 20, 2018

Problem
As of Microsoft.CodeAnalysis.Common 2.8.0-beta4, this package still targets netstandard1.3. This leads to unnecessary bloat in the package graph. Is there any reason we can't add netstandard2.0 (in addition to ns1.3)?

Some results of adding ns2.0 may include:

Version Used:
2.8.0-beta4

Steps to Reproduce:

  1. Download latest .NET Core 2.1 preview builds https://github.com/dotnet/versions/tree/master/build-info/dotnet/product/cli/release/2.1#sdk-installers-and-binaries
  2. dotnet new web
  3. Look at the restore graph

Expected Behavior:
Ideally, ASP.NET Core 2.1 applications shouldn't have .NET Standard 1.x packages in the graph anymore.

Actual Behavior:
The restore graph contains a bunch of packages that are never actually needed to compile a ASP.NET Core 2.1 application. ASP.NET Core depends on roslyn for Razor support, so Microsoft.CodeAnalysis.Common (and all its dependencies System) ends up in the restore graph for aspnet 2.0 apps, but most of the assemblies from the System.IO packages get removed from compilation references because these are already satisfied by newer versions delivered via NETStandard.Library 2.0 or Microsoft.NETCore.App 2.x

@jaredpar
Copy link
Member

Is there any reason we can't add netstandard2.0 (in addition to ns1.3)?

To be clear we definitely can't change from netstandard 1.3 to netstandard 2.0. I know you didn't ask for that but I wanted to elaborate on it a bit for clarity. The reason is that every desktop component we ship in requires us to target net46 (yes this is true even though we only ever actually run on net461+). This is why we continue to target netstandard 1.3.

It is possible for us to add netstandard 2.0 as another target. However that would impact us in a number of ways:

  1. Need to move all of our compiler test projects to multi-target net46 and net461 to ensure we cover both cases.
  2. Need to move all of our other test and product projects back to net46. Many today target net461 and that would cause them to incorrectly injest the netstandard2.0 build.
  3. Would have a significant impact on our CI turn around time for PRs as it would effectively double the number of compiler tests that were run.

This is all work though, not a functional blocker.

make it easier to build ASP.NET Core from source

I don't see how this would make it easier. Source build doesn't need to build any of the netstandard 1.3 or netstandard 2.0 reference assemblies from source. True it does today but that's just a point in time problem. RedHat agreed that these reference assemblies are essentially header files. They don't need to be built as a part of the source build effort.

faster restore times
faster builds (maybe). Less time spent de-duping System.* references

Whose restore and build? This would make our restore and build decidedly worse 😄

reduce size of the .NET Core SDK installers (we think we can save ~10-15 MB in the offline package cache if we can get all aspnet dependencies off netstandard1.x packages)

This is getting into a gray area of my understanding of the SDK but I don't believe this is the case. Given that Roslyn would still have netstandard1.3 as a target won't the offline cache need to support that target as well?

Our current plans for netstandard 2.0 are essentially to wait for VS to move off of net46 which allows us to switch completely.

@natemcmaster
Copy link
Author

I figured this was no small ask. Thanks for clarifying why this is a lot of work right now.

Some responses to your points.

make it easier to build ASP.NET Core from source

I was mostly thinking about this: source-build already has a bunch of dummy packages required to satisfy the restore step during builds. Adding Microsoft.CodeAnalysis.Common's dependencies just expands that graph because it depends on System.Security.Cryptography.X509Certificates -> runtime.native.System.Security.Cryptography.OpenSsl -> (dozens of runtime.native.* packages). For each dummy package, we need to make sure the dummy package satisfies restore, but without borking the generated .deps.json files for the aspnet shared frameworks and console tools.

There may also be a long-term argument to be made, which is, it would be awesome if the .nupkgs required to build for ASP.NET Core were also source-build able, and not just the runtime. For now, we're scoping that out for a lot of reasons; dependency on netstandard1.x packages is one of those.

reduce size of the .NET Core SDK installers

The offline cache currently only needs to support restores against the netstandard2.0 and netcoreapp2.1 frameworks. That means we only need a subset of the full package graph. If Roslyn cross-targeted netstandard2.0, we wouldn't need the netstandard1.3 dependencies in the cache. Roslyn and IdentityModel packages are the last two pulling netstandard1.x packages. (IdentityModel will hopefully bump to ns20 soon - AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#925). Once both have a netstandard2.0 TFM, we're expecting most of the System.* and runtime.native.* packages will naturally drop out of the offline cache.

@jaredpar
Copy link
Member

Closing in favor of the more general issue #29601

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

No branches or pull requests

3 participants