-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[experiment] patch with patch files (re-resolve) #14055
Closed
Closed
Conversation
This file contains 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
edd3172
to
8015563
Compare
☔ The latest upstream changes (presumably #14026) made this pull request unmergeable. Please resolve the merge conflicts. |
8015563
to
f8cbe46
Compare
☔ The latest upstream changes (presumably #14234) made this pull request unmergeable. Please resolve the merge conflicts. |
Since `[patch]` section exists also in config, so have it inboth cargo-features and -Z flag.
`SourceKind::Patched` represents a source patched by local patch files.
One thing left out here is a consistent/stable hash for patches, The are absolute local paths that might introduces unnecessary changes in lockfile. To mitigate this, patches under the current workspace root will be relative.
Only after a few dependency resolution happen, can a `[patch]` entry with patch files know which version it is going to patch. Hence we need to "delay" the patch process until at least one resolution is done, and re-resolves the graph is any patch become applicable with the newly resolved dependency graph.
We've tracked patched depencencies in Cargo.lock with the `patched+` protocol. However, we don't really make use of it when rebuilding from a lockfile. This PR fixes that. You can see the test cases now won't update registry becaseu it starts resuing locked patch dependencies.
f8cbe46
to
6109ea8
Compare
☔ The latest upstream changes (presumably #13947) made this pull request unmergeable. Please resolve the merge conflicts. |
See #13779 (comment). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-configuration
Area: cargo config files and env vars
A-manifest
Area: Cargo.toml issues
A-registries
Area: registries
A-testing-cargo-itself
Area: cargo's tests
A-unstable
Area: nightly unstable support
A-workspaces
Area: workspaces
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
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.
What does this PR try to resolve?
This is an alternative of #13779.
Most parts of the design are the same. The differences are:
patch
entry no longer requires an exact version requirement, e.g.=1.2.3
.Instead, only after a few rounds of dependency resolutions can a patch entry know which package version it is going to patch.
patch
is able to match multiple versions, if applicable.package.version
field to a SemVer incompat version is allowed but kinda useless. The metadata of patched package, for example v2.0.0-to-v1.0.0, is different from the metadata of the real v1.0.0. Thus rustc things they are different versions.How should we test and review this PR?
This is built upon #13779 with two new commits.
While this works, I don't think the code change is in a good state to merge. The design (hacking on a new
SourceKind
) could be inherently wrong.The ugliest part is code that handles the reuse of existing patched in lockfile edd3172. There are rooms for cleanup but I don't believe it'll be significantly better.
Additional information
Doc hasn't got updated.