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

cargo <1.60 failed to select a version of a dependency due to either ? (weak) or dep: (namespaced) features #10623

Open
epage opened this issue May 2, 2022 · 9 comments
Labels
A-dependency-resolution Area: dependency resolution and the resolver C-bug Category: bug

Comments

@epage
Copy link
Contributor

epage commented May 2, 2022

Problem

I did a cargo update and my CI broke when verifying MSRV.

error: failed to select a version for the requirement `libgit2-sys = "^0.13.3"`
candidate versions found which didn't match: 0.13.2+1.4.2, 0.13.1+1.4.2, 0.13.0+1.4.1, ...
location searched: crates.io index
required by package `git2 v0.14.3`
    ... which satisfies dependency `git2 = "^0.14"` (locked to 0.14.3) of package `repor v0.1.0 (/home/epage/src/personal/repro)`
  • cargo 1.60 works
  • cargo 1.59 doesn't work

The error being from the resolver and being cargo-version specific is why I am posting here rather than on the git2 repo.

Steps

$ git clone [email protected]:crate-ci/committed.git
cd committed
cargo +1.59.0 check
cargo +1.60.0 update
cargo +1.59.0 check

Possible Solution(s)

Track parse bad candidates when resolving and report them to the user if they are relevant

Notes

Old cargo cannot see crates in the registry that use weak or namespaced features (or similar new index data).

Version

cargo 1.60.0 (d1fd9fe 2022-03-01)
release: 1.60.0
commit-hash: d1fd9fe2c40a1a56af9132b5c92ab963ac7ae422
commit-date: 2022-03-01
host: x86_64-unknown-linux-gnu
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1m)
os: Linux Mint 20.3 (una) [64-bit]
@epage epage added the C-bug Category: bug label May 2, 2022
@epage
Copy link
Contributor Author

epage commented May 2, 2022

Looks like I missed that libgit2-sys uses a weak dependency (really hard to see that ?), see rust-lang/git2-rs@d8ee105

My guess is that this is expected behavior when loading this into old versions of cargo.

@epage epage closed this as completed May 2, 2022
@epage epage changed the title cargo <1.60 cannot see libgit2-sys 0.13.3 cargo <1.60 cannot see libgit2-sys 0.13.3, should error instead saying index is not understandable May 3, 2022
@epage epage reopened this May 3, 2022
@epage epage added the A-dependency-resolution Area: dependency resolution and the resolver label May 3, 2022
@epage
Copy link
Contributor Author

epage commented May 3, 2022

Considering the usability issue of this, re-opening for us providing better errors.

@epage
Copy link
Contributor Author

epage commented May 24, 2022

This is an issue with Cargo, but not one that's easy to fix. The issue comes because egui uses the new dep: syntax. Older Cargos don't know how to interpret this syntax and so skip parsing the version. Cargo could error on versions that they don't know how to interpret, but then you would be unable to build older versions using an older cargo just because newer versions used a newer feature. The correct solution here is for Cargo to keep track of a reason why any given package/version/feature cannot be selected and display it when relevant. Unfortunately the "when relevant" hard is a really hard feature. There are often hundreds of "reasons" that relate to the final resolution/error, only a handful of which are actually interesting to the user. We might be able to make incremental progress on these simple cases, but I am saving my effort for the multi year project of using pubgrub for our error messages.

From #10688

@epage epage changed the title cargo <1.60 cannot see libgit2-sys 0.13.3, should error instead saying index is not understandable cargo <1.60 failed to select a version of a dependency, should error instead saying index is not understandable May 24, 2022
@epage epage changed the title cargo <1.60 failed to select a version of a dependency, should error instead saying index is not understandable cargo <1.60 failed to select a version of a dependency May 24, 2022
@epage epage pinned this issue May 24, 2022
@ehuss
Copy link
Contributor

ehuss commented May 24, 2022

See also #7929 which discusses some related issues to the resolver not telling you why something wasn't picked.

I took a quick peek, and it seems like it will be quite difficult to thread the information through. There are many layers of abstractions between where the filtering happens and where the resolver fetches it. Also, the resolver API doesn't look like it is designed to funnel partial errors out of the RegistryQueryer (it is only capable of returning nothing or something, not in-between).

I'd love to see the error messages improved, but it looks like it could be a big undertaking.

@epage epage changed the title cargo <1.60 failed to select a version of a dependency cargo <1.60 failed to select a version of a dependency due to either ? (weak) or dep: (namespaced) features May 25, 2022
Ecordonnier added a commit to Ecordonnier/cargo-bitbake that referenced this issue Aug 23, 2022
Due to rust-lang/cargo#10623, cargo-bitbake
needs at least cargo 1.60 to be built.
@epage
Copy link
Contributor Author

