Add a test for regressions in selecting the correct workspace root
This adds a test to check for regressions in selecting the correct workspace when there are nested workspaces.
#10846 solved a problem with nested workspace resolution that was caused by #10776. `@ehuss` [suggested](https://github.com/rust-lang/cargo/pull/10846#issuecomment-1183754728) that a test should be added to ensure that this issue does not pop up again.
I ensured that this worked by testing against commit before #10846. Sporadically I would get an error that was the same as described in #10846.
```
error: package `{path}/cargo/target/tmp/cit/t0/foo/sub/foo/Cargo.toml` is a member of the wrong workspace
expected: {path}/cargo/target/tmp/cit/t0/foo/sub/Cargo.toml
actual: {path}/cargo/target/tmp/cit/t0/foo/Cargo.toml
```
I then tested it on the commit with the fix and the test passed every time.
---
While this does add a test to catch any regression I am worried that it will not catch it every time. It was noted in #10846 that this error would sometimes happen but not every time, in my testing I found this to be true as well. Since this is caused by the `HashMap` order changing each run, switching to something ordered like `BTreeMap` **_should_** catch any regressions every run (if the implementation were to ever change). I'm not sure if this is necessary so I figured I would note the concern here.
clarify profile used for 'cargo install --debug'
TIL that the profile used by `cargo build` is called `dev`, not `debug`. That made me wonder, is that profile also used by `cargo install --debug` (despite the name of the flag being `--debug`, not `--dev`)? Turns out the answer is yes, but the first 2 places where I looked for help did not say that. So this PR changes those two places to be explicit about this.
test(add): Ensure comments are preserved
A comment on killercup/cargo-edit#15 had me worried that `cargo add` was
deleting comments now. It appears that isn't the case for inline
tables.
Standard tables however do delete comments. The work to make sure they
don't conflicts with another need. When changing the source, we delete
the old source fields and append the new which can cause some formatting
to be carried over unnecessarily.
For example, what would normally look like
```toml
cargo-list-test-fixture-dependency = { optional = true, path = "../dependency", version = "0.0.0" }
```
When fixed to preserve comments with my naive solution looks like
```toml
cargo-list-test-fixture-dependency = { optional = true , path = "../dependency", version = "0.0.0" }
```
Note that `optional = true` used to be last, so space separating it and
`}` was kept, now separating it and `,`.
More work will be needed to get this into an ideal state but we can at
least have confidence with inline tables for now.
A comment on killercup/cargo-edit#15 had me worried that `cargo add` was
deleting comments now. It appears that isn't the case for inline
tables.
Standard tables however do delete comments. The work to make sure they
don't conflicts with another need. When changing the source, we delete
the old source fields and append the new which can cause some formatting
to be carried over unnecessarily.
For example, what would normally look like
```toml
cargo-list-test-fixture-dependency = { optional = true, path = "../dependency", version = "0.0.0" }
```
When fixed to preserve comments with my naive solution looks like
```toml
cargo-list-test-fixture-dependency = { optional = true , path = "../dependency", version = "0.0.0" }
```
Note that `optional = true` used to be last, so space separating it and
`}` was kept, now separating it and `,`.
More work will be needed to get this into an ideal state but we can at
least have confidence with inline tables for now.
Fix nested workspace resolution
This fixes a bug that was introduced in #10776 with nested workspaces.
As an example, say we have two workspaces:
`/code/example/Cargo.toml` and `/code/example/sub/Cargo.toml`, and a crate within the `sub` workspace `/code/example/sub/test-crate/Cargo.toml`.
Since the `ws_roots` is a HashMap with randomized ordering, this code will _sometimes_ cause the workspace at `/code/example/Cargo.toml` to be discovered and used _before_ `/code/example/sub/Cargo.toml`,
b1dd22e668/src/cargo/core/workspace.rs (L1704-L1710)
This will then cause the `validate_members` method to fail as the member thinks it is a member of a different workspace than it should be.
b1dd22e668/src/cargo/core/workspace.rs (L874-L891)
This change just makes it so that the input manifest path is walked up to find the (presumably) most appropriate workspace so that the ordering of the `HashMap` doesn't matter.
If you encounter this bug by running cargo nightly, you can workaround it by adding the crate(s) to the `excluded` field in the workspace they don't belong to.
Small tweaks to the future-incompat docs.
This contains a few changes to the future-incompat docs to try to make it a little clearer how it works, and some formatting and wording changes.
Mention `[patch]` config in "Overriding Dependencies"
The "Overriding Dependencies" chapter is the primary documentation for the `[patch]` table, but it does not mention that `[patch]` also works in config files. This adds a note to mention this option.
cc https://github.com/rust-lang/cargo/issues/10832#issuecomment-1177647089
Update terminal-width flag.
The rustc flag `-Zterminal-width` has been stabilized as `--terminal-width` in https://github.com/rust-lang/rust/pull/95635. This updates cargo to use the new flag so that tests will pass.
Tests won't pass until the next nightly is published in about 10 hours from now. I just wanted to post this to get ahead of the breaking change.
This doesn't stabilize in cargo because that will take more time, and this is needed to prevent CI from failing. Will continue the stabilization discussion at https://github.com/rust-lang/rust/issues/84673.
Fix publishing to crates.io with -Z sparse-registry
Attempting to publish a crate to crates.io with `-Z sparse-registry` failed with the following error:
```
error: failed to publish to registry at https://crates.io
Caused by:
the remote server responded with an error: Dependency `serde` is hosted on another registry. Cross-registry dependencies are not permitted on crates.io.
```
The check in `registry.rs` `dep_registry_id != registry_id` caused the `publish` operation include the crates.io index url in the HTTP request because the id was replaced. The crates.io API seems to require that the `registry` field is not present.
This change fixes the issue by making the `registry` function return the non-replaced crates.io `source_id` only for this case. Other replacement indices of crates.io continue to include the registry URL when publishing.
Tested manually by publishing `arlosi-cargo-test` to crates.io with `-Z sparse-registry`
Fixes#10828
r? `@Eh2406`
Make `is_yanked` return `Poll<>`
The `is_yanked` check performed by `cargo install` and `cargo package` was running sequentially (calling `block_until_ready` after every check).
This change makes `is_yanked` return `Poll<>` and runs the check in parallel, which gives better performance for `cargo install --locked` and `cargo package` when using a sparse registry.
fixes#10821
r? `@ehuss`
Fix corrupted git checkout recovery.
This fixes an issue where cargo would not recover from a corrupted git checkout correctly when using `net.git-fetch-with-cli`.
Git dependencies have two clones, the "db" and the "checkout". The "db" is shared amongst multiple checkout revisions from the same repository. The "checkout" is each individual revision. There was some code in `copy_to` which creates the "checkout" that tries to recover from an error. The "checkout" can be invalid if cargo was interrupted while cloning it, or if there is fs corruption. However, that code was failing when using the git CLI. For reasons I did not dig into, the "db" does not have a HEAD ref, so that special-case fetch was failing with a `couldn't find remote ref HEAD` error from `git`.
This changes it so that if the "checkout" is invalid, it just gets blown away and a new clone is created (instead of calling `git fetch` from the "db").
I believe there is some long history for this `copy_to` code where it made more sense in the past. Previously, the "checkout" directories used the `GitReference` string as-is. So, for example, a branch would checkout into a directory with that branch name. At some point, it was changed so that each checkout uses a short hash of the actual revision. Rebuilding the checkout made sense when it was possible for that checkout revision to change (like if new commits were pushed to a branch). That recovery is no longer necessary since a checkout is only ever one revision.
Fixes#10826
add a cache for discovered workspace roots
## History
`@ehuss` [noticed that](https://github.com/rust-lang/cargo/pull/10736#discussion_r894071933) workspace inheritance caused a significant increase in startup times when using workspace inheritance. This brought up the creation of #10747.
When using a similar test setup [to the original](https://github.com/rust-lang/cargo/pull/10736#discussion_r894822022) I got
```
Benchmark 1: cd rust; ../../../target/release/cargo metadata
Time (mean ± σ): 149.4 ms ± 3.8 ms [User: 105.9 ms, System: 31.7 ms]
Range (min … max): 144.2 ms … 162.2 ms 19 runs
Benchmark 2: cd rust-ws-inherit; ../../../target/release/cargo metadata
Time (mean ± σ): 191.6 ms ± 1.4 ms [User: 145.9 ms, System: 34.2 ms]
Range (min … max): 188.8 ms … 193.9 ms 15 runs
```
This showed a large increase in time per cargo command when using workspace inheritance.
During the investigation of this issue, other [performance concerns were found and addressed](https://github.com/rust-lang/cargo/pull/10761). This resulted in a drop in time across the board but heavily favored workspace inheritance.
```
Benchmark 1: cd rust; ../../../target/release/cargo metadata
Time (mean ± σ): 139.3 ms ± 1.7 ms [User: 99.8 ms, System: 29.4 ms]
Range (min … max): 137.1 ms … 144.5 ms 20 runs
Benchmark 2: cd rust-ws-inherit; ../../../target/release/cargo metadata
Time (mean ± σ): 161.7 ms ± 1.4 ms [User: 120.4 ms, System: 31.2 ms]
Range (min … max): 158.0 ms … 164.6 ms 18 runs
```
## Performance after changes
`hyperfine --warmup 10 "cd rust; ../../../target/release/cargo metadata" "cd rust-ws-inherit; ../../../target/release/cargo metadata" --runs 40`
```
Benchmark 1: cd rust; ../../../target/release/cargo metadata
Time (mean ± σ): 140.1 ms ± 1.5 ms [User: 99.5 ms, System: 30.7 ms]
Range (min … max): 137.4 ms … 144.0 ms 40 runs
Benchmark 2: cd rust-ws-inherit; ../../../target/release/cargo metadata
Time (mean ± σ): 141.8 ms ± 1.6 ms [User: 100.9 ms, System: 30.9 ms]
Range (min … max): 138.4 ms … 145.4 ms 40 runs
```
[New Benchmark](https://github.com/rust-lang/cargo/pull/10754)
`cargo bench -- workspace_initialization/rust`
```
workspace_initialization/rust
time: [14.779 ms 14.880 ms 14.997 ms]
workspace_initialization/rust-ws-inherit
time: [16.235 ms 16.293 ms 16.359 ms]
```
## Changes Made
- [Pulled a commit](bbd41a4dca) from `@ehuss` that deduplicated finding a workspace root to make the changes easier
- Added a cache in `Config` to hold found `WorkspaceRootConfig`s
- This makes it so manifests should only be parsed once
- Made `WorkspaceRootConfig` get added to the cache when parsing a manifest
## Testing Steps
To check the new benchmark:
1. `cd benches/benchsuite`
2. `cargo bench -- workspace_initialization/rust`
Using `hyperfine`:
1. run `cargo build --release`
2. extract `rust` and `rust-ws-inherit` in `benches/workspaces`
3. cd `benches/workspaces`
4. Prime the target directory with a cache of `rustc` info. In `rust` and `rust-ws-inherit`, run: `cargo +nightly c -p linkchecker`. Otherwise it would be measuring `rustc` overhead.
4. run `hyperfine --warmup 10 "cd rust; ../../../target/release/cargo metadata" "cd rust-ws-inherit; ../../../target/release/cargo metadata" --runs 40`
closes#10747
fix(add): Don't panic with `--offline`
For some reason, I defined my own `--offline` flag and it didn't get
updated with the global `--offline` flag, so it started failing.
The new test previously paniced and now it doesn't.
Fixes#10814
Fix zsh completions for add and locate-project
Currently, trying to tab-complete either the `cargo add` or `cargo locate-project` subcommands results in output such as this:
```
cargo add _cargo:67: command not found: --default-features[enable the default features]
_cargo:68: command not found: --no-default-features[don't enable the default features]
_cargo:69: command not found: --optional[mark the dependency as optional]
```
This is because some line continuations are missing from `src/etc/_cargo`. This PR adds these line continuations.
This file gets packaged for Rust releases by acdcdfb61b/src/bootstrap/dist.rs (L956) and Rustup ultimately places it in `~/.rustup/toolchains/*/share/zsh/site-functions/_cargo`. `rustup completions zsh cargo` outputs a script which sources this filepath.
The easier approach to testing this is probably to manually copy `_cargo` to its proper location under `~/.rustup`, assuming Rust is already installed via Rustup and Zsh completions for Cargo are already installed, but however you choose to install this, testing is as simple as observing that Zsh can correctly tab complete `cargo add` and `cargo locate-project`.
The zsh tab-completion script had missing line continuations which would
cause `command not found:` errors when trying to complete the
`cargo add` or `cargo locate-project` subcommands.
Bump cargo-util version.
#10546 made a semver-incompatible change to the API of `ProcessBuilder::get_args`. Unfortunately we did not catch that until it was published. This bumps the version of cargo-util to 0.2.1 to accommodate that change. Stable will get version 0.2.0 so that the changes on beta can be released as 0.2.1 in their own time.
cc #10803
