Avoid writing CACHEDIR.TAG if it already exists
Cargo currently unconditionally writes `CACHEDIR.TAG` files even if they already exist.
This practice causes problems for build systems that disallow multiple writes to the same file.
To implement rust-lang/rfcs#3516, we need to decouple the resolver's
behavior from the unstable flag. Since the code path is now dead, I
went ahead and removed it.
review and remove ignored tests in rustfix
### What does this PR try to resolve?
review ignored tests in rustfix crate per #13034.
### How should we test and review this PR?
CI testing
### Additional information
* Removed unproductive test in `parse_and_replace`
* un-ignore proptests, and reduce runtime from ~2s to ~<.25s
Migrate rustfix to the cargo repo
This migrates the `rustfix` crate from https://github.com/rust-lang/rustfix/ to the cargo repo. The cargo team has been responsible for the client-side of `cargo fix`, and it can make it easier to maintain with all our tooling and tracking here. This crate is used by some external parties (like the compiler), so it will need to be maintained like an "ecosystem" package, but hopefully there shouldn't be any outside requirements (I haven't seen any in several years).
After merging, I'll follow up with some things to address in the future, such as:
- Migrating issues from the other repo.
- Opening new issues for some cleanup tasks, such as adding documentation, fixing the `#[ignore]` annotations, fixing testing on windows, maybe migrating the test code to use different dependencies, various code cleanup.
- Archiving the repo.
This fixes an issue where `--quiet` doesn't work with commands that have
subcommands. This is because `config_configure` only looks at the global
and top-level subcommand, and not deeper subcommands. The issue was that
`--quiet` was not defined as a global flag. This was changed in
https://github.com/rust-lang/cargo/pull/6358 in order to give a better
help message for `cargo test --quiet`. I don't remember if clap just
didn't support overriding at the time, or if we just didn't know how it
worked. Anyways, it seems to work to override it now, so I think it
should be fine to mark it as global.
This should bring in `--quiet` more in-line with how `--verbose` works.
This means that `--quiet` is now accepted with `cargo report`,
`cargo help`, and `cargo config`.
This also fixes `--quiet` with `cargo clean gc`.
This should also help with supporting `--quiet` with the new `cargo
owner` subcommands being added in
https://github.com/rust-lang/cargo/pull/11879.
Fixes#12957
This adds a garbage collector which will remove old files from cargo's
global cache.
A general overview of the changes here:
- `cargo::core::global_cache_tracker` contains the `GlobalCacheTracker`
which handles the interface to a sqlite database which stores
timestamps of the last time a file was used.
- `DeferredGlobalLastUse` is a type that implements an optimization for
collecting last-use timestamps so that they can be flushed to disk all
at once.
- `cargo::core::gc` contains the `Gc` type which is the interface for
performing garbage collection. It coordinates with the
`GlobalCacheTracker` for determining what to delete.
- Garbage collection can either be automatic or manual. The automatic
garbage collection supports some config options for defining when
it runs and how much it deletes.
- Manual garbage collection can be performed via options to `cargo
clean`.
- `cargo clean` uses the new package cache locking system to coordinate
access to the package cache to prevent interference with other cargo
commands running concurrently.
This adds a very primitive `du` function for determining the disk usage
in a directory. This should probably be improved or replaced at some
point in the future, this is just a start for supporting tracking sizes
of cache data.
For `cargo install` we'll now show a more specific parse error for
semver, much like other parts of cargo.
This came out of my work on #12801. I was looking at what might be
appropriate to put in a `cargo-util-semver` crate and realized we have
the `ToSemver` trait that exists but doesn't do much, so I dropped it.
I've wanted something like this myself. I dislike using `--open`
because I tend to move up to re-run my `cargo doc` run but then have to
edit it to remove `--open`.
Also makes it annoying when opening docs when `cargo doc` is wrapped by
a tool like `make`.
This was previously attempted in #5592:
- Unlike the request in #5562, this aligns with #5592 in always printing
rather than using a flag as this seems generally useful
- Unlike #5592, this prints as an alternative to "Opening" to keep
things light
- Unlike #5592, this prints afterwards as the link is only valid then
Fixes#5562
add detailed message when target folder path is invalid
# What does this PR try to resolve?
close https://github.com/rust-lang/cargo/issues/12789
add more detailed message when target folder path is invalid
# How should we test and review this PR?
Before this PR, if the target folder refer to broken symbolic link like /a/b/c, then run cargo build, the output is:
```
error: Not a directory (os error 20)
```
the detailed error message is missing.
This PR will add the error context for it, the finall output will be
```
cargo build
error: failed to create directory `/root/workspace/playground/target`
Caused by:
Not a directory (os error 20)
```
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 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.
fix bug: corruption when cargo killed while writing
### What does this PR try to resolve?
fix #11386, superseding #12362
### How should we test and review this PR?
Added unit test showing basic equivalency to existing `write(path, content)`. Full test suite should exercise write.
Added tests for cargo add and remove. These are timing tests, so take a bit of time to run. 5-10s each. They may not fail every time, but do so regularly. Making the change to these two writes seems to prevent me from failing these tests at all.
### Additional information
This uses tempfile::persist which was an existing dependency. atomicwrites crate, an alternative option for this fix, indicates `tempfile::persist` is the same thing. Since we already use tempfile as a dep, I stuck with that.
Errors like this aren't too helpful
```
error: stderr did not match:
1 1 error: failed to parse manifest at `[..]`2 2 3 3 Caused by:4 4 TOML parse error at line 3, column 275 5 |6 6 3 | version = 17
7 | ^8 - invalid type: integer `1`, expected SemVer version 8 + invalid type: integer `1`, expected a string or workspace
```
This was broken in #12581
Add wrappers around std::fs::metadata
This adds wrappers around `std::fs::metadata` and `std::fs::symlink_metadata` which provide better error messages indicating the path that caused the error. This just helps clean up some duplicated code, and is also going to be used to assist with some code changes in #12634.
Add with_stdout_unordered.
This adds the `with_stdout_unordered` method to cargo's test system so that tests can use it to check stdout but ignoring the order of lines. Nothing in this PR actually uses this method, but it is added to support #12634. I also expect it could potentially be useful in other cases in the future.
This adds wrappers around std::fs::metadata and
std::fs::symlink_metadata which provide better error messages indicating
the path that caused the error.
use split_once for cleaner code
### What does this PR try to resolve?
Search the code base for `.splitn(2` and replace with `.split_once` where it was clearer. I don't think any of them matter in practice.
### How should we test and review this PR?
This was an internal re-factor, and the tests still pass.
The two methods have subtly different semantics, so please review carefully.
refactor: Pull out cargo-add MSRV code for reuse
### What does this PR try to resolve?
#12078 added MSRV code in `cargo add`. Our assumption when writing it is that we'd need to generalize the code before reusing it in other places, like `cargo install`. This PR focused purely on that refactor because I'm hopeful it will be useful for other work I'm doing. Despite not having a user for this yet, I think the `cargo install` case is inevitable and I feel this does a bit to clean up MSRV related code by using a more specific type everywhere.
### How should we test and review this PR?
Each commit gradually progresses things along
This changes logged messages from
```
2023-08-23T01:01:59.922018Z DEBUG cargo::core::compiler::fingerprint: filesystem up-to-date "/home/epage/src/personal/dump"
```
To
```
0.041729583s DEBUG cargo::core::compiler::fingerprint: filesystem up-to-date "/home/epage/src/personal/dump"
```
Benefits
- Less horizontal space taken up in boilerplate
- Easier to compare within a run
Downsides
- Harder to correlate with other processes, like with crates.io server
operations
This gives us up to 4 digits for seconds which should be sufficient for
cargo build times.
We could make this more compact by dropping the digits of precision from
9 to 6 but that would require a custom Timer which might be a paint to
keep in sync between packages.
versions and paths of a workspace members between the original and
a checked-out workspace are different, and shouldn't be included in
hash keys when querying packages.
ci: rewrite bump check and respect semver
### What does this PR try to resolve?
This ports `ci/validate-version-bump.sh` and #12200 to the new xtask `bump-check`. It also adds the ability to set the correct baseline revision when checking version bump. That is, it fixes#12347.
In addition, this integrates the community tool `cargo-semver-checks`. SemVer violation can now be detected earlier.
### How should we test and review this PR?
This PR extracts the registry query part from `xtask-unpublished` and removes other pars. I don't feel like we need it in the short term.
For how it works, please the check doc comment in each function.
One concern is that this check is still a required job for bors. I believe `@obi1kenobi` is quite responsive and willing to help if there is something wrong. So, waiting for an upstream fix won't be a problem for cargo. Also as a good citizen in the community, we can always contribute back.
(Take it easy `@obi1kenobi,` don't be stressed out 🙂)
<!-- homu-ignore:end -->
fix: AIX searches dynamic libraries in `LIBPATH`.
### What does this PR try to resolve?
On IBM AIX machines people have encountered issues in `compiletest` and rustc's bootstrap builder. They haven't encountered any in cargo. This PR is made for avoiding potential failures in the future in cargo.
It's documented in <https://www.ibm.com/support/pages/libpath-environment-variables-aix-platforms>:
> The `LIBPATH` environment variable tells AIX applications where to find shared libraries when located in a different directories than those specified in the header section of the executable.
See also the counterpart in <https://github.com/rust-lang/rust/pull/109526>
### How to verify and test this in Cargo's CI?
This is indeed an issue. At IBM people are maintaining a buildbot since GitHub Action doesn't support AIX yet.
ci: check if any version bump needed for member crates
### What does this PR try to resolve?
Check if a crate requires a version bump in CI.
Part of #12033
### How should we test and review this PR?
Demo action result: <https://github.com/weihanglo/cargo/actions/runs/4941952784/jobs/8835049007>
### Additional information
This doesn't devalue #12089. I love the changelog detection, saving maintainers' times a lot ( I am the one writing changelogs for each release for now). However, the amount of code there is too excessive to me. I think someday when we are going to integrate [cargo-release](https://crates.io/crates/cargo-release) and/or [ehuss/cargo-new-release](https://github.com/ehuss/cargo-new-release), we can remove this script perhaps entirely :)
I tried to document the script a bit hard. Hope it won't get worse over time.
These tree packages are considered to be published something.
To reduce the logic of xtask-unpubilshed, let's flag them false for now.
We can always set it `true` when needed.
feat: Add `-Zmsrv-policy` feature flag
### What does this PR try to resolve?
Nothing noticeable....
The intent is to unblock experiments with different compatible MSRV policies like
- #9930
- #10653
- #10903
While I normally don't like PRs that do nothing on their own, this at least allows any one of those efforts to move forward with different people without juggling these base commits for whoever is first to include in their PR
While there isn't an RFC for this yet, this is intended to allow us to experiment to get a better idea of what we should put in an RFC. In some cases, we first do an eRFC for this but I assumed this wouldn't be needed in this case as this builds on rust-lang/rfcs#2495 and, I'm assuming, will be more surgical in nature
### How should we test and review this PR?
The `Summary` changes are largely untested as they will be mostly tested through the future work that builds on this PR. However, I wasn't too concerned about that because the code is relatively trivial.
### Additional information
I chose the name `msrv-policy` to distinguish this unstable feature from `rust-version`. Though those appear in different places (`Cargo.toml` vs `-Z`), I can see them being confusing which was especially apparent when editing `unstable.md` which has an anchor for `rust-version`.
chore: Mark unpublished crates as such
This is a follow up to #12039.
This makes it easier for tools to report less irrelevant information.
I did both `publish = false` and `version = "0.0.0"` to help draw attention to the fact that these crates are internal (inspired by a matklad post).
I left `cargo-test-macro` and `cargo-test-support` in for my own personal bias of one day wanting to see those crates published...
The only one removed that had previously been published was `mdman` but seeing as that was a `0.0.0`, I'm assuming that was a mistake or just reserving the name.
Before:
```console
$ cargo unpublished
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
mdman 0.0.0 0.1.0
resolver-tests - 0.1.0
cargo 0.70.1 0.72.0
semver-check - 0.1.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
benchsuite - 0.1.0
```
After:
```console
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
cargo 0.70.1 0.72.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
```
chore(xtask): Add `cargo xtask unpublished`
### What does this PR try to resolve?
This tries to make it easy to see what existing versions have not been published. A future version of this could post to a PR what the current delta in version numbers for touched crates so reviewer have more context when deciding if they should ask for a crate version to be bumped
```console
$ cargo unpublished
Finished dev [unoptimized + debuginfo] target(s) in 0.12s
Running `/home/epage/src/personal/cargo/target/debug/xtask unpublished`
Updating crates.io index
name published current
==== ========= =======
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
mdman 0.0.0 0.1.0
resolver-tests - 0.1.0
cargo 0.70.0 0.71.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
benchsuite - 0.1.0
```
Room for improvement
- Aligning the start of each column
- Filtering the list by a commit range
- Adding this to an action to post to a review
- Maybe sorting the output
- Marking some our crates as `package.publish = false`, like benchsuite and resolver-tests
### How should we test and review this PR?
This is broken down commit by commit for easier seeing of the building blocks for our first xtask
This is a follow up to #12039.
This makes it easier for tools to report less irrelevant information.
I did both `publish = false` and `version = "0.0.0"` to help draw
attention to the fact that these crates are internal (inspired by a
matklad post).
I left `cargo-test-macro` and `cargo-test-support` in for my own
personal bias of one day wanting to see those crates published...
The only one removed that had previously been published was `mdman` but
seeing as that was a `0.0.0`, I'm assuming that was a mistake or just
reserving the name.
Before:
```console
$ cargo unpublished
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
mdman 0.0.0 0.1.0
resolver-tests - 0.1.0
cargo 0.70.1 0.72.0
semver-check - 0.1.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
benchsuite - 0.1.0
```
After:
```console
name published current
==== ========= =======
cargo-platform 0.1.2 0.1.3
cargo-test-macro - 0.1.0
cargo-test-support - 0.1.0
cargo-util 0.2.3 0.2.4
crates-io 0.36.0 0.36.1
cargo 0.70.1 0.72.0
cargo-credential 0.1.0 0.2.0
cargo-credential-1password 0.1.0 0.2.0
cargo-credential-gnome-secret 0.1.0 0.2.0
cargo-credential-macos-keychain 0.1.0 0.2.0
cargo-credential-wincred 0.1.0 0.2.0
```
crates.io reads rust-version from the tarball directly, but we can include it in
the publish request for the sake of consistency for third-party registries.
This will allow running an xtask without requiring building the world.
In most cases, a user will already have been building cargo but not in
CI.
The packages keep an `xtask-` prefix to help raise awareness of them but
exposed as `cargo <suffix>` to avoid having a direction proxy to wrap
`cargo run -p xtask-<suffix>` as `cargo xtask <suffix>`.
Update windows-sys
This updates the windows-sys dependency from 0.45 to 0.48. This shouldn't add or remove any duplicate dependencies (since there are other dependencies still using 0.45 and 0.42). The intent is to move it along the direction towards unifying in the future (though it seems like a moving target that will be difficult to ever hit).
This also bumps the home crate version. I think it should be OK to make the migration from winapi to windows-sys a patch version, though there seems to be some issues with the way windows-sys works that could introduce some build-time problems in some situations (such as those encountered in https://github.com/rust-lang/rust/pull/108665 and https://github.com/rust-lang/rust/pull/106610). However, I don't expect too much of an issue.
Ed Page
bors
hi-rustin
Guillaume Dallenne
Chris Denton
Weihang Lo
Tobias Bieniek
Arlo Siemsen
renovate[bot]
Eric Huss
Tom Scanlan
Tom Scanlan
David Tolnay
Jacob Finkelman
Lin Yihai
cui fliter
Deadbeef
trevyn
WANG Rui
QiangHeisenberg
Scott Schafer
Utkarsh Gupta
cassaundra