epage commented Apr 25, 2023

Looking into this as rust-lang/rfcs#3416 could end up requiring a new index entry version.

The first step in this process is dealing with the Index Index

It looks like the version is all we need to extract for the IndexIndex, so we could reparse with a smaller set of RegistryPackage and put that into the IndexIndex without breaking things.

We then need to track the failed state through the rest of Sources up to the resolver and report the error. I'm tempted to say we should wrap Summary in an enum (MaybeSummary?) and also pass yanked entries up with it, opening the door for other error reporting and to make sure we actually filter out these unwanted entries when the time comes. For yanked items we do care about, we can change them from the yanked variant to the valid variant.

For me, the big question is how to report errors in the resolver. Could we make the resolver work off of MaybeSummarys and sort it so error cases and unwanted yanked appear "last" but if there is no other choice, we pick them? But we do backtracking, right? So would reporting these error cases break backtracking?

@Eh2406 I'm assuming you are going to say there is nothing we can do in the current resolver to report these errors even if we tunnel the information up. Is that right?

@Eh2406
Copy link
Contributor

Eh2406 commented Apr 26, 2023

I am not as pessimistic as you imagined me to be. I think there is a lot of value in the project you outlined. Providing this information to the resolver allows improvements or replacements of the resolver to use that information, without providing information it's hard to imagine where to start.

