Turn off debuginfo for build dependencies v2
This PR is an alternative to #10493, fixing its most important issue: the unit graph optimization to reuse already built artifacts for dependencies shared between the build-time and runtime subgraphs is preserved (most of the time).
By deferring the default debuginfo level in `dev.build-override` until unit graph sharing, we check whether re-use would happen. If so, we use the debuginfo level to ensure reuse does happen. Otherwise, we can avoid emitting debuginfo and improve compile times (on clean, debug and check builds -- although reuse only happens on debug builds).
I've kept the message explaining how to bump the debuginfo level if an error occurs while running a build script (and backtraces were requested) that was in the previous PR.
I've ran benchmarks on 800 crates, at different parallelism levels, and published the (surprisingly good) results with visualizations, summaries, and raw data [here](https://github.com/lqd/rustc-benchmarking-data/tree/main/experiments/cargo-build-defaults).
Opening this PR as discussed in [Eric's message](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Defaults.20for.20faster.20compilation.20of.20.22for.20host.22.20units/near/304236576l) as draft since 2 tests won't pass. That fixes the `cargo-crev` case we saw as a blocker last time, but doesn't fix all such cases of reuse, like the 2 failing tests:
- [`optional_build_dep_and_required_normal_dep`](642a0e625d/tests/testsuite/build_script.rs (L4449))
- and [`proc_macro_ws`](bd5db301b0/tests/testsuite/features2.rs (L1051))
These failures make sense, since the debuginfo optimization is done during traversal, it depends on the contents of the unit graph. These tests ensure that sharing happens even on finer-grained invocations: respectively, with an optional shared dependency that is later enabled, and building shared dependencies by themselves first and later as part of a complete workspace build.
In both these situations, there is no unit that is shared in the graph during the first invocation, so we do the optimization and remove the debuginfo. When the graph changes in the following invocation, sharing is present and we have to build the shared units again with debuginfo.
These cases feel rarer than `cargo-crev`'s case, but I do wonder if there's a way to fix them, or if it's acceptable to not support them.
Add partial support for SSH known hosts markers
### What does this PR try to resolve?
The SSH `known_hosts` file parsing in Cargo did not previously support
markers. Markers are modifiers on the lines (``@cert-authority`` and
``@revoked`)` which denote special behavior for the details on that line.
Lines were skipped entirely.
This silent skipping of marker lines can be confusing to a user, who
sees that their command line Git/SSH client works for some repository,
but Cargo reports that no host key is found.
This change adds support for the ``@revoked`` marker. This marker denotes
that a key should be rejected outright. It is of limited use without
``@cert-authority`` marker support. However, if it is present in a user's
`known_hosts` file, then Cargo definitely shouldn't accept that key and
probably shouldn't suggest that the user add it to their `known_hosts`
either.
The change also adds support for detecting ``@cert-authority`` markers in
`known_hosts` files. These lines cannot yet be used for host key
verification, but if one is found for a matching host, the user will be
informed that Cargo doesn't support ``@cert-authority`` markers in the
error message. Additionally, the user will be advised to use the
`net.git-fetch-with-cli` config option to use the command line git
client for fetching crates from Git.
Refs: #11577
### How should we test and review this PR?
The changes in this PR are covered by unit tests, all within
`src/cargo/sources/git/known_hosts.rs`.
Additionally, manual testing can be performed. For this you will need
an OpenSSH server (it doesn't need to be a Git server). I'll assume
that you have one running on your local machine at `127.0.0.1`.
#### Setup
1. Create a new Cargo project and add the following line to `[dependencies]`:
```toml
fake-crate = { git = "ssh://127.0.0.1/fake-crate.git" }
```
#### Test missing host key: `HostKeyNotFound` (existing functionality)
1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`.
2. Verify host key not present: `ssh 127.0.0.1`. SSH should tell you `The authenticity of host '127.0.0.1 (127.0.0.1)' can't be established.`
3. Run `cargo build`
4. Expect error from Cargo: `error: unknown SSH host key`
#### Test ``@revoked`` key: `HostKeyRevoked`
1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`.
2. Add host key: `ssh 127.0.0.1` answer `yes`
3. Find all lines in `known_hosts` beginning with `127.0.0.1` (there may be multiple).
4. Add ``@revoked` ` to the beginning of all lines in (3)
5. Run `cargo build`
6. Expect error from Cargo: error: Key has been revoked for `127.0.0.1`
#### Test `@cert-authority`` (not being supported): `HostHasOnlyCertAuthority`
1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`.
2. Run `cargo build`
3. Expect error from Cargo: `error: unknown SSH host key`
4. Check the line after ` The key to add is:` in the error message and copy the key type (e.g. `ecdsa-sha2-nistp256`)
5. Add a line to `known_hosts`: ``@cert-authority` 127.0.0.1 <key-type> AAAAB5Wm` (e.g. ``@cert-authority` 127.0.0.1 ecdsa-sha2-nistp256 AAAAB5Wm`)
7. Run `cargo build`
8. Expect error from Cargo: error: Found a ``@cert-authority`` marker for `127.0.0.1`
### Additional information
Cargo doesn't currently support a few things when checking host keys. This may affect the testing described above.
* Multiple host key types (OpenSSH negotiates the host key type and can support matching the one present in the `known_hosts` file even when it's not the preferred type of the server).
* Wildcard matching of host patterns (there's a FIXME for this)
More information about SSH known host markers can be found
on #11577.
Replace `winapi` with `windows-sys` crate.
### What does this PR try to resolve?
replace `winapi` with `windows-sys` crate.
It's officially maintained.
### How should we test and review this PR?
I just do the replacement of API. I think it is quite clear.
And I found a `cfg` for `windows` may miss checked error.
### Additional information
No one.
Handle .cargo-ok being truncated
This fixes an issue where if the `.cargo-ok` file is truncated then Cargo will get stuck with being unable to extract the package. Creating the `.cargo-ok` file uses `create_new` which will fail if the file exists. If the file gets created, but there is a failure to flush it, or if the filesystem otherwise gets corrupted, then the `create_new` step will fail with `File exists`.
The solution here is to delete the cache directory if the `.cargo-ok` file is truncated, as there may be some uncertainty of the validity of the cache. This also adds better error handling if there is some problem opening the `.cargo-ok` file instead of ignoring errors.
Closes#11638
This test dynamically enables a shared build/runtime dependency, and
therefore doesn't trigger the build/runtime sharing reuse optimization,
as the build dep is initially built without debuginfo for optimization
purposes.
Although `CompileKind::is_host` is currently used for build dependencies prior
to unit graph sharing, it's not a guarantee. So we use `UnitFor::is_for_host`
to make detection more future-proof.
Add some assertions to ensure that debuginfo is not used to compile
build dependencies, in a way that differs between the old and new
defaults: some of the assert elision could match the previous defaults
with debuginfo. These new assertions break if `-C debuginfo` is present
in the commands cargo ran.
chore: Add autolabel for `Command-*` labels
In a recent cargo team meeting it was discussed adding `autolabel` for some labels in the repo. This PR adds auto-labeling for all `Command-*` labels on their source files (not test files).
Update cross test instructions for aarch64-apple-darwin
This updates the instructions for running Cargo's testsuite on aarch64-apple-darwin, which has different requirements than x86_64-apple-darwin.
Make cargo install report needed features
### What does this PR try to resolve?
The problem described in issue #11617 where cargo tells the user, that no binaries are available
because of the chosen features, but does not tell which features could be enabled to fix the
situation.
Fixes#11617.
### How should we test and review this PR?
Try to cargo install a crate, where all binaries need some features enabled, without enabling
the features and check the error message.
The test suite already contains tests for this.
docs(contrib): Remove out-of-date process step
The `-Z` flag seems to now be documented as part of defining the feature in `features.rs`, so (1) the destination is wrong and (2) this will be done naturally as part of creating the feature.
This enum will be used to model the current Option<u32> value in
profiles, as the explicitly set value, and also allow to model a
deferred value: one that can be ignored for optimization purposes,
and used in all other cases.
This allows to have a default debuginfo for build dependencies, that can
change:
- if a dependency is shared between the build and runtime subgraphs, the
default value will be used, to that re-use between units will
continue.
- otherwise, a build dependency is only used in a context where
debuginfo is not very useful. We can optimize build times in this
situation by not asking rustc to emit debuginfo.
The `-Z` flag seems to now be documented as part of defining the feature in
`features.rs`, so (1) the destination is wrong and (2) this will be done
naturally as part of creating the feature.
Do not error for `auth-required: true` without `-Z sparse-registry`
Registries that include `auth-required: true` in their `config.json` currently hit the following error on stable:
```
authenticated registries require `-Z registry-auth`
```
This situation makes it difficult for a registry to optionally offer the `auth-required: true` feature, since it forces users on to the nightly toolchain.
This PR changes the behavior to ignore the `auth-required: true` field of `config.json` without `-Z registry-auth`.
The downside to this change is that it makes it harder to discover why a registry isn't working, since the user will get an HTTP 401 error from the server, rather than a message from Cargo suggesting adding `-Z registry-auth`.
r? `@Eh2406`
Since a @revoked line might deny access to a key which would otherwise
be accepted, we need to process all lines before we decide that a host
key should be accepted.
