This was allowed when switching from `toml` v0.5 to `toml_edit` which
started allowing empty keys when parsing TOML.
This mirrors the change
we made for disallowing empty feature names in #12928.
fix(toml)!: Disallow `[lints]` in virtual workspaces
This was missed with the initial `[lints]` implementation.
While this is a breaking change, this is aligned with ones we've done in the past. A lot of times, we warn first. My hope is that isn't needed this time because
- It only exists virtual workspaces so they aren't published
- It is a nop to have this which is likely to be caught
- This is so new that the number of people using it, and likely running into this case, is quite low.
This was missed with the initial `[lints]` implementation.
While this is a breaking change, this is aligned with ones we've done in
the past. A lot of times, we warn first. My hope is that isn't needed
this time because
- It only exists virtual workspaces so they aren't published
- It is a nop to have this which is likely to be caught
- This is so new that the number of people using it, and likely running
into this case, is quite low.
Limit exported-private-dependencies lints to libraries
### What does this PR try to resolve?
Completed https://github.com/rust-lang/cargo/issues/13039.
This PR limit `exported-private-dependencies` lint in libraray `Target`.
### How should we test and review this PR?
Your can checkout out 2348ac2a20edf772495349d7911938251e343bf1 and run test, it will failed and then it will be passed in the commit 2348ac2a20edf772495349d7911938251e343bf1
### Additional information
In https://github.com/rust-lang/rust/blob/87e1447aa/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs#L856
when the remap result of `work_dir` is an empty string,
LLVM won't generate some symbols for root debuginfo node.
For example, `N_SO` and `N_OSO` on macOS,
or `DW_AT_comp_dir` on Linux when debuginfo is splitted.
Precisely, it is observed that when the `DIFile` of compile unit was
provied with an empty compilation `Directory` string,
LLVM would not emit those symbols for the root DI node.
This behavior is not desired,
resulting in corrupted debuginfo and degrading debugging experience.
This is might not be a bug of `--remap-path-prefix` in rustc,
since `-fdebug-prefix-map` in clang 16 could have the same result
(`DW_AT_comp_dir` is gone when `work_dir` is remapped to an empty string).
However, in gcc 12 `fdebug-prefix-map` will return an absolute work_dir
when an empty string occurs.
To not bother whether this needs to be fixed in rustc or not,
let's fix it by always appending an explicit `.`
when `--remap-path-prefix` remaps to relative workspace root
a.k.a. where rustc is invoking.
For more on gcc/clang remap options, see
https://reproducible-builds.org/docs/build-path/
With `gix-config` now being fixed, it will properly respect `GIT_CONFIG_NOSYSTEM`
both for system-wide configuration as well as for the git installation configuration.
That way, credential helpers provided by the git installation won't be called anymore,
which prevents them from 'somehow' emitting information to stderr.
The latter was previously disabled for credential helpers, and despite
everything^1 looking like it should work, it simply didn't.
[1]: https://github.com/rust-lang/cargo/pull/13117#issuecomment-1844881287
fix(toml): Disallow inheriting of dependency public status
### What does this PR try to resolve?
This is a step towards rust-lang/rust#44663. When discussing inheriting this field
for #13046, we realized that we should probably start by disallowing
inheritance. We can always add it later. imo the principle of what should
be inherited is what is truely common among dependencies. For example,
we don't allow removing features. Public should not be universally
applied and likely should be explicit so its not over-done, especially
since we can't (atm) lint for when a public dependency could be
non-public.
### How should we test and review this PR?
### Additional information
This reverts parts of #12817
re-enable previously disabled tests with Windows-specific fix
Related to #11821 for which this is a fix.
However, it's probably not yet the optimal solution, depending on how `stderr` of subprocesses should be handled.
### Tasks
* [x] try to fix the issue with an env var.
- Failure, as one warning remains that seems to originate from a C# HTTP client
* [x] figure out if `stderr` should be on or off by default - on by default like before, but now one can control it.
* [x] create a new `gix` release and use it here
### Review Notes
* Personally, I think `cargo` should keep `stderr` to be inherited so users can see potentially relevant warnings or errors provided by credential helpers. Thus this is still the default, but the tests that need it explicitly disabled `stderr` of credential helpers.
----
On Windows, `gix` will call the `git-credential-manager, but with
`stderr` set to `inherit` which makes any errors visible to the user,
just like `git` does.
```
1 1 Updating git repository `https://foo.bar/foo/bar`
2 +warning: auto-detection of host provider took too long (>2000ms)
3 +warning: see https://aka.ms/gcm/autodetect for more information.
4 +fatal: A task was canceled.
5 +warning: auto-detection of host provider took too long (>2000ms)
6 +warning: see https://aka.ms/gcm/autodetect for more information.`
````
This, however, isn't what's desirable in tests sometimes, nor may
it be desirable in Cargo.
For now, it seems easiest to disable this particular feature that
issues the warning messages, even though a future `gix` update
should allow to control what to do with `stderr`.
On Windows, `gix` will call the `git-credential-manager, but with
`stderr` set to `inherit` it makes any errors visible to the user,
just like `git` does.
```
1 1 Updating git repository `https://foo.bar/foo/bar`
2 +warning: auto-detection of host provider took too long (>2000ms)
3 +warning: see https://aka.ms/gcm/autodetect for more information.
4 +fatal: A task was canceled.
5 +warning: auto-detection of host provider took too long (>2000ms)
6 +warning: see https://aka.ms/gcm/autodetect for more information.`
````
This, however, isn't what's desirable in tests sometimes, nor may
it be desirable in Cargo.
In the latest version of `gix`, it's possible to control `stderr`
which is now set on a per-test basis.
Also note that for `cargo` as a whole the default didn't change,
and stderr of spawned helper programs will remain visible in the
enclosing terminal.
feat(spec): Extend PackageIdSpec with source kind + git ref for unambiguous specs
### What does this PR try to resolve?
This tries to add just enough information to Package ID Specs that we can be sure they are unambiguous. On its own thats important but this will unblock #12914 so we can have a user-facing ID that can be used with cargo's CLI.
More specifically, this adds
- `git+`, etc prefixes to the scheme
- These were previously stabilized for `cargo metadata`s source id urls
- Like with `SourceID`, this will allow `PackageIDSpec` to generate `directory+` and `local-registry+` prefixes but not parse them. I'm assuming that the layers where those are dealt with they won't appear here but I don't have enough experience with them
- git ref query strings for URLs
Things from `SourceID` that this does not include
- precise: this seems less related to matching (e.g. ignored for `impl Ord for SourceId`)
- canonical URL for git kinds: this could be nice for users but users aren't the target audience and we can switch to these later
Fixes#10256
### How should we test and review this PR?
Per-commit
### Additional information
This is a step towards #44663. When discussing inheriting this field
for #13046, we realized that we should probably start by disallowing
inheritance. We can always add it later. imo the principle of what should
be inherited is what is truely common among dependencies. For example,
we don't allow removing features. Public should not be universally
applied and likely should be explicit so its not over-done, especially
since we can't (atm) lint for when a public dependency could be
non-public.
This reverts parts of #12817
refactor(schemas): Pull out mod for proposed schemas package
Originally for #12801 we talked about a `cargo-util-manifest-schema` package
- `util` in the name to not clash with plugins
- manifest specific to keep the scope down
The problem is we have types that aren't manifest specific, like
- `PartialVersion` (currently slated for `cargo-util-semverext`)
- `RustVersion`
- `PackageIdSpec`
- `SourceKind` (soon)
Things get messy if we try to break things down into common packages. Instead, I think it'd be useful to have a schemas package that has mods for each type of schema, re-exporting what is needed.
Normally, componentizing your package by the layer in the stack is a recipe for pain.
I don't think that'll apply here because these are meant to be so low level.
The other big concern could be compile times. My hope is it won't be too bad.
So this moves the `util/toml` types into the module and we can add more in the future.
This is more robust than `thread::sleep`, ensuring
* foo is running before the first `cargo uninstall` call
* foo is stopped before the second `cargo uninstall` call
Include declared list of features in fingerprint for `-Zcheck-cfg`
This PR include the declared list of features in fingerprint for `-Zcheck-cfg` (#10554)
`--check-cfg` verifies that `#[cfg()]` attributes are valid in Rust code, which includes `--cfg features foo` definitions that came from `[features]` tables in `Cargo.toml`. For us to correctly re-check cfgs on feature changes,we need to include them in the fingerprint.
For example, if a user deletes an entry from `[features]`, they should then get warnings about any `#[cfg()]` attributes left in the code. Without this change, that won't happen. With this change, it now does.
Otherwise changing (add, removing, ...) features from the `[features]`
table would not cause rustc to be called with the new `--check-cfg`,
showing potentially outdated warnings.
Originally for #12801 we talked about a `cargo-util-manifest-schema` package
- `util` in the name to not clash with plugins
- manifest specific to keep the scope down
The problem is we have types that aren't manifest specific, like
- `PartialVersion` (currently slated for `cargo-util-semverext`)
- `RustVersion`
- `PackageIdSpec`
- `SourceKind` (soon)
Things get messy if we try to break things down into common packages.
Instead, I think it'd be useful to have a schemas package that has mods
for each type of schema, re-exporting what is needed.
Normally, componentizing your package by the layer in the stack is a
recipe for pain.
I don't think that'll apply here because these are meant to be so low
level.
The other big concern could be compile times. My hope is it won't be
too bad.
So this moves the `util/toml` types into the module and we can add more
in the future.
Have cargo add --optional <dep> create a <dep> = "dep:<dep> feature
### What does this PR try to resolve?
`cargo add --optional <dep>` would create a `<dep> = "dep:<dep>` feature iff
- `rust-version` is unset or is new enough for the syntax
- `dep:<dep>` doesn't already exist
Fixes https://github.com/rust-lang/cargo/issues/11010
### How should we test and review this PR?
As the `dep:` syntax is only available starting with Rust 1.60. this pr maintains the previous usage convention in the earlier version.
run
```shell
cargo add --optional <dep>
```
with different rust-version to verify.
Add `--public` for `cargo add`
## What does this PR try to resolve?
Complete https://github.com/rust-lang/cargo/issues/13037
This PR want to add `--public/--no public` flag for `cargo add`
Note: this assumes we'll remove workspace inheritance support for `public` as it sounds like we'll be reverting it https://github.com/rust-lang/rust/issues/44663#issuecomment-1826474620. If we decide to keep workspace inheritance, we'll need to come back and update this.
## How should we test and review this PR?
Most of Code were reference `cargo add --optional`, So can reviewed the new code based on the part of `optional` code.
The new testcases were origin from the `cargo add --optional` part.
- `public` testcase：there is no dependencies and will be add `public` dependencies.
- `no_public` testcase: there is no dependencies and will be add `no_public` dependencies.
- `overwrite_public` testcase: the dependencies already exists but will be overwrite with `public`.
- `overwrite_no_public` testcase: the dependencies already exists but will be overwrite with `no_public`.
- `overwrite_public_with_no_public` testcase: the dependencies already marked as `no_public` and will be overwrite with `public`.
- `overwrite_no_public_with_public` testcase: the dependencies already marked as `public` and will be overwrite with `no_public`.
Fixed uninstall a running binary failed on Windows
### What does this PR try to resolve?
Fixes https://github.com/rust-lang/cargo/issues/3364
The problem reproduce when you try to uninstall a running binary and it will failed on Windows, this is because that cargo had already remove the installed binary tracker information in disk, and next to remove the running binary but it is not allowed in Windows. And to the worst, you can not uninstall the binary already and only to reinstall it by the `--force` flag.
### How to solve the issue?
This PR try to make the uninstall a binary more reasonable that only after remove the binary sucessfully then remove the tracker information on binary and make the remove binary one by one.
### How should we test and review this PR?
Add testcase 0fd4fd357b
- install the `foo`
- run `foo` in another thread.
- try to uninstall running `foo` and only failed in Windows.
- wait the `foo` finish, uninstall `foo`
- install the `foo`
### Additional information
Don't filter on workspace members when scraping doc examples
Fixes https://github.com/rust-lang/cargo/issues/13074.
I confirmed locally that it fixed the issue in docs.rs.
cc `@willcrichton`
r? `@epage`
Fixes error count display is different when there's only one error left
### What does this PR try to resolve?
When there's only 1 error left, the number 1 appears in the output so that it scans the same as the output when there's more than 1 error, so:
```
error: could not compile `crate` (lib test) due to 1 previous error
```
instead of the current:
```
error: could not compile `crate` (lib test) due to a previous error
```
Fixes#12390
rustc related PR [#114759](https://github.com/rust-lang/rust/pull/114759)
fix: reorder `--remap-path-prefix` flags for `-Zbuild-std`
### What does this PR try to resolve?
Order of `--remap-path-prefix` flags is important for `-Zbuild-std`.
We want to show `/rustc/<hash>/library/std` instead of `std-0.0.0`.
Fixesrust-lang/rust#117839
### How should we test and review this PR?
Follow the steps in rust-lang/rust#117839, or run
```
CARGO_RUN_BUILD_STD_TESTS=true cargo +nightly t --test build-std
```
to verify.
Handle $message_type in JSON diagnostics
### What does this PR try to resolve?
Unblocks https://github.com/rust-lang/rust/pull/115691.
Without this change, Cargo's testsuite fails in `doc::doc_message_format` and `metabuild::metabuild_failed_build_json`.
### How should we test and review this PR?
Tested with and without https://github.com/rust-lang/rust/pull/115691.
In Cargo repo: `cargo test --test testsuite`
In Rust repo: `x.py test src/tools/cargo` (separately on master and $message_type PR)
This is an alternative to #12158's `CARGO_WORKSPACE_DIR` that was
implementing the solution to #3946 that previously discussed in the
cargo team meeting.
`CARGO_WORKSPACE_DIR` is a bit awkward to document / describe because
its the effective workspace directory of the thing being built.
If the thing being built doesn't have a workspace, it falls back to
`CARGO_MANIFEST_DIR`.
It would also be hard to take into account what the
`CARGO_WORKSPACE_DIR` would be for path dependencies into foreign
workspaces *and* it wouldn't solve the problem the user is having.
What the user really wants is the CWD of rustc when it is invoked.
This is much simpler to describe and is accurate when using a path
dependency to a foreign package.
Because the CWD is a much simpler mechanism to talk about, I figured we
could diverge from our prior consensus and make it always present,
rather than limiting it to tests.
Remaining work for #3946: get this stabilized
Exited with hard error when custom build file no existence or not in package
## What does this PR try to resolve?
Fixed https://github.com/rust-lang/cargo/issues/11383
## How should we test and review this PR?
Add test `build_script_outside_pkg_root`, this will check `custom_build.rs` existence and whether in the package root, if not then exited with a hard error
## Additional information
The code just handle the `custom build` target that i know how to test it. Other target type is skipped.
If the only path is a loop then counted as the shortest path.
This is a fix for #12941
This graph data structure is used to store dependency DAGs. Where each edge represents a dependency from a package to the package that fulfilled the dependency. Different parts of the resolver store this data in opposite directions, sometimes packages point at the things that depend on them other times packages point to the parents that required them. Error messages often need to report on why a package is in the graph, either by walking up toward parents or down toward children depending on how this graph is stored. #12678 unified the two different walking implementations, and replace them with a breadth first search so as to find the shortest path. This code ignored when edge pointed at a package that had already been reached, because that generally describes a longer path to an existing package.
Unfortunately, when I said this was a DAG that was a simplification. There can be cycles introduced as dev-dependencies. The existing code would reasonably ignore the cycles figuring that if we continue searching we would eventually find the root package (a package that nothing depended on). Missing the possibility that the root package created the cycle.
Now we search through the entire graph looking for a root package. If we do not find a root package we report the path to the last package we processed.
fix(resolver): Prefer MSRV, rather than ignore incompatible
### What does this PR try to resolve?
This is another experiment for #9930.
Comparing preferring over exclusively using MSRV compatible:
Benefits
- Better error messages
- `--ignore-rust-version` is implicitly sticky
Downsides
- Can't backtrack for MSRV compatible version
- Still requires workspace-wide MSRV (compared to our desired end state of declaring MSRV as yet another dependency)
### How should we test and review this PR?
### Additional information
Note: `--ignore-rust-version` is not yet implemented for the resolver.
This builds on #12930
This is another experiment for #9930.
Comparing preferring over exclusively using MSRV compatible:
Benefits
- Better error messages
- `--ignore-rust-version` is implicitly sticky
Downsides
- Can't backtrack for MSRV compatible version
- Still requires workspace-wide MSRV (compared to our desired end state of declaring MSRV as yet another dependency)
This builds on #12930
Before, when running `cargo update <member>`, we'd not reuse the
previous resolve result and when the resolver started walking into the
dependencies, it would do a git fetch.
Now, we won't even try to resolve the workspace members and so we won't
look at those dependencies and do git fetch.
This will make `cargo update <workspace-member>`
match `cargo update --workspace`.
I considered whether there were other ways of handling this but I
figured aiming for consistency in approaches was the best way.
We can investigate improving those approaches separately.
There are other discrepancies in the different code paths (handling of
patches, adding sources) but I'm deferring looking over those.
Between this and #12602, this should finnally resolve#12599.
Fixes#12599
Only filter out target if its in the package root
### What does this PR try to resolve?
Only filter out target if its in the package root. Fixed https://github.com/rust-lang/cargo/issues/12790
### How should we test and review this PR?
Add two testcase in tests/testsuite/package.rs for this PR.
- `include_files_called_target_project` testcase test the logic for none git repository. By the way, our PR was based on git repository, so this testcase was a little irrelevant.
- `include_files_called_target_git` testcase was made for git repository. There are two cases here, one is the target in the uncommitted state, at this time should not be included, and one is the target in the committed state, at this time should be included
### Additional information
This fixes an issue where `--quiet` doesn't work with commands that have
subcommands. This is because `config_configure` only looks at the global
and top-level subcommand, and not deeper subcommands. The issue was that
`--quiet` was not defined as a global flag. This was changed in
https://github.com/rust-lang/cargo/pull/6358 in order to give a better
help message for `cargo test --quiet`. I don't remember if clap just
didn't support overriding at the time, or if we just didn't know how it
worked. Anyways, it seems to work to override it now, so I think it
should be fine to mark it as global.
This should bring in `--quiet` more in-line with how `--verbose` works.
This means that `--quiet` is now accepted with `cargo report`,
`cargo help`, and `cargo config`.
This also fixes `--quiet` with `cargo clean gc`.
This should also help with supporting `--quiet` with the new `cargo
owner` subcommands being added in
https://github.com/rust-lang/cargo/pull/11879.
Fixes#12957
This was requested to separate the interaction of `cargo clean` and the
cleaning of global cache data, and to minimize the impact of this
initial implementation.
This adds a garbage collector which will remove old files from cargo's
global cache.
A general overview of the changes here:
- `cargo::core::global_cache_tracker` contains the `GlobalCacheTracker`
which handles the interface to a sqlite database which stores
timestamps of the last time a file was used.
- `DeferredGlobalLastUse` is a type that implements an optimization for
collecting last-use timestamps so that they can be flushed to disk all
at once.
- `cargo::core::gc` contains the `Gc` type which is the interface for
performing garbage collection. It coordinates with the
`GlobalCacheTracker` for determining what to delete.
- Garbage collection can either be automatic or manual. The automatic
garbage collection supports some config options for defining when
it runs and how much it deletes.
- Manual garbage collection can be performed via options to `cargo
clean`.
- `cargo clean` uses the new package cache locking system to coordinate
access to the package cache to prevent interference with other cargo
commands running concurrently.
For `cargo install` we'll now show a more specific parse error for
semver, much like other parts of cargo.
This came out of my work on #12801. I was looking at what might be
appropriate to put in a `cargo-util-semver` crate and realized we have
the `ToSemver` trait that exists but doesn't do much, so I dropped it.
refactor(toml): Pull out the schema
### What does this PR try to resolve?
On its own, this PR is a net negative for readability / complexity. It moves all of the serde types related to manifest from `toml/mod.rs` to `toml/schema.rs`, leaving a lot of the functions and some trait implementations back in `toml/mod.rs` (some basic functions that made sense for the type on their own were also moved).
So why do this? This is an ooch towards having the schema broken out into a separate package (#12801). To do this, we need to
- Identify what all types need to be put in the package. This refactor highlights the dependence on `RustVersion` and `PackageIdSpec`
- Highlights what functionality we need to find a new home for
Follow up PRs would
- Find better homes for the logic in `toml/mod.rs`, moving us away from having `impl schema::Type` blocks.
- Pull out a `src/cargo/util_semver` package to own `PartialVersion` (at least) as prep for making a `cargo-util-semver` package
- Move `RustVersion` to `manifest-toml/schema.rs`, deciding what functionality needs to move with the type
- Move or copy `PackageIdSpec` into `manfest-toml/schema.rs`, deciding what functionality remain in `core/` and what moves over
- Move `toml/schema.rs` to `src/cargo/util_schema`
- Actually make `cargo-util-semver` and `cargo-util-manifest-schema` packages
So why do this now? This is a big change! By being incremental
- Reduce churn for me and others
- Be easier to review
- Collect feedback as we go on the whole plan to avoid more painful changes later
We *can* back this out if needed but the further we go, the more painful it will be.
### How should we test and review this PR?
### Additional information
fix(cli): Clarify --test is for targets, not test functions
We already refer to test targets as "test targets" instead of "tests" in `--test` but not `--tests` or in the error output. This makes it uniformly refer to them as "test targets", making it clearer that these aren't test functions.
Fixes#7864
We already refer to test targets as "test targets" instead of "tests" in
`--test` but not `--tests` or in the error output. This makes it
uniformly refer to them as "test targets", making it clearer that these
aren't test functions.
Fixes#7864
Add new packages to [workspace.members] automatically
### What does this PR try to resolve?
If a new package is created in a workspace, this change adds the package's path to the workspace's members list automatically.
It doesn't add the package to the list if the path is in the workspace's exclude list, or if the members list doesn't exist already. I noticed that a `cargo_new` test broke if I create the members list when it doesn't exist. This is because the workspace's manifest can be used for package templating. I think it's better to not break that use case.
Fixes#6378
### How should we test and review this PR?
I've included tests in the `cargo_new` suite.
When a user runs `cargo new` or `cargo init` within a workspace, Cargo will automatically add the new package to the members list in the workspace if necessary. The heuristic to add the new package is as follows:
- If there is no `members` list in the workspace yet, a new `members` list is created.
- If there is an `exclude` statement, Cargo checks if the new package should be excluded. If it doesn't match the `exclude` list, the package is added to the `members` list.
- If there is a glob expression in the `members` list that matches the new package, the package is not added to the `members` list.
- If the existent `members` list is sorted, Cargo tries to preserve the ordering when it adds the new package.
This change doesn't try to format the resulting `members` list in any way, leaving the formatting decissions to the user.
Signed-off-by: David Calavera <david.calavera@gmail.com>
To have a separate manifest API (#12801), we can't rely on interning
because it might be used in longer-lifetime applications.
I consulted https://github.com/rosetta-rs/string-rosetta-rs when
deciding on what string type to use for performance.
Originally, I hoped to entirely replacing string interning. For that, I
was looking at `arcstr` as it had a fast equality operator. However,
that is only helpful so long as the two strings we are comparing came
from the original source. Unsure how likely that is to happen (and
daunted by converting all of the `Copy`s into `Clone`s), I decided to
scale back.
Concerned about all of the small allocations when parsing a manifest, I
assumed I'd need a string type with small-string optimizations, like
`hipstr`, `compact_str`, `flexstr`, and `ecow`.
The first three give us more wiggle room and `hipstr` was the fastest of
them, so I went with that.
I then double checked macro benchmarks, and realized `hipstr` made no
difference and switched to `String` to keep things simple / with lower
dependencies.
When doing this, I had created a type alias (`TomlStr`) for the string
type so I could more easily swap it out if needed
(and not have to always write out a lifetime).
With just using `String`, I went ahead and dropped that.
I had problems getting the cargo benchmarks running, so I did a quick
and dirty benchmark that is end-to-end, covering fresh builds, clean
builds, and resolution. I ran these against a fresh clone of cargo's
code base.
```console
$ ../dump/cargo-12801-bench.rs run
Finished dev [unoptimized + debuginfo] target(s) in 0.07s
Running `target/debug/cargo -Zscript -Zmsrv-policy ../dump/cargo-12801-bench.rs run`
warning: `package.edition` is unspecified, defaulting to `2021`
Finished dev [unoptimized + debuginfo] target(s) in 0.04s
Running `/home/epage/.cargo/target/0a/7f4c1ab500f045/debug/cargo-12801-bench run`
$ hyperfine "../cargo-old check" "../cargo-new check"
Benchmark 1: ../cargo-old check
Time (mean ± σ): 119.3 ms ± 3.2 ms [User: 98.6 ms, System: 20.3 ms]
Range (min … max): 115.6 ms … 124.3 ms 24 runs
Benchmark 2: ../cargo-new check
Time (mean ± σ): 119.4 ms ± 2.4 ms [User: 98.0 ms, System: 21.1 ms]
Range (min … max): 115.7 ms … 123.6 ms 24 runs
Summary
../cargo-old check ran
1.00 ± 0.03 times faster than ../cargo-new check
$ hyperfine --prepare "cargo clean" "../cargo-old check" "../cargo-new check"
Benchmark 1: ../cargo-old check
Time (mean ± σ): 20.249 s ± 0.392 s [User: 157.719 s, System: 22.771 s]
Range (min … max): 19.605 s … 21.123 s 10 runs
Benchmark 2: ../cargo-new check
Time (mean ± σ): 20.123 s ± 0.212 s [User: 156.156 s, System: 22.325 s]
Range (min … max): 19.764 s … 20.420 s 10 runs
Summary
../cargo-new check ran
1.01 ± 0.02 times faster than ../cargo-old check
$ hyperfine --prepare "cargo clean && rm -f Cargo.lock" "../cargo-old check" "../cargo-new check"
Benchmark 1: ../cargo-old check
Time (mean ± σ): 21.105 s ± 0.465 s [User: 156.482 s, System: 22.799 s]
Range (min … max): 20.156 s … 22.010 s 10 runs
Benchmark 2: ../cargo-new check
Time (mean ± σ): 21.358 s ± 0.538 s [User: 156.187 s, System: 22.979 s]
Range (min … max): 20.703 s … 22.462 s 10 runs
Summary
../cargo-old check ran
1.01 ± 0.03 times faster than ../cargo-new check
```
I've wanted something like this myself. I dislike using `--open`
because I tend to move up to re-run my `cargo doc` run but then have to
edit it to remove `--open`.
Also makes it annoying when opening docs when `cargo doc` is wrapped by
a tool like `make`.
This was previously attempted in #5592:
- Unlike the request in #5562, this aligns with #5592 in always printing
rather than using a flag as this seems generally useful
- Unlike #5592, this prints as an alternative to "Opening" to keep
things light
- Unlike #5592, this prints afterwards as the link is only valid then
Fixes#5562
feat(toml): Allow version-less manifests
### What does this PR try to resolve?
Expected behavior with this PR:
- `package.version` defaults to `0.0.0`
- `package.publish` is defaulted to `version.is_some()`
This also updates "cargo script" to rely on this new behavior.
My motivation is to find ways to close the gap between "cargo script" and `Cargo.toml`. With "cargo script", we want to allow people to only write however much of a manifest is directly needed for the work they are doing (which includes having no manifest). Each difference between "cargo script" and `Cargo.toml` is a cost we have to pay in our documentation and a hurdle in a users understanding of what is happening.
There has been other interest in this which I also find of interest (from #9829):
- Lower boilerplate, whether for [cargo xtasks](https://github.com/matklad/cargo-xtask), nested packages (rust-lang/rfcs#3452), etc
- Unmet expectations from users because this field is primarily targeted at registry operations when they want it for their marketing version (#6583).
- Make "unpublished" packages stand out
This then unblocks unifying `package.publish` by making the field's default based on the presence of a version as inspired by the proposal in #9829. Without this change, we were trading one form of boilerplate (`version = "0.0.0"`) for another (`publish = false`).
Fixes#9829Fixes#12690Fixes#6153
### How should we test and review this PR?
The initial commit has test cases I thought would be relevant for this change and you can see how each commit affects those or existing test cases. Would definitely be interested in hearing of other troubling cases to test
Implementation wise, I made `MaybeWorkspaceVersion` deserializer trim spaces so I could more easily handle the field being an `Option`. This is in its own commit.
### Additional information
Alternatives considered
- Making the default version "stand out more" with it being something like `0.0.0+HEAD`. The extra noise didn't seem worth it and people would contend over what the metadata field *should be*
- Make the default version the lowest version possible (`0.0.0-0`?). Unsure if this will ever really matter especially since you can't publish
- Defer defaulting `package.publish` and instead error
- Further unifying more fields made this too compelling for me :)
- Put this behind `-Zscript` and make it a part of rust-lang/rfcs#3502
- Having an affect outside of that RFC, I wanted to make sure this got the attention it deserved rather than getting lost in the noise of a large RFC.
- Don't just default the version but make packages versionless
- I extended the concept of versionless to `PackageId`'s internals and saw no observable difference
- I then started to examine the idea of version being optional everywhere (via `PackageId`s API) and ... things got messy especially when starting to look at the resolver. This would have also added a lot of error checks / asserts for "the version must be set here". Overall, the gains seemed questionable and the cost high, so I held off.
