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.
