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

[patch] with duplicate values causes panic #7264

Closed
ehuss opened this issue Aug 19, 2019 · 2 comments · Fixed by #7303
Closed

[patch] with duplicate values causes panic #7264

ehuss opened this issue Aug 19, 2019 · 2 comments · Fixed by #7303
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-patch Area: [patch] table override C-bug Category: bug

Comments

@ehuss
Copy link
Contributor

ehuss commented Aug 19, 2019

Problem
A [patch] table with multiple values using package can cause a panic.

[package]
name = "foo"
version = "0.1.0"
edition = "2018"

[dependencies]
bitflags = "1.0"

[patch.crates-io]
bitflags = {git="https://github.com/bitflags/bitflags.git"}
bitflags2 = {git="https://github.com/bitflags/bitflags.git", package="bitflags"}

Results in:

    Updating git repository `https://github.com/bitflags/bitflags.git`
thread 'main' panicked at 'assertion failed: remaining.next().is_none()', src/tools/cargo/src/cargo/core/registry.rs:648:13
stack backtrace:
   0: std::panicking::default_hook::{{closure}}
   1: std::panicking::default_hook
   2: std::panicking::rust_panic_with_hook
   3: std::panicking::begin_panic
   4: <core::iter::adapters::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
   5: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T,I>>::from_iter
   6: cargo::core::summary::Summary::map_dependencies
   7: cargo::core::registry::lock
   8: cargo::ops::resolve::resolve_with_previous
   9: cargo::ops::resolve::resolve_with_registry
  10: cargo::ops::resolve::resolve_ws_with_method
  11: cargo::ops::cargo_compile::compile_ws
  12: cargo::ops::cargo_compile::compile
  13: cargo::commands::build::exec
  14: cargo::cli::main
  15: cargo::main
  16: std::rt::lang_start::{{closure}}
  17: std::panicking::try::do_call
  18: __rust_maybe_catch_panic
  19: std::rt::lang_start_internal
  20: main

Notes

This appears to happen ever since package renaming was introduced up to latest nightly-2019-08-19.

Obviously you shouldn't have two patches to the same location, but it shouldn't panic.

@ehuss ehuss added C-bug Category: bug A-dependency-resolution Area: dependency resolution and the resolver A-patch Area: [patch] table override labels Aug 19, 2019
@Hoverbear
Copy link

Confirmed I hit this when testing #7263 :

log-0-3-9-use-static-module = { package = "log", git = "https://github.com/busyjay/log", branch = "use-static-module" }
log-0-4-6-revert-to-static = { package = "log", git = "https://github.com/busyjay/log", branch = "revert-to-static" }

@cbeck88
Copy link

cbeck88 commented Aug 22, 2019

Why shouldn't you have 2 patches to the same location? It seems like it could work -- maybe you need two versions of same code without feature unification?

I could always fork the repo a second time without changes, and then it would be expected to work, right?

bors added a commit that referenced this issue Aug 27, 2019
Fixes around multiple `[patch]` per crate

This commit fixes a few bugs in handling of `[patch]` where multiple
version of the same crate name have been patched in. Two sub-issues were
discovered when investigating #7264:

* The first issue is that the source of the assertion, the logic in
  `lock`, revealed a fundamental flaw in the logic. The `lock` function
  serves the purpose of applying a lock file to a dependency candidate
  and ensure that it stays within locked versions of dependencies, if
  they're previously found. The logic here to handle `[patch]`, however,
  happened a bit too late and was a bit too zealous to simply lock a
  dependency by name instead of accounting for the version as well.

  The updated logic is to move the locking of dependencies here to
  during the normal iteration over the list of dependencies. Adjacent to
  `matches_id` we check `matches_ignoring_source`. If the latter returns
  `true` then we double-check that the previous dependency is still in
  `[patch]`, and then we let it through. This means that patches with
  multiple versions should be correctly handled where edges drawn with
  `[patch]` are preserved.

* The second issue, after fixing this, was found where if the exact same
  version was listed in `[patch]` multiple times then we would
  continuously update the original source since one of the replacements
  gets lost along the way. This commit adds a first-class warning
  disallowing patches pointing to the exact same crate version, since we
  don't have a way to prioritize amongst them anyway.

Closes #7264
@bors bors closed this as completed in 803bf32 Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-patch Area: [patch] table override C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants