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

What's the release policy the libraries? Why the nuget required dependency version did not match the real version? #45107

Closed
chucklu opened this issue Nov 23, 2020 · 18 comments
Labels
area-System.Text.Json question Answer questions and provide assistance, not an issue with source code or documentation.

Comments

@chucklu
Copy link

chucklu commented Nov 23, 2020

The System.Text.Json 4.7.2 shows it depends on System.ValueTuple (>= 4.5.0) (and the assembly version is 4.0.3.0) for .NETFramework 4.6.1.
However when I get the System.Text.Json 4.7.2 library, and check the references by dnSpy, and it show that the dependency is
// System.ValueTuple, Version=4.0.2.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
and System.ValueTuple assembly version 4.0.2.0 is mapped to nuget version 4.4

The System.Text.Json 4.7.2 actually depends on System.ValueTuple nuget 4.4, why you marked it as System.ValueTuple (>= 4.5.0)?

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Nov 23, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@chucklu
Copy link
Author

chucklu commented Nov 23, 2020

And this could be problem, when I nuget try to resolve the bindingRedirect
NuGet/Home#10302 (comment)
I have created two branches for tests, it seems the different reference order make the auto bindingRedirect behave differently
https://github.com/ChuckTest/NuGet-10302/tree/class-library-first
https://github.com/ChuckTest/NuGet-10302/tree/console-app-first
Currently console app reference class library,
When in console-app-first mode, it means reference CsvHelper 12.1.2 in console app first, then reference Microsoft.Extensions.Configuration.Json 3.1.6 in class library.
When in class-library-first mode, it means reference Microsoft.Extensions.Configuration.Json 3.1.6 in class library first, then reference CsvHelper 12.1.2 in console app .

And at last, the bindingRedirect is different for these two mode. console-app-first mode will lose bindingRedirect for System.ValueTuple.

@chucklu
Copy link
Author

chucklu commented Nov 23, 2020

From my test case, when use console app first mode.
Under the following order,
Console App1-->ClassLibrary1
Console App1-->CsvHelper-->System.ValueTuple(>= 4.4.0)
ClassLibrary1-->System.Text.Json (>= 4.7.2)-->System.ValueTuple (>= 4.5.0)
The auto generated bindingRedirect will lose the redirect for System.ValueTuple, which means the System.ValueTuple (>= 4.5.0) will not take effect.

Is this corect behavior?

@chucklu
Copy link
Author

chucklu commented Nov 23, 2020

From my test case, when use class library first mode.
Under the following order,
Console App1-->ClassLibrary1
ClassLibrary1-->System.Text.Json (>= 4.7.2)-->System.ValueTuple (>= 4.5.0)
Console App1-->CsvHelper-->System.ValueTuple(>= 4.4.0)
The auto generated bindingRedirect will have the redirect for System.ValueTuple, which means the System.ValueTuple (>= 4.5.0) will take effect.
However the package System.ValueTuple installed in Console App1 is still keep as System.ValueTuple 4.4.0.
Currently nuget is not able to unify the package version to higher across projects automatically. This time I when rebuild solution, I will get a warning about the version mismatch, and have to resolve it manually.(However in console app first mode, there is no build warning)

@GrabYourPitchforks GrabYourPitchforks added question Answer questions and provide assistance, not an issue with source code or documentation. area-Meta labels Nov 23, 2020
@huoyaoyuan
Copy link
Member

Why the nuget required dependency version did not match the real version?

Package version and assembly version are different things. They are handled by different systems.

For historical technical reason, they are mismatched.

.NET Framework 4.x was released 10 years ago, and it takes the 4.x version numbers. Due the exact binding behavior on .NET Framework, there were updates that didn't change assembly version.

They are re-unified for 5.0+ version. Some of the packages won't have 5.0+ version, and will just keep as-is.

@chucklu
Copy link
Author

chucklu commented Nov 23, 2020

I am not talking about the package version(the nuget version) and assembly version.
I am talking about the dependency's assembly version, System.Text.Json 4.7.2 required System.ValueTuple assembly version >=4.0.3.0(on nuget site) and actually it required System.ValueTuple =4.0.2.0. when you check the references of System.Text.Json 4.7.2 through dnSpy.

@huoyaoyuan
Copy link
Member

Well, I have no idea about this. The GAC version is 4.0.0.0.

@chucklu
Copy link
Author

chucklu commented Dec 4, 2020

@trylek @sandreenko @premun @akoeplinger Any update on this issue? Who is responsible for publish the NuGet package of https://www.nuget.org/packages/System.Text.Json/4.7.2?Or you can find the one who can help in this topic

@sandreenko
Copy link
Contributor

Hmm, I am not familiar with this, looking at the area expert for area-Meta I think @vargaz could help.

@akoeplinger
Copy link
Member

The area-meta label is the wrong one since the question is about System.Text.Json.

@ViktorHofer
Copy link
Member

cc @joperezr and @ericstj for the question.

@joperezr
Copy link
Member

