patch can conflict on not activated packages #11770
Merged
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
|
r? @weihanglo (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-dependency-resolution
Eh2406
force-pushed
the
non-active-conflict
branch
from
b93f4c8
to
4369d8a
Compare
weihanglo
reviewed
Mar 1, 2023
| .map(|&c| (cx.is_active(c).expect("not currently active!?"), c)) | ||
| .max() | ||
| .unwrap(); | ||
| let (backtrack_critical_age, backtrack_critical_id) = shortcircuit_max( |
There was a problem hiding this comment.
Given we've changed this. Does this line still hold?
/// Panics if the input conflict is not all active in `cx`.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
bors
added
S-waiting-on-bors
This comment was marked as outdated.
This comment was marked as outdated.
bors
added
S-waiting-on-author
|
Sorry |
Eh2406
force-pushed
the
non-active-conflict
branch
from
4369d8a
to
2c712d5
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
weihanglo
added a commit
to weihanglo/rust
that referenced
this pull request
Mar 7, 2023
23 commits in 9880b408a3af50c08fab3dbf4aa2a972df71e951..c1334b059c6dcceab3c10c81413f79bb832c8d9d 2023-02-28 19:39:39 +0000 to 2023-03-07 19:21:50 +0000 - Add `CARGO_PKG_README` (rust-lang/cargo#11645) - path dependency: fix cargo-util version (rust-lang/cargo#11807) - Adding display of which target failed to compile (rust-lang/cargo#11636) - Fix `CARGO_CFG_` vars for configs defined both with and without value (rust-lang/cargo#11790) - Breaking endless loop on cyclic features in added dependency in cargo-add (rust-lang/cargo#11805) - Enhance the doc of timing report with graphs (rust-lang/cargo#11798) - Make `sparse` the default protocol for crates.io (rust-lang/cargo#11791) - Use sha2 to calculate SHA256 (rust-lang/cargo#11795) - gitoxide progress bar fixes (rust-lang/cargo#11800) - Check publish_to_alt_registry publish content (rust-lang/cargo#11799) - chore: fix missing files in autolabel trigger_files (rust-lang/cargo#11797) - chore: Update base64 (rust-lang/cargo#11796) - Fix some doc typos (rust-lang/cargo#11794) - chore(ci): Enforce cargo-deny in CI (rust-lang/cargo#11761) - Some cleanup for unstable docs (rust-lang/cargo#11793) - gitoxide integration: fetch (rust-lang/cargo#11448) - patch can conflict on not activated packages (rust-lang/cargo#11770) - fix(toml): Provide a way to show unused manifest keys for dependencies (rust-lang/cargo#11630) - Improve error for missing crate in --offline mode for sparse index (rust-lang/cargo#11783) - feat(resolver): `-Zdirect-minimal-versions` (rust-lang/cargo#11688) - feat: Use test name for dir when running tests (rust-lang/cargo#11738) - Jobserver cleanup (rust-lang/cargo#11764) - Fix help string for "--charset" option of "cargo tree" (rust-lang/cargo#11785) Note that some 3rd-party licensing allowed list changed due to the introducion of `gix` dependency
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 8, 2023
Update cargo 25 commits in 9880b408a3af50c08fab3dbf4aa2a972df71e951..7d3033d2e59383fd76193daf9423c3d141972a7d 2023-02-28 19:39:39 +0000 to 2023-03-08 17:05:08 +0000 - Revert "rust-lang/cargo#11738" - Use test name for dir when running tests (rust-lang/cargo#11812) - Update CHANGELOG for 1.68 backports (rust-lang/cargo#11810) - Add `CARGO_PKG_README` (rust-lang/cargo#11645) - path dependency: fix cargo-util version (rust-lang/cargo#11807) - Adding display of which target failed to compile (rust-lang/cargo#11636) - Fix `CARGO_CFG_` vars for configs defined both with and without value (rust-lang/cargo#11790) - Breaking endless loop on cyclic features in added dependency in cargo-add (rust-lang/cargo#11805) - Enhance the doc of timing report with graphs (rust-lang/cargo#11798) - Make `sparse` the default protocol for crates.io (rust-lang/cargo#11791) - Use sha2 to calculate SHA256 (rust-lang/cargo#11795) - gitoxide progress bar fixes (rust-lang/cargo#11800) - Check publish_to_alt_registry publish content (rust-lang/cargo#11799) - chore: fix missing files in autolabel trigger_files (rust-lang/cargo#11797) - chore: Update base64 (rust-lang/cargo#11796) - Fix some doc typos (rust-lang/cargo#11794) - chore(ci): Enforce cargo-deny in CI (rust-lang/cargo#11761) - Some cleanup for unstable docs (rust-lang/cargo#11793) - gitoxide integration: fetch (rust-lang/cargo#11448) - patch can conflict on not activated packages (rust-lang/cargo#11770) - fix(toml): Provide a way to show unused manifest keys for dependencies (rust-lang/cargo#11630) - Improve error for missing crate in --offline mode for sparse index (rust-lang/cargo#11783) - feat(resolver): `-Zdirect-minimal-versions` (rust-lang/cargo#11688) - feat: Use test name for dir when running tests (rust-lang/cargo#11738) - Jobserver cleanup (rust-lang/cargo#11764) - Fix help string for "--charset" option of "cargo tree" (rust-lang/cargo#11785) --- ~~This update is primarily for making rust-lang/cargo#11630 into 1.69~~ (will file a beta backport then). However, just look into the licenses and dependencies permitted list, it looks a bit unfortunate but inevitable I guess? r? `@ehuss` cc `@Muscraft`
saethlin
pushed a commit
to saethlin/miri
that referenced
this pull request
Mar 11, 2023
Update cargo 25 commits in 9880b408a3af50c08fab3dbf4aa2a972df71e951..7d3033d2e59383fd76193daf9423c3d141972a7d 2023-02-28 19:39:39 +0000 to 2023-03-08 17:05:08 +0000 - Revert "rust-lang/cargo#11738" - Use test name for dir when running tests (rust-lang/cargo#11812) - Update CHANGELOG for 1.68 backports (rust-lang/cargo#11810) - Add `CARGO_PKG_README` (rust-lang/cargo#11645) - path dependency: fix cargo-util version (rust-lang/cargo#11807) - Adding display of which target failed to compile (rust-lang/cargo#11636) - Fix `CARGO_CFG_` vars for configs defined both with and without value (rust-lang/cargo#11790) - Breaking endless loop on cyclic features in added dependency in cargo-add (rust-lang/cargo#11805) - Enhance the doc of timing report with graphs (rust-lang/cargo#11798) - Make `sparse` the default protocol for crates.io (rust-lang/cargo#11791) - Use sha2 to calculate SHA256 (rust-lang/cargo#11795) - gitoxide progress bar fixes (rust-lang/cargo#11800) - Check publish_to_alt_registry publish content (rust-lang/cargo#11799) - chore: fix missing files in autolabel trigger_files (rust-lang/cargo#11797) - chore: Update base64 (rust-lang/cargo#11796) - Fix some doc typos (rust-lang/cargo#11794) - chore(ci): Enforce cargo-deny in CI (rust-lang/cargo#11761) - Some cleanup for unstable docs (rust-lang/cargo#11793) - gitoxide integration: fetch (rust-lang/cargo#11448) - patch can conflict on not activated packages (rust-lang/cargo#11770) - fix(toml): Provide a way to show unused manifest keys for dependencies (rust-lang/cargo#11630) - Improve error for missing crate in --offline mode for sparse index (rust-lang/cargo#11783) - feat(resolver): `-Zdirect-minimal-versions` (rust-lang/cargo#11688) - feat: Use test name for dir when running tests (rust-lang/cargo#11738) - Jobserver cleanup (rust-lang/cargo#11764) - Fix help string for "--charset" option of "cargo tree" (rust-lang/cargo#11785) --- ~~This update is primarily for making rust-lang/cargo#11630 into 1.69~~ (will file a beta backport then). However, just look into the licenses and dependencies permitted list, it looks a bit unfortunate but inevitable I guess? r? `@ehuss` cc `@Muscraft`
This was referenced May 27, 2023
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.
What does this PR try to resolve?
In the resolver there is a data structure called a
conflicting_activations, which records all the reasons a "better" version was not picked for the package being resolved. Normally, these are packages that are currently active that for one reason or another block one of the other versions. Several optimizations assumed that this was always true, even going so far as to use.expect. This was a mistake because when there's apatchinvolved there can be conflicts on a version that is not selected. So the correct behavior is to fall back to skip the optimizations and try all versions when aconflicting_activationsare not active.How should we test and review this PR?
This adds two automated tests based on the reproductions in #7463 and #11336. So the test suite now covers this case. It can also be tested by reconstructing the repositories originally reported in those issues.
Additional information
It could be that in the long term the correct fix is to figure out how to support patch without having a conflicting activation that is not activated. But that would be a much bigger change. And for now this assumption is only used in optimizations, so this gets people unstuck.