This commit adds support for passing the keyword "default"
to either the CLI "--jobs" argument on the "[build.jobs]"
section of ".cargo/config".
This is dony by:
1. Changing the "jobs" config type to an enum that holds
a String or an Integer(i.e. i32).
2. Matching the enum & casting it to an integer
Signed-off-by: Charalampos Mitrodimas <charmitro@gmail.com>
It was unnecessary to pass `spilt-debuginfo` if there is no debuginfo.
Tests are touched here only for matching rustflags invocation stderr
in the original test suite.
Previously, `Debuginfo::None` meant "don't pass -C debuginfo" and `Explicit(None)` meant
"-C debuginfo=0", which occasionally led to caching bugs where cargo would sometimes pass
`-C debuginfo=0` and sometimes not. There are no such bugs currently that we know of, but
representing them the same within cargo avoids the possibility of the bug popping up again in the
future.
I tested the `with_stderr_does_not_contain_tests` with this diff to ensure they did not pass:
```diff
diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs
index 55ec17182..c186dd00a 100644
--- a/src/cargo/core/compiler/mod.rs
+++ b/src/cargo/core/compiler/mod.rs
@@ -1073,9 +1073,7 @@ fn build_base_args(
let debuginfo = debuginfo.into_inner();
// Shorten the number of arguments if possible.
- if debuginfo != TomlDebugInfo::None {
cmd.arg("-C").arg(format!("debuginfo={}", debuginfo));
- }
cmd.args(unit.pkg.manifest().lint_rustflags());
if !rustflags.is_empty() {
```
fix(add): Reduce the chance we re-format the user's `[features]` table
### What does this PR try to resolve?
#11743 pointed out that we re-format the users `[features]` table when running `cargo add` which was a bug introduced in #11099.
This reduces the chance people will run into this problem
- Reducing the scope of the `fmt` call
- Preserving formatting in a simple case
Actually removing the `fmt` case can make some common formatting cases more complex to do "right", so I'm punting on that for now.
### How should we test and review this PR?
Look at the individual commits as I show how each change improves the behavior of `cargo add`.
This is a carry-over from cargo-edit where we had to worry about the UX
of all of the behavior while now we are just relying on built-in cargo
behavior and don't need to test it specifically for `cargo add`.
On my machine, this test takes 11s.
refactor(tests): Reduce cargo-add setup load
This just gets rid of irrelevant packages in the registry. Looking into which versions aren't needed would require a deeper pass, so I held off on that for now.
Before, the tests were in the 300-500ms range and now they take 100-300ms.
This did call to my attention that `unrelated` is misspelled as `unrelateed` but holding off on fixing that to reduce conflicts.
Warn when an edition 2021 crate is in a virtual workspace with default resolver
Edition 2021 updates the default resolver to version "2", but developers using virtual workspaces commonly don't get this update because the virtual workspace defaults to version "1". Warn when this situation occurs so those developers can explicitly configure their workspace and will be more likely to know that they will need to update it in the future.
Fixes#10112
This just gets rid of irrelevant packages in the registry. Looking into
which versions aren't needed would require a deeper pass, so I held off
on that for now.
Before, the tests were in the 300-500ms range and now they take
100-300ms.
This did call to my attention that `unrelated` is misspelled as
`unrelateed` but holding off on fixing that to reduce conflicts.
Edition 2021 updates the default resolver to version "2", but developers
using virtual workspaces commonly don't get this update because the
virtual workspace defaults to version "1". Warn when this situation
occurs so those developers can explicitly configure their workspace and
will be more likely to know that they will need to update it in the
future.
refactor(tests): Reduce cargo-remove setup load
This reduces the number of packages published in tests. This is an artifact of when I changed `cargo-edit` from relying on crates.io to test-generated published packages. I took the fastest path to making that conversion and took the shortcut of creating everything for every test. I had assumed the cost was low but `@Muscraft` noticed that this takes up a lot of space which we run out of on CI occasionally and I expect a lot of small files are slowing down windows.
This only updates `cargo-remove`. I'll be doing a follow up for `cargo-add`.
fix(lints): Switch to -Zlints so stable projects can experiment
### What does this PR try to resolve?
In #12115, we explored how we can let stable projects
experiment with `[lints]` to provide feedback. What we settled on is
switching from the `cargo-features` manifest key to the `-Z` flag as
`cargo-features` always requires nightly while `-Z` only requires it
when being passed in. This means a project can have a `[lints]` table
and have CI / contributors run `cargo +nightly check -Zlints` when they
care about warnings.
### How should we test and review this PR?
Demonstrate how you test this change and guide reviewers through your PR.
With a smooth review process, a pull request usually gets reviewed quicker.
If you don't know how to write and run your tests, please read the guide:
https://doc.crates.io/contrib/tests
### Additional information
I considered reworking the code to show the user the errors they would encounter once the feature is stable but held off. I wasn't quite sure what language to use and most likely a user would have something doing error reporting, like CI, so it should be fine.
feat: `lints` feature
### What does this PR try to resolve?
Implement rust-lang/rfcs#3389 which shifts a subset of `.cargo/config.toml` functionality to `Cargo.toml` by adding a `[lints]` table.
This **should** cover all of the user-facing aspects of the RFC
- This doesn't reduce what flags we fingerprint
- This will fail if any lint name as `::` in it. What to do in this case was in the RFC discussion but I couldn't find the thread to see what all was involved in that discussion
- This does not fail if a `[lints]` table is present or malformed unless nightly with the `lints` feature enabled
- The idea is this will act like a `[lints]` table is present in an existing version of rust, ignore it
- The intent is to not force an MSRV bump to use it.
- When disabled, it will be a warning
- When disabled, it will be stripped so we don't publish it
Tracking issue for this is #12115.
### How should we test and review this PR?
1. Look at this commit by commit to see it gradually build up
2. Look through the final set of test cases to make sure everything in the RFC is covered
I tried to write this in a way that will make it easy to strip out the special handling of this unstable feature, both in code and commit history
### Additional information
I'd love to bypass the need for `cargo-features = ["lints"]` so users today can test it on their existing projects but hesitated for now. We can re-evaluate that later.
I broke out the `warn_for_feature` as an experiment towards us systemitizing this stabilization approach which we also used with #9732. This works well when we can ignore the new information which isn't too often but sometimes happens.
This does involve a subtle change to `profile.rustflags` precedence but
its nightly and most likely people won't notice it? The benefit is its
in a location more like the rest of the rustflags.
In rust-lang/cargo#12115, we explored how we can let stable projects
experiment with `[lints]` to provide feedback. What we settled on is
switching from the `cargo-features` manifest key to the `-Z` flag as
`cargo-features` always requires nightly while `-Z` only requires it
when being passed in. This means a project can have a `[lints]` table
and have CI / contributors run `cargo +nightly check -Zlints` when they
care about warnings.
This does involve a subtle change to `profile.rustflags` precedence but
its nightly and most likely people won't notice it? The benefit is its
in a location more like the rest of the rustflags.
The weakening of debuginfo for build script shouldn't turn debuginfo
to `DebugInfo::None`. That will result in not passing `-C debuginfo=0`
to rustc, leading to build artifact cache miss.
Fix redacting tokens in http debug.
Unfortunately it seems like #8222 didn't properly redact tokens when connecting to an http2 server. There were multiple problems:
* For some reason, curl changes the authorization header to be lowercase when using http2.
* Curl also logs the h2h3 lines separately with a different syntax.
This fixes it by checking for these additional cases.
This also adds a test, but it doesn't actually detect this problem because we don't have an http2 server handy. You can test this yourself by running `CARGO_LOG=trace CARGO_HTTP_DEBUG=true cargo publish --token a-unique-token --allow-dirty --no-verify`, and verifying the output does not contain the given token text.
do not try an exponential number of package names
re #11934, and as discussed in the cargo team meeting, this changes the strategy to "the original, all underscore, and all dashes".
I was excessively proud of the `hyphen_combination_num` based implementation when I came up with it. But it's always been a hack. I'm glad to be the one to remove it.
Optimize usage under rustup.
Closes#10986
This optimizes cargo when running under rustup to circumvent the rustup proxies. The rustup proxies introduce overhead that can make a noticeable difference.
The solution here is to identify if cargo would normally run `rustc` from PATH, and the current `rustc` in PATH points to something that looks like a rustup proxy (by comparing it to the `rustup` binary which is a hard-link to the proxy). If it detects this situation, then it looks for a binary in `$RUSTUP_HOME/toolchains/$TOOLCHAIN/bin/$TOOL`. If it finds the direct toolchain executable, then it uses that instead.
## Considerations
There have been some past attempts in the past to address this, but it has been a tricky problem to solve. This change has some risk because cargo is attempting to guess what the user and rustup wants, and it may guess wrong. Here are some considerations and risks for this:
* Setting `RUSTC` (as in https://github.com/rust-lang/rustup/pull/2958) isn't an option. This makes the `RUSTC` setting "sticky" through invocations of different toolchains, such as a cargo subcommand or build script which does something like `cargo +nightly build`.
* Changing `PATH` isn't an option, due to issues like https://github.com/rust-lang/rustup/issues/3036 where cargo subcommands would be unable to execute proxies (so things like `+toolchain` shorthands don't work).
* Setting other environment variables in rustup (as in https://github.com/rust-lang/rustup/pull/3207 which adds `RUSTUP_TOOLCHAIN_DIR` the path to the toolchain dir) comes with various complications, as there is risk that the environment variables could get out of sync with one another (like with `RUSTUP_TOOLCHAIN`), causing tools to break or become confused.
There was some consideration in that PR for adding protections by using an encoded environment variable that could be cross-checked, but I have concerns about the complexity of the solution.
We may want to go with this solution in the long run, but I would like to try a short term solution in this PR first to see how it turns out.
* This won't work for a `rustup-toolchain.toml` override with a [`path`](https://rust-lang.github.io/rustup/overrides.html#path) setting. Cargo will use the slow path in that case. In theory it could try to detect this situation, which may be an exercise for the future.
* Some build-scripts, proc-macros, or custom cargo subcommands may be doing unusual things that interfere with the assumptions made in this PR. For example, a custom subcommand could call a `cargo` executable that is not managed by rustup. Proc-macros may be executing cargo or rustc, assuming it will reach some particular toolchain. It can be difficult to predict what unusual ways cargo and rustc are being used. This PR (and its tests) tries to make extra sure that it is resilient even in unusual circumstances.
* The "dev" fallback in rustup can introduce some complications for some solutions to this problem. If a rustup toolchain does not have cargo, such as with a developer "toolchain link", then rustup will automatically call either the nightly, beta, or stable cargo if they are available. This PR should work correctly, since rustup sets the correct `RUSTUP_TOOLCHAIN` environment variable for the *actual* toolchain, not the one where cargo was executed from.
* Special care should be considered for dynamic linking. `LD_LIBRARY_PATH` (linux), `DYLD_LIBRARY_PATH` (macos), and `PATH` (windows) need to be carefully set so that `rustc` can find its shared libraries. Directly executing `rustc` has some risk that it will load the wrong shared libraries. There are some mitigations for this. macOS and Linux use rpath, and Windows looks in the same directory as `rustc.exe`. Also, rustup configures the dyld environment variables from the outer cargo. Finally, cargo also configures these (particularly for the deprecated compiler plugins).
* This shouldn't impact installations that don't use rustup.
* I've done a variety of testing on the big three platforms, but certainly nowhere exhaustive.
* One of many examples is making sure Clippy's development environment works correctly, which has special requirements for dynamic linking.
* There is risk about future rustup versions changing some assumptions made here. Some assumptions:
* It assumes that if `RUSTUP_TOOLCHAIN` is set, then the proxy *always* runs exactly that toolchain and no other. If this changes, cargo could execute the wrong version. Currently `RUSTUP_TOOLCHAIN` is the highest priority [toolchain override](https://rust-lang.github.io/rustup/overrides.html) and is fundamental to how toolchain selection becomes "sticky", so I think it is unlikely to change.
* It assumes rustup sets `RUSTUP_TOOLCHAIN` to a value that is exactly equal to the name of the toolchain in the `toolchains` directory. This works for user shorthands like `RUSTUP_TOOLCHAIN=nightly`, which gets converted to the full toolchain name. However, it does not work for `path` overrides (see above).
* It assumes the `toolchains` directory layout is always `$RUSTUP_HOME/toolchains/$TOOLCHAIN`. If this changes, then I think the only consequence is that cargo will go back to the slow path.
* It assumes downloading toolchains is not needed (since cargo running from the toolchain means it should already be downloaded).
* It assumes there is no other environment setup needed (such as the dyld paths mentioned above).
My hope is that if assumptions are no longer valid that the worst case is that cargo falls back to the slow path of running the proxy from PATH.
## Performance
This change won't affect the performance on Windows because rustup currently alters PATH to point to the toolchain directory. However, https://github.com/rust-lang/rustup/pull/3178 is attempting to remove that, so this PR will be required to avoid a performance penalty on Windows. That change is currently opt-in, and will likely take a long while to roll out since it won't be released until after the next release, and may be difficult to get sufficient testing.
I have done some rough performance testing on macOS, Windows, and Linux on a variety of different kinds of projects with different commands. The following attempts to summarize what I saw.
The timings are going to be heavily dependent on the system and the project. These are the values I get on my systems, but will likely be very different for everyone else.
The Windows tests were performed with a custom build of rustup with https://github.com/rust-lang/rustup/pull/3178 applied and enabled (stock rustup shows no change in performance as explained above).
The data is summarized in this spreadsheet: https://docs.google.com/spreadsheets/d/1zSvU1fQ0uSELxv3VqWmegGBhbLR-8_KUkyIzCIk21X0/edit?usp=sharing
`hello-world` has a particularly large impact of about 1.68 to 2.7x faster. However, a large portion of this overhead is related to running `rustc` at the start to discover its version and querying it for information. This is cached after the first run, so except for first-time builds, the effect isn't as noticeable. The "check with info" row is an benchmark that removes `target/debug/deps` but keeps the `.rustc_info.json` file.
Incremental builds are a bit more difficult to construct since it requires customizing the commands for each project. I only did an incremental test for cargo itself, running `touch src/cargo/lib.rs` and then `cargo check --lib`.
These measurements excluded the initial overhead of launching the rustup proxy to launch the initial cargo process. This was done just for simplicity, but it makes the test a little less characteristic of a typical usage, which will have some constant overhead for running the proxy.
These tests were done using [`hyperfine`](https://crates.io/crates/hyperfine) version 1.16.1. The macOS system was an M2 Max (12-thread). The Windows and Linux experiments were run on a AMD Ryzen Threadripper 2950X (32-thread). Rust 1.68.2 was used for testing. I can share the commands if people want to see them.
support for shallow clones and fetches with `gitoxide`
This PR makes it possible to enable shallow clones and fetches for git dependencies and crate indices independently with the `-Zgitoxide=fetch,shallow_deps` and `-Zgitoxide=fetch,shallow_index` respectively.
### Tasks
* [x] setup the shallow option when fetching, differentiated by 'registry' and 'git-dependency'
* [x] validate registries are cloned shallowly *and* fetched shallowly
* [x] validate git-dependencies are cloned shallowly *and* fetched shallowly
* [x] a test to show what happens if a shallow index is opened with `git2` (*it can open it and fetch like normal, no issues*)
* [x] assure that `git2` can safely operate on a shallow clone - we unshallow it beforehand, both for registries and git dependencies
* [x] assure git-deps with revisions are handled correctly (they should just not be shallow, and they should unshallow themselves if they are)
* [x] make sure shallow index clones aren't seen by older cargo's
* [x] make sure shallow git dependency clones aren't seen by older cargo's
* [x] shallow.lock test and more test-suite runs with shallow clones enabled for everything
* [x] release new version of `gix` with full shallow support and use it here
* [x] check why `shallow` files remain after unshallowing. Should they not rather be deleted if empty? - Yes, `git` does so as well, implemented [with this commit](2cd5054b0a)
* ~~see if it can be avoided to ever unshallow an existing `-shallow` clone by using the right location from the start. If not, test that we can go `shallow->unshallow->shallow` without a hitch.~~ Cannot happen anymore as it can predict the final location perfectly.
* [x] `Cargo.lock` files don't prevent shallow clones
* [x] assure all other tests work with shallow cloning enabled (or fix the ones that don't with regression protection)
* [x] can the 'split-brain' issue be solved for good?
### Review Notes
* there is a chance of 'split brain' in git-dependencies as the logic for determining whether the clone/fetch is shallow is repeated in two places. This isn't the case for registries though.
### Notes
* I am highlighting that this is the `gitoxide` version of shallow clones as the `git2` version [might soon be available](https://github.com/libgit2/libgit2/pull/6396) as well. Having that would be good as it would ensure interoperability remains intact.
* Maybe for when `git2` has been phased out, i.e. everything else is working, I think (unscientifically) there might be benefits in using worktrees for checkouts. Admittedly I don't know the history of why they weren't used in the first place. Also: `gitoxide` doesn't yet support local clones and might not have to if worktrees were used instead.
Doing so seems cleaner as there should be no logical difference between
shallow or not-shallow when fetching. We want a specific object, and should
get it with the refspec. `git` will assure we see all objects we need,
handling shallow-ness for us.
Note that one test needed adjustments due to the different mechanism used
when fetching local repositories, requiring more changes to properly 'break'
the submodule repo when `gitoxide` is used.
cargo-tree: Handle -e no-proc-macro when building the graph
### What does this PR try to resolve?
Makes `-e no-proc-macro` more useful when combined with `-i` or `-d`. Fixes#12030.
### How should we test and review this PR?
The new and existing tests should cover this, I hope!
### Additional information
Pruning proc-macro crates during graph construction is closer to how the edge-based filters work (`[no-]build` etc.), so even though `no-proc-macro` isn't technically filtering on edges, it's following a well-established code path.
This is an improvement over the previous version which would use unshallowing that effectively
makes a shallow repo *not* shallow.
Furthermore, we will now only fetch a single commit, each time we fetch, which should be faster
for the server as well as for the client.
We also make it possible to fetch individual commits that would be specified via Cargo.lock.
A couple of test expectations are adjusted accordingly.
Is this desirable behaviour? Unfortunately, there is no alternative
as adding shallow to an existing index most definitely breaks backwards
compatibility.
The implementation hinges on passing information about the kind of clone
and fetch to the `fetch()` method, which then configures the fetch accordingly.
Note that it doesn't differentiate between initial clones and fetches as
the shallow-ness of the repository is maintained nonetheless.
crates.io reads rust-version from the tarball directly, but we can include it in
the publish request for the sake of consistency for third-party registries.
The headers can significantly contribute to noise in the output,
drowning out the rest of the output. Most investigation will likely be
focused on the case where cargo completely fails to download, so this
only shows the full detail in the final error message.