The current design of the resolver can only report on the last thing it tried when it ran out of options. Many problems are direct and simple. For simple cases even this will be a huge improvement. "egui = "^0.18" matches two versions 0.18.0, 0.18.1 but cargo could not parse them" is way better than what we had (cc #10688). Of course, adding things for it to try (that we know will not work) may also hide other issues. We may not see the underlying issue related to a deep dependency, because the shallower dependency can now try a version it didn't know how to parse. There are ways use heuristics to make this better, and I will happily support anyone who's willing to put in the work to do it incrementally. In order to get a good error message that includes both the problems with the deep dependency and the unavailable shallow dependency versions, will require a rewrite of the resolver.

epage added a commit to epage/cargo that referenced this issue Jun 6, 2023
Eh2406 pushed a commit to Eh2406/cargo that referenced this issue Sep 5, 2023
Eh2406 pushed a commit to Eh2406/cargo that referenced this issue Sep 7, 2023
Eh2406 pushed a commit to Eh2406/cargo that referenced this issue Sep 7, 2023
Eh2406 pushed a commit to Eh2406/cargo that referenced this issue Sep 8, 2023
Eh2406 pushed a commit to Eh2406/cargo that referenced this issue Sep 11, 2023
@polarathene
Copy link

Considering the usability issue of this, re-opening for us providing better errors.

Below examples are a bit verbose, but may provide some value? 🤷‍♂️ (I'm mostly documenting the experience)


The hashbrown 0.14.2 example differs slightly from your error output, in that:

  • It informs the user that it could not satisfy 0.14.2 semver, but found 0.14.2 version of the crate.
  • Actual failure was due to not being able to satisfy -Z msrv-policy.

I don't really use such old toolchain releases, but was contributing to a crate which did where I encountered this resolver confusion (later discovering -Z msrv-policy).

  • Initially I thought it was due to cargo resolving compatible crates like hashbrown by the rust-version and rolling back parent dependencies until satisfied (similar to -Z msrv-policy but not quite the same).
  • Later it was clarified on Discord to me that this was due to the 1.60.0 changes to Cargo.toml being adopted, making those crates invisible to older toolchains.

I'm not sure if this sort of incompatibility has also been introduced again in newer toolchain releases since too? (It was definitely unexpected 😅 )


Example - Resolving ordered-multimap = "0.4" with Rust 1.56.1

Resolving with rust-version via -Z msrv-policy / cargo-upgrade

If ahash had already adopted rust-version, this could have resolved correctly (EDIT: it will declare rust-version = "1.60.0" for future releases):

# `Cargo.toml` with single dep of `ordered-multimap = "0.4"` and `rust-version = "1.56.1"`:
# No failure encountered implies it was successful meeting the MSRV requirement (or at least on best effort)
$ cargo +nightly update -Z msrv-policy


# hashbrown respecting rust-version, resolves ahash semver (assuming compatibility with hashbrown rust-version)
# - 0.12.3 has rust-version 1.56.0, and `ahash = "0.7"` semver
# - 0.11.2 is similar, but published before `ahash 0.7.6` was released (which implicitly raised MSRV to 1.60)
$ cargo tree
app v0.1.0 (/app)
└── ordered-multimap v0.4.3
    ├── dlv-list v0.3.0
    └── hashbrown v0.12.3
        └── ahash v0.7.7
            ├── getrandom v0.2.10
            │   ├── cfg-if v1.0.0
            │   └── libc v0.2.149
            └── once_cell v1.17.2
            [build-dependencies]
            └── version_check v0.9.4


# Fails because `ahash >= 0.7.6` uses `Cargo.toml` syntax that can't be parsed by Rust <1.60.0:
$ cargo +1.56.1 check
    Updating crates.io index
error: failed to select a version for the requirement `ahash = "=0.7.7"`
candidate versions found which didn't match: 0.4.8, 0.4.1, 0.4.0, ...
location searched: crates.io index
required by package `hashbrown v0.12.3`
    ... which satisfies dependency `hashbrown = "=0.12.3"` of package `ordered-multimap v0.4.3`
    ... which satisfies dependency `ordered-multimap = "=0.4.3"` of package `app v0.1.0 (/app)`


# Or with cargo-edit installed, and a `ordered-multimap = "0.4.0" semver:
# cargo-upgrade uses same `-Z msrv-policy` approach AFAIK (but applies changes to `Cargo.toml`, raising min version)
$ cargo +1.56.1 upgrade
    Updating 'https://github.com/rust-lang/crates.io-index' index
    Checking app's dependencies
name             old req compatible latest new req note
====             ======= ========== ====== ======= ====
ordered-multimap 0.4.0   0.4.3      0.7.1  0.4.3   incompatible
   Upgrading recursive dependencies
note: Re-run with `--incompatible` to upgrade incompatible version requirements

Resolving with cargo +1.56.1 check (without a pre-existing Cargo.lock)

This time rust-version doesn't affect resolving, but the toolchain being below 1.60.0 notably affects the resolver (silently):

$ cargo +1.56.1 check
    Updating crates.io index
    Finished dev [unoptimized + debuginfo] target(s) in 1.65s

$ cargo tree
app v0.1.0 (/app)
└── ordered-multimap v0.4.0
    ├── dlv-list v0.2.3
    │   └── rand v0.8.5
    │       ├── libc v0.2.149
    │       ├── rand_chacha v0.3.1
    │       │   ├── ppv-lite86 v0.2.17
    │       │   └── rand_core v0.6.4
    │       │       └── getrandom v0.2.10
    │       │           ├── cfg-if v1.0.0
    │       │           └── libc v0.2.149
    │       └── rand_core v0.6.4 (*)
    └── hashbrown v0.9.1
        └── ahash v0.4.8
  • Resolves to ordered-multimap 0.4.0 instead, with hashbrown 0.9.1 satisfying the semver ahash ="0.4.4" which resolves to the ahash 0.4.8 release.

    Ignore these points regarding `once_cell` (unrelated)

    (EDIT: With default-features = false, so ignore the once_cell sub-points that follow as that depends upon the "compile-time-rng" feature installing const-random)

    • ahash 0.4.8 technically resolves to a MSRV of 1.60.0 due to an implicit dependency semver resolving to once_cell 1.18.0 (detected via cargo-msrv) which adopted the dep: feature. However since these Rust 1.60.0 changes prevent resolving that on earlier toolchains, 1.17.2 and below are compatible to resolve for 1.56.1 instead (without needing -Z msrv-policy).
    • The actual MSRV for ahash 0.4.8 is as low as Rust 1.56.0 (via -Z minimal-versions --package once_cell) - since the min semver bound for once_cell is set to 1.15 (which introduced edition = "2021" + adopted rust-version from that release onwards).
  • The semver range ordered-multimap = "0.4" could not resolve to newer releases of 0.4.x, due to those releases bumping minor version of hashbrown (which also bumped ahash minors that relied on the since yanked ahash releases, or releases with incompatible dep: syntax in Cargo.toml that requires Rust >=1.60.0).

  • ahash could publish a new release with an appropriate rust-version for a better error message, but that would be too late now as it'd resolve to ahash 0.7.7 as the release prior to declaring rust-version (if no release with rust-version declared is compatible).


Example - hashbrown 0.14.2 leveraging rust-version

Whereas if our Cargo.toml (with rust-version = "1.56.1") instead uses hashbrown = "0.14.2" (latest version, which has rust-version = "1.63.0"), the failure is caught when using -Z msrv-policy this time:

# Not a helpful failure message, `cargo check` is better at communicating when the MSRV of a crate is too high:
$ cargo +nightly update -Z msrv-policy
    Updating crates.io index
error: failed to select a version for the requirement `hashbrown = "^0.14"`
candidate versions found which didn't match: 0.14.2, 0.14.1, 0.14.0, ...
location searched: crates.io index
required by package `app v0.1.0 (/app)`
perhaps a crate was updated and forgotten to be re-vendored?


$ cargo tree
app v0.1.0 (/app)
└── hashbrown v0.14.2
    ├── ahash v0.8.6
    │   ├── cfg-if v1.0.0
    │   ├── once_cell v1.18.0
    │   └── zerocopy v0.7.20
    │   [build-dependencies]
    │   └── version_check v0.9.4
    └── allocator-api2 v0.2.16


# Better failure message (1.60.0 used to avoid `Cargo.toml` syntax incompatibility):
$ cargo +1.60.0 check
error: package `hashbrown v0.14.2` cannot be built because it requires rustc 1.63.0 or newer, while the currently active rustc version is 1.60.0

# But not if the chosen toolchain lacks the newer `Cargo.toml` syntax support (version doesn't appear to exist):
$ cargo +1.56.1 check
error: failed to select a version for the requirement `hashbrown = "=0.14.2"`
candidate versions found which didn't match: 0.13.2, 0.13.1, 0.12.3, ...
location searched: crates.io index
required by package `app v0.1.0 (/app)`

-Z msrv-policy not being stable, the error message there is perhaps less of a concern?

  • ❌ Failure was due to semver not covering a release compatible with the requested rust-version.
  • ❌ Error message confusingly lacks the context, stating it found 0.14.2 but that it didn't match the 0.14.2 semver requirement.

Example - rust-version is only reliable when constraints to support it are known

Collapsed as slightly off-topic

This is only referencing hashbrown as it was part of the prior hashbrown troubleshooting with understanding how to leverage rust-version. I'm sure the issue described below will be applicable to other crates and their dependency chains until a common rust-version baseline is adopted more broadly in the ecosystem (perhaps with edition = "2024").

Unfortunately, as rust-version appears to be more of a guide for the MSRV, it does not necessarily mean it'll be successful with the default features (or remain successful in future due to implicit dependency updates within the semver bound):

# Build with toolchain specified in hashbrown `rust-version`:
$ cargo +1.63.0 check
...
error[E0658]: use of unstable library feature 'core_c_str'
...
For more information about this error, try `rustc --explain E0658`.
error: could not compile `allocator-api2` due to 6 previous errors
warning: build failed, waiting for other jobs to finish...

Without tracking down the latest compatible version, the minimum version specified by hashbrown can at least be relied upon:

# Cargo.lock update attempt based on compile error:
$ cargo +nightly update --package allocator-api2 -Z minimal-versions
    Updating crates.io index
 Downgrading allocator-api2 v0.2.16 -> v0.2.9

# Success
$ cargo +1.63.0 check
    Checking hashbrown v0.14.2
    Checking app v0.1.0 (/app)
    Finished dev [unoptimized + debuginfo] target(s) in 0.36s

Or opt-out of the default allocator-api2 dependency / feature in Cargo.toml if viable (usually more inconvenient: #3126 ):

# Opt-out of default feature "allocator-api2":
hashbrown = { version = "0.14.2", default-features = false, features = ["ahash", "inline-more"] }

That probably would have been preventable with -Z msrv-policy too - if allocator-api2 0.2.10 had declared rust-version = "1.63.0" compatibility (prior to the 0.2.11 release, which should also have set rust-version = "1.64.0").

@epage
Copy link
Contributor Author

epage commented Nov 1, 2023

Bad error messages with -Zmsrv-policy are independent of this issue. #9930 is the issue for the MSRV resolver. The error messages in particular are a known issue and what have held us back from working to implement and stabilize this. We went ahead and created throwaway unstable-only solution for now and will need to do major re-works of the dependency resolver to improve the error messages.

@epage
Copy link
Contributor Author

epage commented Jan 31, 2024

Not quite the same error but a related problem with us showing an unrelated error that is rooted in MSRV can be found at rust-cli/env_logger#304. env_logger is relying on a feature change in 1.71 but in 1.70, the error focuses on the features and doesn't give any hint that the true problem is MSRV related.

I haven't created a separate issue for this yet as I'm still not sure what I'd be asking for moving forward (since we can't time travel to <1.71).

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 C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

5 participants
@ehuss @epage @Eh2406 @polarathene and others