Cache index files based on contents hash
Since #10507 Cargo has known how to read registry cached files whose index version starts with the hash of the file contents. Git makes it very cheap to determine the hash of a file. This PR switches cargo to start writing the new format.
Cargoes from before #10507 will not know how to read, and therefore overwrite, cached files written by Cargos after this PR.
Cargos after this PR can still read, and will consider up-to-date cached files written by all older Cargos.
As I'm writing this out I'm thinking that there may not be any point in writing a file that has both. An alternative implementation just writes the file contents hash. 🤔
fix: specifies the max length for crate name
Provides the maximum length on the crate name as is defined
in the [crate's validation source code from crates.io][1].
[1]: 3fc08ba57e/src/models/krate.rs (L74)
Provides the maximum length on the crate name as is defined
in the [crate's validation source code from crates.io][1].
[1]: 3fc08ba57e/src/models/krate.rs (L74)
refactor(cli): Lazy load config
This is trying to clarify `-C` support when it is implemented in #10952
Cargo currently has two initialization states for Config, `Config::default` (process start) and `config.configure` (after parsing args). The most help we provide for a developer touching this code is a giant `CAUTION` comment in one of the relevant functions.
Currently, #10952 adds another configuration state in the middle where the `current_dir` has been set.
The goal of this PR is to remove that third configuration state by
- Lazy loading `Config::default` so it can be called after parsing `-C`
- Allowing `-C` support to assert that the config isn't loaded yet to catch bugs with it
The hope is this will make the intent of the code clearer and reduce the chance for bugs.
In doing this, there are two intermediate refactorings
- Make help behave like other subcommands
- Before, we had hacks to intercept raw arguments and to intercept clap errors and assume what their intention was to be able to implement our help system.
- This flips it around and makes help like any other subcommand,
simplifying cargo initialization.
- We had to upgrade clap because this exposed a bug where `Command::print_help` wasn't respecting `disable_colored_help(true)`
- Delay fix's access to config
Personally, I also find both changes make the intent of the code clearer.
To review this, I recommend looking at the individual commits. As this is just refactors, this has no impact on testing.
My hope is to make it so we can lazy load the config. This makes it so
we only load the config for the fix proxy if needed.
I also feel like this better clarifies the intention of the code that we
are running in a special mode.
Before, we had hacks to intercept raw arguments and to intercept clap
errors and assume what their intention was to be able to implement our
help system.
This flips it around and makes help like any other subcommand,
simplifying cargo initialization.
chore: Don't show genned docs in ripgrep
The goal is to help new (and existing) users more quickly find the
appropriate files to edit (like in #11033). The main downside is for
someone trying to find output to verify what it looks like, a simple
search won't turn up results but there are other ways around that
(`--no-ignore`, `git status` after doing a man generation, etc).
The goal is to help new (and existing) users more quickly find the
appropriate files to edit (like in #11033). The main downside is for
someone trying to find output to verify what it looks like, a simple
search won't turn up results but there are other ways around that
(`--no-ignore`, `git status` after doing a man generation, etc).
Rework test error handling
This reworks how errors are handled when running tests and benchmarks. There were some cases where Cargo was eating the actual error and not displaying it. For example, if a test process fails to launch, it only displayed the `could not execute process` message, but didn't explain why it failed to execute. This fixes it to ensure that the full error chain is displayed.
This also tries to simplify how the errors are handled, and makes them more uniform across `test` and `bench`, and with doctests.
This also changes the `--no-fail-fast` behavior to report errors as they happen instead of grouped at the end (and prints a summary at the end). This helps to make it clearer when a nonstandard error happens. For example, before:
```
Running tests/t1.rs (target/debug/deps/t1-bb449dfa37379ba1)
running 1 test
Running tests/t2.rs (target/debug/deps/t2-1770ae8367bc97ce)
running 1 test
test bar ... FAILED
failures:
---- bar stdout ----
thread 'bar' panicked at 'y', tests/t2.rs:3:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
bar
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
error: test failed.
Caused by:
process didn't exit successfully: `/Users/eric/Temp/z12/target/debug/deps/t1-bb449dfa37379ba1` (signal: 11, SIGSEGV: invalid memory reference)
process didn't exit successfully: `/Users/eric/Temp/z12/target/debug/deps/t2-1770ae8367bc97ce` (exit status: 101)
```
and the changes to that are:
```diff
`@@` -1,6 +1,10 `@@`
Running tests/t1.rs (target/debug/deps/t1-bb449dfa37379ba1)
running 1 test
+error: test failed, to rerun pass `--test t1`
+
+Caused by:
+ process didn't exit successfully: `/Users/eric/Temp/z12/target/debug/deps/t1-bb449dfa37379ba1` (signal: 11, SIGSEGV: invalid memory reference)
Running tests/t2.rs (target/debug/deps/t2-1770ae8367bc97ce)
running 1 test
`@@` -18,8 +22,7 `@@`
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
-error: test failed.
-
-Caused by:
- process didn't exit successfully: `/Users/eric/Temp/z12/target/debug/deps/t1-bb449dfa37379ba1` (signal: 11, SIGSEGV: invalid memory reference)
- process didn't exit successfully: `/Users/eric/Temp/z12/target/debug/deps/t2-1770ae8367bc97ce` (exit status: 101)
+error: test failed, to rerun pass `--test t2`
+error: 2 targets failed:
+ `--test t1`
+ `--test t2`
```
In the first example, when it says `Running tests/t1.rs`, there is no error message displayed until after all the tests finish, and that error message is not associated with the original test. This also includes the "to rerun" hint with `--no-fail-fast`.
Very slight `cargo add` documentation improvements
As discussed in https://github.com/rust-lang/book/pull/3331, a quick explanation of the `Features` part of the message that gets printed to stdout when adding some dependency.
Consider the following example:
```
cargo add my-crate
Updating crates.io index
Adding my-crate v0.1.0 to dependencies.
Features:
+ foo
- bar
```
It was not clear to me what `+foo` and `-bar` meant until `@carols10cents'` kindly explained it to me. Hopefully the documentation now clarifies this.
TODO:
- [x] Run `./build-man.sh`
This refers to the `Features` part of the message that gets printed to
stderr after successfully adding some dependency, e.g.:
```
cargo add my-crate
Updating crates.io index
Adding my-crate v0.1.0 to dependencies.
Features:
+ foo
- bar
```
This tells us that the `foo` feature is enabled and `bar` is disabled.
Update compiling requirements.
This updates the requirements for building cargo itself. It adds a little more clarification on exactly what is needed, and what some of the options are.
This may be a little bit too much detail, as usually I suspect most users will just run `cargo build` and follow the error message instructions on what to install next.
Bump git2 to 0.15 and libgit2-sys to 0.14
This will allow cargo to avoid vendored builds of git2 in up-to-date
environments going forward, and brings in the [libgit2 1.4.4 CVE fix].
[libgit2 1.4.4 CVE fix]: https://github.com/libgit2/libgit2/releases/tag/v1.4.4
Apply GitHub fast path even for partial hashes
### What does this PR try to resolve?
As flagged in https://github.com/rust-lang/cargo/pull/10079#issuecomment-1170940132, it's not great to assume that git SHAs would always be 40-character hex strings. In the future they will be longer.
> Git is on a long-term trajectory to move to SHA256 hashes ([current status](https://lwn.net/SubscriberLink/898522/f267d0e9b4fe9983/)). I suppose when that becomes available/the default it's possible for a 40-digit hex-encoded hash not to be the full hash. Will this fail for that case?
The implementation from #10079 fails in that situation because it turns dependencies of the form `{ git = "…", rev = "[…40 hex…]" }` into fetches with a refspec `+[…40 hex…]:refs/commit/[…40 hex…]`. That only works if the 40 hex digits are the *full* long hash of your commit. If it's really a prefix ("short hash") of a 64-hex-digit SHA-256 commit hash, you'd get a failure that resembles:
```console
error: failed to get `dependency` as a dependency of package `repro v0.0.0`
Caused by:
failed to load source for dependency `dependency`
Caused by:
Unable to update https://github.com/rust-lang/dependency?rev=b30694b4d9b29141298870b7993e9aee10940524
Caused by:
revspec 'b30694b4d9b29141298870b7993e9aee10940524' not found; class=Reference (4); code=NotFound (-3)
```
This PR updates the implementation so that Cargo will curl GitHub to get a resolved long commit hash *even if* the `rev` specified for the git dependency in Cargo.toml already looks like a SHA-1 long hash.
### Performance considerations
⛔ This reverses a (questionable, negligible) benefit of #10079 of skipping the curl when `rev` is a long hash and is not already present in the local clone. These curls take 200-250ms on my machine.
🟰 We retain the much larger benefit of #10079 which comes from being able to precisely fetch a single `rev`, instead of fetching all branches and tags in the upstream repo and hoping to find the rev somewhere in there. This accounts for the entire performance difference explained in the summary of that PR.
🟰 We still skip the curl when `rev` is a **long hash** of a commit that is already previously fetched.
🥳 After this PR, we also curl and hit fast path when `rev` is a **short hash** of some upstream commit. For example `{ git = "https://github.com/rust-lang/cargo", rev = "b30694b4d9" }` would previously have done the download-all-branches-and-tags codepath because `b30694b4d9` is not a long hash. After this PR, the curl to GitHub informs us that `b30694b4d9` resolves to the long hash `b30694b4d9b29141298870b7993e9aee10940524`, and we download just that commit instead of all-branches-and-tags.
### How should we test and review this PR?
I tested with the following dependency specs, using `/path/to/target/release/cargo generate-lockfile`.
```toml
# Before: slow path (fetch all branches and tags; 70K git objects)
# After: fast path (20K git objects)
cargo = { git = "https://github.com/rust-lang/cargo", rev = "b30694b4d9b2914129" }
```
```toml
# Before and after: fast path
cargo = { git = "https://github.com/rust-lang/cargo", rev = "b30694b4d9b29141298870b7993e9aee10940524" }
```
```toml
# Before and after: fast path
cargo = { git = "https://github.com/rust-lang/cargo", rev = "refs/heads/rust-1.14.0" }
```
```toml
# Before and after: same error "revspec 'rust-1.14.0' not found"
# You are supposed to use `branch = "rust-1.14.0"`, this is not considered a `rev`
cargo = { git = "https://github.com/rust-lang/cargo", rev = "rust-1.14.0" }
```
I made sure these all work both with and without `rm -rf ~/.cargo/git/db/cargo-* ~/.cargo/git/checkouts/cargo-*` in between each cargo invocation.
FYI `@djc`
Update non-ASCII crate name warning message
This PR fixes an outdated warning when initializing crates sometimes.
### What does this PR try to resolve?
Per [a Zulip convo](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Non-ASCII.20crate.20name.20status/near/294876491) on the topic, non-ASCII crate names are no longer allowed on any toolchain since https://github.com/rust-lang/rust/pull/73305, during the `non_ascii_idents` feature's development. Cargo however tells the user that they are accepted on Nightly. Rust and Cargo should agree on this point to avoid future confusion.
### How should we test and review this PR?
This should be covered by the existing test that was changed but if desired its easy to test with a checkout:
```
Running `/Users/fox/x/Forks/cargo/target/release/cargo init 'ああああ'`
warning: the name `ああああ` contains non-ASCII characters
Non-ASCII crate names are not supported by Rust.
```
Add more tests for aggressive or precise update
### What does this PR try to resolve?
When I was working on https://github.com/rust-lang/cargo/pull/10988, I found there is no test for the aggressive update. So I added it.
Also, I added a test for precise update to make sure it won't force update the deps of SPEC.
### How should we test and review this PR?
Unit tests.
Walkdir's [`filter_entry()`][1] won't call the predicate if the entry
is essentially an `Err` from its underyling `IntoIter`. That means
Cargo hasn't had a chance to call `filter` on an entry that should be
excluded but eventually return an `Err` and cause the loop to stop.
For instance, a broken symlink which should bee excluded by `filter`
will generate an error since `filter` closure is not called with it.
The solution is calling `filter` if an error occurs with a path
(because it has yet been called with that path).
If it's exactly excluded, ignore the error.
[1]: abf3a15887/src/lib.rs (L1042-L1058)
Improve error message for wrong target names
### What does this PR try to resolve?
Fixes https://github.com/rust-lang/cargo/issues/10982
We can print all available targets for users.
### How should we test and review this PR?
- [x] unit test
