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

[DCR]: NuGet allows version conflict resolution to cross major version boundaries potentially resulting in runtime errors #11457

Open
iconics-milan opened this issue Dec 16, 2021 · 2 comments
Labels
Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Type:DCR Design Change Request

Comments

@iconics-milan
Copy link

NuGet Product(s) Affected

Visual Studio Package Management UI

Current Behavior

Per semver, new major versions are allowed to introduce breaking changes in their API.
Yet NuGet will not inform of any potential problems and will download packages with a higher major version than specified by a dependency, unless the dependency explicitly specifies a maximum version in its PackageReference.

To demonstrate using purely MS packages:

  1. in Visual Studio (currently using 2019), create a new project using the template "ASP.NET Core Empty"
  2. in the wizard, check the box "Run on .NET Framework"
  3. (optionally) upgrade the .NET Framework and all nugets in the project to their latest available versions
  4. add the nuget Microsoft.Azure.Devices.Client 1.39.0 to the project
    When run, the app immediately crashes. Now continue:
  5. Remove Microsoft.Azure.Devices.Client again
  6. Add the nuget Microsoft.Azure.NotificationHubs 4.1.0
    Run again. It will not crash, but won't run either. After disabling "Just my code" in Visual Studio, you'll notice exceptions somewhere deep, preventing the app from running properly.

These are just two examples that are easy to reproduce, but we found more during our recent package upgrade. At this point, we are unable to tell confidently whether our application is fine, or if some new issue will appear when our app hits an uncommon execution path.

I know that NuGet would be able to warn about the potential incompatibilities if every PackageReference in every nuget was version locked to the current major version. But since MS does not do that, nobody will.

Desired Behavior

At least, NuGet should print a warning.
This means: if any PackageReference does not contain an explicit maximum version, then assume that it is only compatible with the current major version, as specified by the minimum version of the PackageReference. (If the package author did include a maximum version, then no change in behavior is needed)
There could also be an option to turn this warning into an error, so that developers cannot ignore this.

Ideally though, NuGet should be able to suggest ways how to resolve these issues. If applied to the case described previously, NuGet should suggest to downgrade Microsoft.Azure.Devices.Client or Microsoft.Azure.NotificationHubs, and also determine to which version. Debian's apt system can do something like this.

Additional Context

This isn't new, it is also known as the Diamond dependency problem and discussed here: https://codeblog.jonskeet.uk/2019/10/25/options-for-nets-versioning-issues/
That blog suggests other solutions to this problems, but printing a warning is I think the least NuGet can do, and would be sufficient in many cases.

People are being hit by this problem, and it seems that they simply try randomly upgrading/downgrading, until the application seems to works. For example:
#6673 (comment)
https://stackoverflow.com/questions/59402761/system-methodaccessexception-attempt-by-method-microsoft-extensions-logging-c
https://stackoverflow.com/questions/66416331/could-not-load-type-microsoft-extensions-primitives-inplacestringbuilder-from

@iconics-milan iconics-milan added Triage:Untriaged Type:DCR Design Change Request labels Dec 16, 2021
@nkolev92 nkolev92 added Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. and removed Triage:Untriaged labels Dec 16, 2021
@nkolev92 nkolev92 changed the title [DCR]: NuGet mixes incompatible packages together resulting in runtime errors [DCR]: NuGet allows version conflict resolution to cross major version boundaries potentially resulting in runtime errors Dec 17, 2021
@iconics-milan
Copy link
Author

iconics-milan commented Mar 18, 2022

Hello @heng-liu , it is disappointing to see that this was tagged as Icebox.
Are there other mechanisms implemented/planned that would help with the runtime errors, or are there other tools implemented/planned that would help detecting potential errors? Can the 'dotnet audit' help for example?

@shuebner
Copy link

shuebner commented Nov 21, 2022

Just wow.

First, the documentation recommends following SemVer, which includes incrementing the major version on breaking changes.
Second, nuget describes its own concept of versions with breaking changes in major releases.
Independently, it also independently claims to support SemVer.

But then during restore -- the moment in which this claimed support actually matters -- it does not care about any of it and will not even warn on major version differences between declared and resolved dependencies, as long as the major version is higher?

That makes no sense.

And there is not even a sane workaround.

Every package author would need to specify a maximum version to get sane behavior that is consistent with nuget's own rules.
Considering that exclusive upper bounds have been broken for years, authors would need to specify an inclusive upper bound of e. g. 1.999.999.999 just to be safe or try their luck with an exclusive upper bound of 2.0.0-0.

Ridiculous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:Restore Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Type:DCR Design Change Request
Projects
None yet
Development

No branches or pull requests

5 participants