[PubGrub] Narrow down cases when pre-release decisions are marked invalid #7799
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xedin
requested review from
bnbarham,
MaxDesiatov,
jakepetroules and
francescomikulis
as code owners
MaxDesiatov
changed the title
|
@swift-ci test |
MaxDesiatov
reviewed
Jul 19, 2024
Sources/PackageGraph/Resolution/PubGrub/PubGrubDependencyResolver.swift
Outdated
Show resolved
Hide resolved
MaxDesiatov
reviewed
Jul 19, 2024
MaxDesiatov
reviewed
Jul 19, 2024
MaxDesiatov
reviewed
Jul 19, 2024
MaxDesiatov
approved these changes
Jul 19, 2024
…ecifier support prerelease versions
…rsions Prefer packages that don't allow pre-release versions to allow propagation logic it find dependencies that limit the range before making any decisions. This means that we'd always prefer release versions.
… correctly The intersection between release and pre-release ranges is allowed to produce a pre-release range which means that the solver is allowed to make a pre-release version decision even when some of the versions didn't allow pre-releases.
xedin
force-pushed
the
prerelease-intersections
branch
from
c9af997 to
ea543aa
Compare
|
@swift-ci please test |
|
@swift-ci please test Windows platform |
xedin
added a commit
to xedin/swift-package-manager
that referenced
this pull request
Jul 22, 2024
…alid (swiftlang#7799) Pre-release decisions are valid if the derived range they were inferred from supports pre-release versions. Currently, after deriving a pre-release assignment, the solver would assert in some circumstances depending on order in which the dependencies are specified in the manifest. This change makes it possible to infer pre-release verison of a package even when intermediate derivations did not support pre-release versions. For example one package could declare `swift-syntax` dependency as `508.0.1` ..< `601.0.0` and another could narrow it to `600.0.0-latest` ..< `601.0.0`. Since the most constrained range in such case supports pre-releases a decision to use the lastest pre-release version of `600.0.0` should be valid if there is no `600.0.0` release yet. It's important to note that on each path a decision for a package is always made based on the last undecided term's requirements which means - based on the current semantics - if such derivation doesn't support pre-release, even though intermediate ones did, the solver would select a released version. This PR is not aim to change that. There is a caveat which we cannot narrowly fix - if package depends on an exact pre-release version the inference is going to fail when there is another dependency to the same package that doesn't support pre-releases. For example: Package `A` depends on packages `B` and `C` as follows: `A` depends on `B` = `0.0.1-alpha` and `C` = `1.0.0` `B` version `0.0.1-alpha` has no dependencies `C` version `1.0.0` depends on `B` - `0.0.1` ..< `0.0.2` In this situation the solver would fail because `0.0.1-alpha` does not satisfy range - `0.0.1` ..< `0.0.2` inferred from `C`. - Delays version decisions for packages with pre-release ranges until there are no more packages without pre-release versions. This also means that if a previous decision narrows down the range to releases-only the package would be made a contender on the next step of the solver. - Relaxes the `isValidDecision` check for pre-release decisions but makes sure that the last derivation supports pre-release versions. In situations like swiftlang#7658 and swiftlang#7643 the solver would no longer assert and would produce a solution with a pre-release version for `swift-syntax`. (cherry picked from commit 43fd240)
bnbarham
pushed a commit
that referenced
this pull request
Jul 23, 2024
…alid (#7799) Pre-release decisions are valid if the derived range they were inferred from supports pre-release versions. Currently, after deriving a pre-release assignment, the solver would assert in some circumstances depending on order in which the dependencies are specified in the manifest. This change makes it possible to infer pre-release verison of a package even when intermediate derivations did not support pre-release versions. For example one package could declare `swift-syntax` dependency as `508.0.1` ..< `601.0.0` and another could narrow it to `600.0.0-latest` ..< `601.0.0`. Since the most constrained range in such case supports pre-releases a decision to use the lastest pre-release version of `600.0.0` should be valid if there is no `600.0.0` release yet. It's important to note that on each path a decision for a package is always made based on the last undecided term's requirements which means - based on the current semantics - if such derivation doesn't support pre-release, even though intermediate ones did, the solver would select a released version. This PR is not aim to change that. There is a caveat which we cannot narrowly fix - if package depends on an exact pre-release version the inference is going to fail when there is another dependency to the same package that doesn't support pre-releases. For example: Package `A` depends on packages `B` and `C` as follows: `A` depends on `B` = `0.0.1-alpha` and `C` = `1.0.0` `B` version `0.0.1-alpha` has no dependencies `C` version `1.0.0` depends on `B` - `0.0.1` ..< `0.0.2` In this situation the solver would fail because `0.0.1-alpha` does not satisfy range - `0.0.1` ..< `0.0.2` inferred from `C`. - Delays version decisions for packages with pre-release ranges until there are no more packages without pre-release versions. This also means that if a previous decision narrows down the range to releases-only the package would be made a contender on the next step of the solver. - Relaxes the `isValidDecision` check for pre-release decisions but makes sure that the last derivation supports pre-release versions. In situations like #7658 and #7643 the solver would no longer assert and would produce a solution with a pre-release version for `swift-syntax`. (cherry picked from commit 43fd240)
This was referenced Jul 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pre-release decisions are valid if the derived range they were inferred from supports pre-release versions.
Motivation:
Currently, after deriving a pre-release assignment, the solver would assert
in some circumstances depending on order in which the dependencies are
specified in the manifest.
This change makes it possible to infer pre-release verison of a package
even when intermediate derivations did not support pre-release versions.
For example one package could declare
swift-syntaxdependency as508.0.1..<601.0.0and another could narrow it to600.0.0-latest..<601.0.0.Since the most constrained range in such case supports pre-releases a decision
to use the lastest pre-release version of
600.0.0should be valid if thereis no
600.0.0release yet.It's important to note that on each path a decision for a package is always
made based on the last undecided term's requirements which means -
based on the current semantics - if such derivation doesn't support pre-release,
even though intermediate ones did, the solver would select a released version.
This PR is not aim to change that.
There is a caveat which we cannot narrowly fix - if package depends on an exact
pre-release version the inference is going to fail when there is another dependency
to the same package that doesn't support pre-releases. For example:
Package
Adepends on packagesBandCas follows:Adepends onB=0.0.1-alphaandC=1.0.0Bversion0.0.1-alphahas no dependenciesCversion1.0.0depends onB-0.0.1..<0.0.2In this situation the solver would fail because
0.0.1-alphadoes not satisfy range -0.0.1..<0.0.2inferred fromC.Modifications:
Delays version decisions for packages with pre-release ranges until there are no more packages without pre-release versions. This also means that if a previous decision narrows down the range to releases-only the package would be made a contender on the next step of the solver.
Relaxes the
isValidDecisioncheck for pre-release decisions but makes sure that the last derivation supports pre-release versions.Result:
In situations like #7658 and #7643 the solver would no longer assert and would produce a solution with a pre-release version for
swift-syntax.