joperezr commented Dec 10, 2020

I know why this is happening and the answer is kind of hard to understand without understanding how we build our product. The TL;DR version is that when referencing assemblies that are not live built any longer, we don't always build against the same assembly as the one we emit a package reference to. But here is a bit more info on why this happens:

  • Packages System.Text.Json 4.7.2 and System.ValueTuple 4.5.0 (and 4.4.0) ship out of different release branches (Json ships out of release/3.1 branch in corefx, while ValueTuple ships out of release/2.1 branch in corefx)
  • When building System.Text.Json assembly, we have to restore some version of System.ValueTuple as we don't live build it on release/3.1 any longer, and the version that we end up restoring is assembly version 4.0.2.0 which is the one that we build against and that is why System.Text.Json assembly depends on ValueTuple version 4.0.2.0.
  • We then build the System.Text.Json 4.7.2 package, but when building the package we will look at what is the baselined version (latest version we should reference) of ValueTuple, and according to https://github.com/dotnet/corefx/blob/aae1eab04d154a198c0e254ca003277f5079e484/pkg/Microsoft.Private.PackageBaseline/packageIndex.json#L6000 this should be 4.5.0, even though the assembly version inside that package is not the one that we built against.

That is where the inconsistency is happening, which in the end makes you need a binding redirect in net461 in order to be able to run. We have an issue #28503 that tracks the fix for all of the packages that are in this boat (that require binding redirects today in order to run well on net461) but haven't yet fixed them.

Is there a reason why you can't just let msbuild tooling suggest and apply the binding redirect for ValueTuple? It would be good to understand why is that workaround not working for you, so that we continue to make a case for fixing issue #28503 which today is not high in the priority list.

@chucklu
Copy link
Author

chucklu commented Dec 11, 2020

Is there a reason why you can't just let msbuild tooling suggest and apply the binding redirect for ValueTuple?

@joperezr Sorry, I don't know how to make masbuild tool suggest and apply the binding redirect.
Currently I install the packages from the nuget management window of visual studio, It will get the bindingRedirect automatically.
And I encounter a problem, the different referenced order, make the bindingRedirect behaved differently(It will lose a bindingRedirect in one case). You can check the detail here NuGet/Home#10302, I also mentioned the problem in previous comments in this issue.

@joperezr
Copy link
Member

Ok, yeah so I checked your example from the small repro you pointed out above and I can see what the problem is. The issue here is that you are still using packages.config which is the old way of using package references, the new way is called PackageReference. The big problem with packages.config is that transitive dependencies don't always flow to consuming projects, so in this particular case, your ConsoleApp1 is not seeing the reference that ClassLibrary1 has to System.Text.Json, so it never sees a conflict between the ValueTuple used, so it never suggests any binding redirect. The reason why you are seeing different behaviors when you do classLibrary first, or console app first, is because the binding redirects that get added in the old packages.config world happen at package installation time, so with console app first version it will miss the transitive dependency.

The new way of having references to NuGet packages is called PackageReference, and that will fix all of the problems you are hitting (I created a PR in your sample to show the changes ChuckTest/NuGet-10302#1). You can find instructions and more details on how to migrate to packagereference here which is essentially what I did in order to get this to work. The big advantage in this case with PackageReference is that a) the dependencies from the library do flow to the console app and more importantly b) the binding redirects get calculated by msbuild on the fly (while building your app) so that is why msbuild knows exactly what binding redirect is the correct one to add to your console app in order for things to work. Let me know if you have any questions.

@chucklu
Copy link
Author

chucklu commented Dec 15, 2020

@joperezr Thanks for your explanation, I just reviewed limitation of package reference.
Although we are using the visual studio 2019, I am not sure if it would be better to switch to package reference.

Limitations

NuGet PackageReference is not available in Visual Studio 2015 and earlier. Migrated projects can be opened only in Visual Studio 2017 and later.
Migration is not currently available for C++ and ASP.NET projects.
Some packages may not be fully compatible with PackageReference. For more information, see package compatibility issues.

In addition, there are some differences in how PackageReferences work compared to packages.config. For example - constraining upgrade versions is not supprted by PackageReference but add support for Floating Versions.

@joperezr
Copy link
Member

The general recommendation is that unless you depend on a package that only works with packages.config (for example if the package needs to run an msi at installation time) or you are using an old version of VS, you should upgrade to package reference. The feature has been redesigned in order to fix precisely the type of issues you were hitting on this issue, and that is were all the new investment and features are going since packages.config is now just legacy.

@chucklu
Copy link
Author

chucklu commented Dec 16, 2020

@joperezr Thanks, I will switch to packages reference.

@chucklu chucklu closed this as completed Dec 16, 2020
@chucklu
Copy link
Author

chucklu commented Dec 16, 2020

@joperezr I have filed another new issue about the library required another not required library. https://github.com/dotnet/sdk/issues/14985

@ghost ghost locked as resolved and limited conversation to collaborators Jan 15, 2021
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Jan 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

9 participants