feat: add package name and version to warning messages
### What does this PR try to resolve?
Hi, this PR Fixes#8018, where i add the package name and version on warning messages.
As this is my first contribution to the project, if anything is not in compliance in the PR let me know and I will be happy to correct it.
Do not call it "Downgrading" when difference is only build metadata
### What does this PR try to resolve?
When a `cargo update --precise` changes a dependency between 2 versions which differ only in build metadata, Cargo prints a log referring to it as "Updating" or "Downgrading" the dependency, depending on a comparison between the build metadatas.
This is usually not meaningful, given that build metadata is often stuff like git commit hashes, which are not meaningfully ordered.
```console
Updating crates.io index
Downgrading foo v0.0.1+43ef4fe -> v0.0.1+2c65d16
Updating bar v0.0.2+bc17664 -> v0.0.2+c144a98
```
~~This PR changes to the word "Switching" when the version major, minor, patch, and pre-release value are not being changed.~~
This PR uses the word "Updating" when the version major, minor, patch, and pre-release value are unchanged, regardless of whether the build metadata is going up or down.
### How should we test and review this PR?
- `cargo test`
- `cargo build --release`
- `/path/to/cargo/target/release/cargo add tonic_datastore_v1`
- `/path/to/cargo/target/release/cargo update -p tonic_datastore_v1 --precise 0.1.0+3562b6cb3`
- `/path/to/cargo/target/release/cargo update -p tonic_datastore_v1 --precise 0.1.0+ee9e8e4e6`
Before:
<img src="https://github.com/rust-lang/cargo/assets/1940490/93e377e7-928e-4cec-aff6-451166ef7c81" width="500">
~~After:~~
<img src="https://github.com/rust-lang/cargo/assets/1940490/bb71459e-469a-4e09-bb8a-4083f34bce79" width="500">
After:
<img src="https://github.com/rust-lang/cargo/assets/1940490/8804e2fe-d0de-4c9e-b463-a5742daf9446" width="500">
chore: Sort dependency tables
This will make `cargo add` put things in the right place.
Verified by doing `cargo add arcstr && git diff && git reset --hard`
fix(install): Suggest an alternative version on MSRV failure
### What does this PR try to resolve?
Moves users from a bad error message, suggesting `--locked` which won't do anything, to suggesting a version of a package to use instead.
The side benefit is errors get reported sooner
- Before downloading the `.crate`
- When installing multiple packages, before building the first
This comes at the cost of an extra `rustc` invocation.
### How should we test and review this PR?
Per-commit this builds it up, from tests to the final design.
### Additional information
This is also written in a way to align fairly well with how we'd likely implement #10903.
This improved error message will still be useful after that issue is resolved when the MSRV compatible version is outside of the version req.
rustdoc: remove the word "Version" from test cases
Needed for https://github.com/rust-lang/rust/pull/115948 to merge.
That PR gets rid of the word "Version" in rustdoc's HTML output, and it splits spaced versions on their first space, to fit in the tight horizontal spacing. This causes Cargo's test suite to fail, because it look for the word "Version", even though things are working as they should.
These tests work on both current nightly and on that pull request.
The next step would be to also automatically install an MSRV compatible
version if compatible with the version req (#10903).
This improved error message will still be useful if the MSRV compatible
version is outside of the version req.
I did this as the first step
- Helps people now, not needing to wait on `-Zmsrv-policy` to be stabilized
- Has fewer questions on how it should be done (or if it should be)
This will also report the error without having to download the `.crate`
first.
If installing multiple packages, this will also report it immediately,
rather than waiting for the other packages to be installed first.
This also offers us more flexibility in the error we report,
like suggesting more appropriate fixes.
refactor(core): Clean up deserialization of PackageId
This came as part of another change that I'm abandoning but I felt this made things cleaner, so I decided to keep it.
Add new package cache lock modes
The way locking worked before this PR is that only one cargo could write to the package cache at once (otherwise it could cause corruption). However, it allowed cargo's to read from the package cache while running a build under the assumption that writers are append-only and won't affect reading. This allows multiple builds to run concurrently, only blocking on the part where it is not possible to run concurrently (downloading to the cache).
This introduces a new package cache locking strategy to support the ability to safely modify existing cache entries while other cargos are potentially reading from the cache. It has different locking modes:
- `MutateExclusive` (new) — Held when cargo wants to modify existing cache entries (such as being introduced for garbage collection in #12634), and ensures only one cargo has access to the cache during that time.
- `DownloadExclusive` (renamed) — This is a more specialized name for the lock that was before this PR. A caller should acquire this when downloading into the cache and doing resolution. It ensures that only one cargo can append to the cache, but allows other cargos to concurrently read from the cache.
- `Shared` (new) — This is to preserve the old concurrent build behavior by allowing multiple concurrent cargos to hold this while a build is running when it is reading from the cache
**Reviewing suggestions:**
There are a few commits needed to help with testing which are first. The main commit has the following:
- `src/cargo/util/cache_lock.rs` is an abstraction around package cache locks, and is the heart of the change. It should have comments and notes which should guide what it is doing. The `CacheLocker` is stored in `Config` along with all our other global stuff.
- Every call to `config.acquire_package_cache_lock()` has been changed to explicitly state which lock mode it wants to lock the package cache in.
- `Context::compile` is the key point where the `Shared` lock is acquired, ensuring that no mutation is done while the cache is being read.
- `MutateExclusive` is not used in this PR, but is being added in preparation for #12634.
- The non-blocking `try_acquire_package_cache_lock` API is not used in this PR, but is being added in preparation for #12634 to allow automatic gc to skip running if another cargo is already running (to avoid unnecessary blocking).
- `src/cargo/util/flock.rs` has been updated with some code cleanup (removing unused stuff), adds support for non-blocking locks, and renames some functions to make their operation clearer.
- `tests/testsuite/cache_lock.rs` contains tests for all the different permutations of ways of acquiring locks.
This tries to align the `Filesystem` method names so they have the same
form, and also so they spell out exactly what they do. (An alternative
would be to have a builder, similar to `OpenOptions`, but that seems a
bit overkill here.)
This also extends the docs to try to make it clearer how this works.
This introduces a new `CacheLocker` which manages locks on the package
cache. Instead of either being "locked" or "not locked", the new locker
supports multiple modes:
- Shared lock: Cargo can read from the package sources, along with any
other cargos reading at the same time.
- Download exclusive lock: Only one cargo can perform downloads.
Download locks do not interfere with Shared locks, since it is
expected that downloading does not modify existing files (only adds
new ones).
- Mutate exclusive lock: Only one cargo can have this lock, and it also
prevents shared locks. This is so that the cargo can modify the
package cache (such as deleting files) without breaking concurrent
processes.
Set and verify all MSRVs in CI
### What does this PR try to resolve?
Allow us to advertise an MSRV for all public packages, unblocking #12432.
Packages are broken down into two MSRV policies
- Internal packages: `rust-version=stable`
- Public packages: `rust-version=stable-2`
To support this
- RenovateBot will automatically update all MSRVs
- CI will verify all packages build according to their MSRV
Since this takes the "single workspace" approach, the downside is that common dependencies are subject to each package's MSRV. We also can't rely on `-Zmsrv-policy` to help us generate a lockfile because it breaks down when trying to support multiple MSRVs in a workspace
### How should we test and review this PR?
Per commit
### Additional information
#12381 skipped setting some MSRVs because we weren't sure how to handle it. For `cargo-credential`, we needed to do something and did one-off verification (#12623). `cargo-hack` recently gained the ability to automatically select MSRVs for each package allowing us to scale this up to the entire workspace.
I don't know if we consciously chose an MSRV policy for `cargo-credential` but it looked like N-2 so that is what I stuck with and propagated out.
- Without an aggressive sliding MSRV, we discourage people from using newer features because they will feel like they need permission which means it needs to be justified
- Without an aggressive sliding MSRV, if the MSRV at one point in time works for someone, they tend to assume it will always work, leading to frustration at unmet expectations.
I switched the MSRV check to `cargo check` from `cargo test` because I didn't want to deal with the out-of-diskspace issues and `check` will catch 99% of MSRV issues.
Potential future improvements to `cargo-hack`
- Allow `--version-range ..stable` so we can verify all MSRVs that aren't stable which would bypass the diskspace issues and allow us to more easily use `cargo test` again
- Verify on a `cargo package` version of a crate (taiki-e/cargo-hack#216)
crates-io: Add doc comment for `NewCrate` struct
This PR adds a small doc comment to the `NewCrate` struct in the `crates-io` package, linking to https://doc.rust-lang.org/cargo/reference/registry-web-api.html#publish to give additional context.
I saw other links to https://doc.rust-lang.org in some of the doc comments, so I'm hoping this is correct vs. linking directly to the file with a relative path?
