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

Apply version ranges to nuspecs during fix-up command #3835

Merged

Conversation

baronfel
Copy link
Contributor

Per #2883, version ranges specified in paket.dependencies weren't patched into auto-generated nuspecs as part of the fix-nuspecs call. This PR attempts to add them in, in addition to a slight cleanup of the existing logic to make it easier to follow in my view.

The approach is to first determine what the nuspec file ranges should be for all known dependencies. This is done by taking the ranges specified in the dependencies file, which are optimistic, and modifying them with the 'resolved' information from the lock file. What this means depends on the type of range specified:

  • for greater than, minimums, specific versions and overrides, this means changing the semver used for that bound to be the locked version. This seems safe for every case.
  • for maximums, this means changing the range to a Between, because we want to ensure that consumers use at least the version that this package was built against, while not going higher than this package's own build requirements
  • similarly for less than, we want to ensure that the minimum bound is changed to the version that this package was built against
  • finally for Ranges, we change the lower bound to an inclusive lower bound on the locked version for similar reasons.

Once we have these ranges derived, we iterate through the dependencies in the nuspec as before, only taking action if we encounter a dependency that we know is a direct dependency.

Caveats:

  • this only tracks the main group of dependencies
    • we have the references file for this project, we could use that information to look for the range of the group that was actually referenced.
  • this does not take into account targetframework restrictions.

I don't have tests for this yet, but from manual testing on F# Formatting (after adding an upper bound myself to the dependencies file and manually calling fix-nuspecs on the generated nuspecs), this seems to work:

nuget FSharp.Compiler.Service >= 35.0 < 36.0 lowest_matching:true

results in a nuspec with the range

<dependency id="FSharp.Compiler.Service" version="[35.0.0,36.0.0)" exclude="Build,Analyzers" />

src/Paket.Core/PublicAPI.fs Outdated Show resolved Hide resolved
@baronfel
Copy link
Contributor Author

There's one failure on appveyor that's in an old-framework test, I'm not sure it's relevant here

@baronfel
Copy link
Contributor Author

@forki do you have opinions/pointers on where/how I can test this?

@forki
Copy link
Member

forki commented Apr 25, 2020

is this something that should be released in 5.x? In that case can you please retarget it against bugfix?

@baronfel baronfel force-pushed the 2883-fixup-version-ranges-in-dotnet-sdk branch from 8cf804a to bc81861 Compare April 25, 2020 19:13
@baronfel
Copy link
Contributor Author

Retargeted as requested 👍

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

Successfully merging this pull request may close these issues.

2 participants