Open epage opened 2 years ago
epage
commented
2 years ago Looks like I missed that libgit2-sys uses a weak dependency (really hard to see that ?), see https://github.com/rust-lang/git2-rs/commit/d8ee105a8f3ce4d5c57cd091b67943aab86b176a
My guess is that this is expected behavior when loading this into old versions of cargo.
epage
commented
2 years ago Considering the usability issue of this, re-opening for us providing better errors.
epage
commented
2 years ago This is an issue with Cargo, but not one that's easy to fix. The issue comes because
eguiuses the newdep: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.
ehuss
commented
2 years ago 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
commented
1 year ago 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
commented
1 year ago 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.
polarathene
commented
10 months ago 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:
0.14.2 semver, but found 0.14.2 version of the crate.-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).
hashbrown by the rust-version and rolling back parent dependencies until satisfied (similar to -Z msrv-policy but not quite the same).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 😅 )
ordered-multimap = "0.4" with Rust 1.56.1rust-version via -Z msrv-policy / cargo-upgradeIf 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 requirementscargo +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.8Resolves 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.
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).
hashbrown 0.14.2 leveraging rust-versionWhereas 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?
rust-version.0.14.2 but that it didn't match the 0.14.2 semver requirement.rust-version is only reliable when constraints to support it are knownepage
commented
10 months ago 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
commented
7 months ago 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).
Problem
I did a
cargo updateand my CI broke when verifying MSRV.The error being from the resolver and being cargo-version specific is why I am posting here rather than on the git2 repo.
Steps
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