Skip to content
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

Update System.Reflection.Metadata to 6.0.1 #10516

Closed
am11 opened this issue Aug 19, 2022 · 12 comments
Closed

Update System.Reflection.Metadata to 6.0.1 #10516

am11 opened this issue Aug 19, 2022 · 12 comments

Comments

@am11
Copy link
Member

am11 commented Aug 19, 2022

Updating System.Reflection.Metadata requires extra care and coordination in several repositories, since VS, MSBuild, Roslyn, SDK and HostModel from runtime rely on it. Currently most reprostories are at 6.0.0, while runtime is moved to 6.0.1, arcade is at 1.6.1 and aspnetcore has switched to 7.0 rc1. This is to unify its version to 6.0.1 so we stay on the latest patch of v6 band.

cc @ViktorHofer, @jkoritzinsky
context: dotnet/runtime#74231 (comment)

@ViktorHofer
Copy link
Member

@dougbu
Copy link
Member

dougbu commented Aug 20, 2022

Sorry, how would this work and why is it necessary❔ dotnet/aspnetcore gets its System.Reflection.Metadata version through the normal dependency flow and an override from Arcade would likely conflict as well as causing confusion. A downgrade also doesn't sound appealing.

Put another way, why not use dependency flow everywhere❔

@jkoritzinsky
Copy link
Member

With the issues with assembly loading and binding redirects on desktop MSBuild/VS, we generally end up needing to align SRM versions to some level to avoid conflicts around the System.Collections.Immutable.dll assembly version or we cause breaks in the SDK.

@dougbu
Copy link
Member

dougbu commented Aug 21, 2022

  1. Arcade doesn't seem able to enforce a maximum version and its overrides are easily ignored in consuming repos
  2. Why not settle on the version dotnet/runtime puts into the .NET 6, .NET 7, and .NET 8 channels❔

@ViktorHofer
Copy link
Member

Why not settle on the version dotnet/runtime puts into the .NET 6, .NET 7, and .NET 8 channels❔

Unsure if that would work for the Visual Studio case. @rainersigwald @marcpopMSFT do you know the package version requirements for VS components?

@rainersigwald
Copy link
Member

VS is fairly flexible on when it can update dependencies AFAIK but it can be a long process of onion-peeling to get them in--I generally have to coordinate with @jaredpar and @AArnott and get review from several teams that have dependencies.

However, I'm not sure that the SDK version needs to match the MSBuild/VS version. .NET Framework should be able to load assemblies of different versions side-by-side, and there should not be any SRM types in public interfaces between the SDK and MSBuild (MSBuild doesn't expose any that I know of). As such, as long as the .NET Framework copy of SRM.dll is in the SDK's .NET Framework tasks folder, it should be able to freely update to runtime-latest. @am11 was that not the case?

@jaredpar
Copy link
Member

VS is fairly flexible on when it can update dependencies AFAIK but it can be a long process of onion-peeling to get them in

For major releases it's still an onion-peeling effort but overall I think we have a decent handle on the problem. Basically it's a slog but we know the path we have to take.

Servicing fixes are a different problem entirely because the runtime uses different assembly versions for the different binaries. The net6.0 binaries end up with 6.0.0.0 while netstandard2.0 ends up with 6.0.0.1. That is a big problem as Visual Studio is only setup to have one version per binary at the moment. It's possible to get Visual Studio to have TFM specific versions but that is work that hasn't been done yet.

When we initially looked at inserting SRM 6.0.0.1 into Visual Studio @AArnott had a PR where he'd attempted to do something similar. But we looked at it and decided the amount of work involved was very high and it was probably cheaper to just wait for net7.0 to ship and insert SRM 7.0.0.0 instead.

@AArnott
Copy link

AArnott commented Aug 23, 2022

That is a big problem as Visual Studio is only setup to have one version per binary at the moment. It's possible to get Visual Studio to have TFM specific versions but that is work that hasn't been done yet.

VS actually does have multiple versions of many assemblies (too many, in fact). We're meeting regularly now to try to reduce both the duplication of the same version of assemblies and the span of versions that a given assembly simple name has. But in the meantime, nothing stops .NET 6 processes from having and sharing another version of the assembly.
The design meetings and prototypes we're building are attempting to unify the way we share assemblies across VS processes, including across runtimes, so yes, this story will hopefully improve soon.

@jaredpar
Copy link
Member

jaredpar commented Aug 23, 2022

VS actually does have multiple versions of many assemblies (too many, in fact). We're meeting regularly now to try to reduce both the duplication of the same version of assemblies and the span of versions that a given assembly simple name has.

Please correct me if I'm wrong but I think we're discussing two different problems.

The problem I believe you're discussing is when two teams pick two different versions of a NuPkg. For example team one picks Awesome.Util version 1.0 and team two picks Awesome.Util version 2.0. This overall bloats the install size of Visual Studio, increases complexity because we're juggling more dependencies, etc ... It's a problem that can be fixed simply by the two teams agreeing on a single version.

The problem I'm discussing is when everyone agrees on a single version of a package: SRM 6.0.0.1. The problem is the implementation of that NuPkg uses different assembly versions based on the TFM chosen. In this case net472 has version 6.0.0.1 and netstandard2.0 / net6.0 has 6.0.0.0. There is no way of fixing this problem because it's a choice forced on us by the way in which the runtime team versions assemblies.

Potentially we could fix it by saying that we use the netstandand2.0 version everywhere. Essentially even if you're building a net6.0 process you get the netstandard2.0 version. That would likely lead to a host of other problems though. I don't think it's a realistic option (happy to be persuaded differently).

@dougbu
Copy link
Member

dougbu commented Aug 23, 2022

I think the $(AssemblyVersion) rules in dotnet/runtime are the same as in dotnet/aspnetcore: Basically, let Arcade control the numbers (increasing in every release) except for .NET Framework assemblies that match assemblies in the shared framework (but ship separately). Since S.R.M. is in Microsoft.NETCore.App, that should mean the netstandard2.0 assembly version is 6.0.0.0 too. That's what I see in the S.R.M v6.0.1 package:
image

@AArnott
Copy link

AArnott commented Aug 23, 2022

In this case net472 has version 6.0.0.1 and netstandard2.0 / net6.0 has 6.0.0.0. There is no way of fixing this problem because it's a choice forced on us by the way in which the runtime team versions assemblies.

Ah, ok. We do have this problem already (I think with S.C.I.) We solve it be applying a binding redirects that includes the backwards element. For example: 0.0.0.0-6.0.0.1->6.0.0.0. So folks can reference the netstandard2.0 6.0.0.1 assembly version and still work against the net472 6.0.0.0 assembly version. Folks should not have to avoid referencing the assembly version that nuget naturally provides. We can fix a binding redirect if this applies to more assemblies than are currently accounted for in the .config file.

I agree with what I think @jaredpar is saying, that we should always ship the runtime-specific build of an assembly for loading into a running process instead of something else like netstandard, which could potentially be meant as a ref assembly since the runtime assembly is expected to always be chosen for runtime.

@michellemcdaniel
Copy link
Contributor

Arcade was not designed to enforce versions of packages used in the individual product repos. Repos will need to coordinate with each other to make sure that their versions of this package are aligned.

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

No branches or pull requests

8 participants