Auto merge of #10867 - Muscraft:remove-unneeded-nightly-from-tests, r=ehuss
remove`.masquerade_as_nightly_cargo()` from various tests the no longer need it
When looking at making [`.masquerade_as_nightly_cargo()` take in a list of reasons](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/problems.20finding.20.60.2Emasquerade_as_nightly_cargo.28.29.60), I found various tests that no longer needed `.masquerade_as_nightly_cargo()`. This removes it from all of the offending tests.
I tried my best to find all tests that no longer need it but I could have missed one or two.
Auto merge of #10866 - Muscraft:update-extra-link-tests, r=ehuss
remove `.masquerade_as_nightly_cargo()` from build_script_extra_link_arg.rs
When looking at making [`.masquerade_as_nightly_cargo()` take in a list of reasons](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/problems.20finding.20.60.2Emasquerade_as_nightly_cargo.28.29.60), I found that [`build_script_extra_link_arg.rs`](https://github.com/rust-lang/cargo/blob/8827baaa781b37872134c1ba692a6f0aeb37890e/tests/testsuite/build_script_extra_link_arg.rs) still was using it. `extra-link-arg` was [stabilized in 1.56.0](https://github.com/rust-lang/cargo/blob/8827baaa781b37872134c1ba692a6f0aeb37890e/src/doc/src/reference/unstable.md#extra-link-arg), so this PR removes `.masquerade_as_nightly_cargo()` from all the tests in this file.
Auto merge of #10862 - Muscraft:workspace-selection-test, r=epage
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.
Auto merge of #10861 - RalfJung:cargo-install-debug, r=ehuss
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.
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.
Ed Page [Mon, 11 Jul 2022 21:14:34 +0000 (16:14 -0500)]
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.
Auto merge of #10846 - EmbarkStudios:fix-workspace-resolution, r=epage
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`,
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.
Auto merge of #10836 - ehuss:patch-config-docs, r=weihanglo
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
Auto merge of #10833 - ehuss:terminal-width-new-flag, r=Eh2406
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.
Auto merge of #10831 - arlosi:sparse-publish-fix, r=Eh2406
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`
Auto merge of #10830 - arlosi:parallel_yank, 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.
Auto merge of #10829 - ehuss:corrupted-checkout, r=weihanglo
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.
Auto merge of #10776 - Muscraft:cache-workspace-discovery, r=weihanglo
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](https://github.com/rust-lang/cargo/pull/10736/commits/bbd41a4dca8e001c8a45979c563293ef0a5c0d77) 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`
Auto merge of #10816 - turrisxyz:Pinned-Dependencies-GitHub, r=epage
chore: Set permissions for GitHub actions
Restrict the GitHub token permissions only to the required ones; this way, even if the attackers will succeed in compromising your workflow, they won’t be able to do much.
- Included permissions for the action. https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions
[Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)
Restrict the GitHub token permissions only to the required ones; this way, even if the attackers will succeed in compromising your workflow, they won’t be able to do much.
- Included permissions for the action. https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions
[Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)
Auto merge of #10810 - Diomendius:bugfix-zsh-completions, r=weihanglo
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 https://github.com/rust-lang/rust/blob/acdcdfb61b7b472bfacbb8bb889bdf3204827f2e/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.
Auto merge of #10804 - ehuss:bump-cargo-util, r=joshtriplett
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.
bors [Wed, 29 Jun 2022 22:13:09 +0000 (22:13 +0000)]
Auto merge of #10799 - Urgau:check-cfg-fix-config-deserialization, r=ehuss
Fix deserialization of check-cfg in config.toml
When improving the check-cfg implementation in https://github.com/rust-lang/cargo/pull/10566 I changed the internal representation of `check_cfg` from multiple `bool` options to one `Option<(bool, bool, bool, bool)>` but I didn't realize until https://github.com/rust-lang/rust/issues/82450#issuecomment-1169836928 that the internal representation is actually somewhat public as it's used in the `[unstable]` in `.cargo/config.toml`.
And because TOML cannot represent tuples there is no way to set it from the `[unstable]` section. This PR fix this oversight by using a custom deserializer method similar to what was already done for `build-std`.
```console
$ rm -rf ~/.cargo/git/db/cargo-* ~/.cargo/git/checkouts/cargo-*
$ time $CARGO generate-lockfile
$ du -shc ~/.cargo/git/db/cargo-* ~/.cargo/git/checkouts/cargo-*
```
Using current cargo from the most recent nightly, the `generate-lockfile` command downloads 69704 git objects in 7.0 seconds, consuming 41 MB on disk.
Using cargo built from this PR by `cargo build --release`, the same command downloads 21481 objects in 2.2 seconds, consuming 17 MB on disk.
Once libgit2 is able to do shallow clones (https://github.com/libgit2/libgit2/issues/3058) this can be even more of a speedup. Using command-line git (which does not use libgit2) and `time git fetch --depth=1 https://github.com/rust-lang/cargo b30694b4d9b29141298870b7993e9aee10940524` indicates that it downloads just 262 objects in 1.1 seconds.
bors [Fri, 24 Jun 2022 18:27:15 +0000 (18:27 +0000)]
Auto merge of #10778 - epage:test, r=ehuss
refactor(test): Clarify asserts are for UI
In writing the contrib documentation for functional vs ui tests, I
realized that as we work to make snapbox work for the functional tests,
we'll need distinct `Assert` objects since we'll want to elide a lot
more content in functional tests. I'm making room for this by
qualifying the existing asserts as being for "ui".
bors [Thu, 23 Jun 2022 20:12:03 +0000 (20:12 +0000)]
Auto merge of #10785 - ehuss:fix-dead_code-diag, r=Eh2406
Fix tests due to change in dead_code diagnostic.
https://github.com/rust-lang/rust/pull/97853 changed some diagnostics which is causing some tests to fail on the latest nightly. This updates the tests to work on both stable and nightly.
bors [Wed, 22 Jun 2022 20:02:24 +0000 (20:02 +0000)]
Auto merge of #10755 - jonhoo:stabilize-config-cli, r=ehuss
Stabilize config-cli
This stabilizes the `--config` CLI argument as per [this FCP](https://github.com/rust-lang/cargo/issues/7722#issuecomment-1114369809).
It also makes the adjustment [suggested by `@ehuss](https://github.com/rust-lang/cargo/issues/7722#issuecomment-1098612205)` to allow stabilizing `--config path` without _also_ stabilizing [`config-include`](https://doc.rust-lang.org/cargo/reference/unstable.html#config-include).
I took a guess that this would land in 1.63 and put that in the tombstone entry in the unstable docs, but let me know if that's likely to be wrong.
Ed Page [Tue, 21 Jun 2022 19:59:54 +0000 (14:59 -0500)]
refactor(test): Clarify asserts are for UI
In writing the contrib documentation for functional vs ui tests, I
realized that as we work to make snapbox work for the functional tests,
we'll need distinct `Assert` objects since we'll want to elide a lot
more content in functional tests. I'm making room for this by
qualifying the existing asserts as being for "ui".
bors [Mon, 20 Jun 2022 15:34:50 +0000 (15:34 +0000)]
Auto merge of #10774 - Muscraft:update-benchsuite-deps, r=Eh2406
remove unused dependency from benchsuite
In #10754 I added a new benchmark to the benchsuite. While figuring out the best way to add the new benchmark, I added `cargo-test-support` as a dependency. It appears I missed removing it before making the PR.
This PR removes `cargo-test-support` since it is not needed
bors [Mon, 20 Jun 2022 14:47:36 +0000 (14:47 +0000)]
Auto merge of #10758 - epage:docs, r=weihanglo
docs(contrib): Add documentation for ui tests
### What does this PR try to resolve?
This only adds information about snapshot testing using `snapbox` and
keeps the functional testing documentation focused on the existing
facilities. We can updated this as our use of `snapbox` matures.
### How should we test and review this PR?
I did not generate and verify the HTML
In writing this, I did notice that we define
`cargo_test_support::compare::assert` but only use it for filesystem
asserts and not binary asserts. We should probably add our own function
that wraps `snapbox::cmd::Command::cargo()` and passes in
`cargo_test_support::compare::assert`. I've left that out of this PR to
keep things focused.
bors [Sat, 18 Jun 2022 06:53:48 +0000 (06:53 +0000)]
Auto merge of #10754 - Muscraft:benchsuite, r=epage
Add a benchmark for workspace initialization
It [was suggested](https://github.com/rust-lang/cargo/pull/10736#discussion_r894822022) that a benchmark for workspace initialization should be added. This was suggested because there were issues with the performance of [workspace inheritance](https://github.com/rust-lang/cargo/issues/10747) as well as a general way to track the workspace initialization time across cargo changes
### Changes
- Moved common functions out of `resolve.rs` to a shared `lib.rs`
- Added a new struct to be used when creating a new benchmark
- This was done because `env!("CARGO_TARGET_TMPDIR")` would fail to compile when put inside of the new `lib.rs`
- Added a new workspace test for workspace inheritance
- This new workspace does not have a repo that it was built from and if one needs to be made I can change that
bors [Fri, 17 Jun 2022 16:46:26 +0000 (16:46 +0000)]
Auto merge of #10764 - kornelski:clarify-http, r=Eh2406
Use specific terminology for sparse HTTP-based registry
Before the options is popularized, I'd like to take opportunity to give it a unique name used consistently. It's been called "http" registry, but that's a rather generic term, especially that existing git-based registry also uses HTTP as its transport.
New registry URLs use `sparse+https://` prefix, so calling it "sparse" registry seems more appropriate.
Ed Page [Tue, 14 Jun 2022 22:03:50 +0000 (17:03 -0500)]
docs(contrib): Add documentation for ui tests
This only adds information about snapshot testing using `snapbox` and
keeps the functional testing documentation focused on the existing
facilities. We can updated this as our use of `snapbox` matures.
In writing this, I did notice that we define
`cargo_test_support::compare::assert` but only use it for filesystem
asserts and not binary asserts. We should probably add our own function
that wraps `snapbox::cmd::Command::cargo()` and passes in
`cargo_test_support::compare::assert`. I've left that out of this PR to
keep things focused.
bors [Tue, 14 Jun 2022 23:11:47 +0000 (23:11 +0000)]
Auto merge of #10753 - epage:clap32-deprecated, r=weihanglo
chore: Upgrade to clap 3.2
I decided to use cargo as a test case for upgrading to clap 3.2 prior to release. From the builder API's perspective, a lot has change. While the changelog summarizes it, the release announcement is still pending. In short, the API is now typed. You declare what type an `Arg` is and access it by that type. flags (both `is_present` and `occurrences_of`) are also now specified through `ArgAction` and the result gets stored like any other arg value. I made a `ArgMatchesExt::flag` and `command_prelude::flag` functions to make working with these easier.
Now that clap exposes non-panicking variants of its functions, I switched from a "look before you leap" approach with `is_arg_valid` to a "better to ask forgiveness than permission" with checking the error variant. I decided to just make a convenience to swallow that error type. Not a fan of how loose things are but I think this will just be something we iterate on over time.
Arlo Siemsen [Thu, 9 Jun 2022 03:04:33 +0000 (22:04 -0500)]
Improve testing framework for http registries
Improve integration of the http server introduced by the http-registry feature.
Now the same HTTP server is used for serving downloads, the index, and
the API.
This makes it easier to write tests that deal with authentication and
http registries.
bors [Tue, 7 Jun 2022 21:57:52 +0000 (21:57 +0000)]
Auto merge of #10725 - arlosi:http-cratesio, r=Eh2406
Make -Z http-registry use index.crates.io when accessing crates-io
Use `sparse+https://index.crates.io/` to access crates.io when `-Z http-registry` is set.
* `Cargo.lock` files still emit the github URL `https://github.com/rust-lang/crates.io-index`.
* Allows publishing to a source-replaced `crates-io` only for `index.crates.io`. Other source-replacements of `crates-io` are still blocked.
projects / cargo.git / log