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

Closed
epage opened this issue May 2, 2022 · 9 comments · Fixed by #14927
Labels
A-dependency-resolution Area: dependency resolution and the resolver C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

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.
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).

@epage epage added the S-triage Status: This issue is waiting on initial triage. label Nov 20, 2024
github-merge-queue bot pushed a commit that referenced this issue Dec 11, 2024
…ge (#14897)

### What does this PR try to resolve?

Instead of saying no package found when there are hidden `Summary`s,
we'll instead say why the summary was hidden in the cases of
- Yanked packages
- Schema mismatch
- Offline packages?

The schema mismatch covers part of #10623. Whats remaining is when we
can't parse the `Summary` but can parse a subset (name, version, schema
version, optionally rust-version). That will be handled in a follow up.

### How should we test and review this PR?

This has a couple of risky areas
- Moving the filtering of `IndexSummary` variants from the Index to the
Registry Source. On inspection, there were other code paths but they
seemed innocuous to not filter, like asking for a hash of a summary
- Switching `PackageRegistry` to preserve the `IndexSummary` variant for
regular sources and overrides
- I did not switch patches to preserve `IndexSummary` as that was more
invasive and the benefits seemed more minor (normally people patch over
registry dependencies and not to them)

### Additional information
github-merge-queue bot pushed a commit that referenced this issue Dec 12, 2024
…14923)

### What does this PR try to resolve?

The user likely intended what they said and we should tell them why it
didn't work.
Rejected versions also imply alt versions below them.

Fixes #4260

In changing the errors from alt-versions to rejected-versions, I found
part of the old error message was better and changed the message
introduced in #14897 from matching alt-names to alt-versions.

This includes the test for #14921. The "fix" was done before this
because as it was part of the refactors in prep for this.

### How should we test and review this PR?

### Additional information

Next steps after this are to still produce an
`IndexSummary::Unsupported` even if there are deserialization errors, so
long as we know the name and version which should take care of #10623
and #14894.
epage added a commit to epage/cargo that referenced this issue Dec 12, 2024
While rust-lang#14897 reported packages with an unsupported index schema version,
that only worked if the changes in the schema version did not cause
errors in deserializing `IndexPackage` or in generating a `Summary`.

This extends that change by recoverying on error with a more lax,
incomplete parse of `IndexPackage` which should always generate a valid
`Summary`.

To help with a buggy Index, we also will report as many as we can.
This does not provide a way to report to users or log on cache reads if
the index entry is not at least `{"name": "<string>", "vers": "<semver>"}`.

As a side effect, the index cache will include more "invalid" index
entries.
That should be ok as we will ignore the invalid entry in the cache when
loading it.
Ignoring of invalid entries dates back to rust-lang#6880 when the index cache was
introduced.

Fixes rust-lang#10623
Fixes rust-lang#14894
epage added a commit to epage/cargo that referenced this issue Dec 12, 2024
While rust-lang#14897 reported packages with an unsupported index schema version,
that only worked if the changes in the schema version did not cause
errors in deserializing `IndexPackage` or in generating a `Summary`.

This extends that change by recoverying on error with a more lax,
incomplete parse of `IndexPackage` which should always generate a valid
`Summary`.

To help with a buggy Index, we also will report as many as we can.
This does not provide a way to report to users or log on cache reads if
the index entry is not at least `{"name": "<string>", "vers": "<semver>"}`.

As a side effect, the index cache will include more "invalid" index
entries.
That should be ok as we will ignore the invalid entry in the cache when
loading it.
Ignoring of invalid entries dates back to rust-lang#6880 when the index cache was
introduced.

Fixes rust-lang#10623
Fixes rust-lang#14894
github-merge-queue bot pushed a commit that referenced this issue Dec 13, 2024
### What does this PR try to resolve?

While #14897 reported packages with an unsupported index schema version,
that only worked if the changes in the schema version did not cause
errors in deserializing `IndexPackage` or in generating a `Summary`.

This extends that change by recoverying on error with a more lax,
incomplete parse of `IndexPackage` which should always generate a valid
`Summary`.

To help with a buggy Index, we also will report as many as we can.
This does not provide a way to report to users or log on cache reads if
the index entry is not at least `{"name": "<string>", "vers":
"<semver>"}`.

Fixes #10623
Fixes #14894

### How should we test and review this PR?

My biggest paranoia is some bad interaction with the index cache
including more "invalid" index entries.
That should be ok as we will ignore the invalid entry in the cache when
loading it.
Ignoring of invalid entries dates back to #6880 when the index cache was
introduced.

### Additional information
@weihanglo weihanglo unpinned this issue Dec 13, 2024
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 S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

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