feat(cli): Add '-n' to dry-run
This came from #12638 and my many small frustrations from wanting to use `-n` and not being able to.
We do not have any existing `-n` flags for this to be confused with.
I would wager that `-n` is such an entrenched short flag in build tools that it would not make sense for us to use it with any other flag.
For a survey of where `-n` is used as a short, see https://www.gnu.org/prep/standards/html_node/Option-Table.html#Option-Table
feat: stabilize credential-process and registry-auth
Stabilization PR for `registry-auth` and `credential-process`.
Tracking approval of this stabilization is done in the via the FCP in [#8933](https://github.com/rust-lang/cargo/issues/8933#issuecomment-1711990123). This PR is here to help reviewers of the FCP.
* Stabilizes `registry-auth` and `credential-process`
* Makes authenticated registries require a credential provider
* Adds stable documentation for credential providers and authenticated registries
Closes#8933Closes#10474
This came from #12638 and my many small frustrations from wanting to use
`-n` and not being able to.
We do not have any existing `-n` flags for this to be confused with.
I would wager that `-n` is such an entrenched short flag in build tools that it would
not make sense for us to use it with any other flag.
For a survey of where `-n` is used as a short, see
https://www.gnu.org/prep/standards/html_node/Option-Table.html#Option-Table
feat(help): Add styling to help output
### What does this PR try to resolve?
Try to make `--help` output easier to parse by using terminal styling
Screenshots:
![Screenshot from 2023-09-06 09-57-11](https://github.com/rust-lang/cargo/assets/60961/61069af4-ef05-40ad-9240-fedea44d4c71)
![Screenshot from 2023-09-06 09-57-21](https://github.com/rust-lang/cargo/assets/60961/d2e69024-42aa-47c0-ad0f-24e43551b8db)
![Screenshot from 2023-09-06 09-57-36](https://github.com/rust-lang/cargo/assets/60961/e3d895e2-745f-48c6-9e84-d6fb67198d6d)
*(`nargo` is my shell script wrapping `cargo run --manifest-path cargo/Cargo.toml`)*
### How should we test and review this PR?
At this time, the only styling snapshotting library I know of is a pain to use, so testing this requires manually running the commands which I did. Screenshots are included for easier evaluation of the general idea.
Snapshotting of the plain text output ensures we don't have accidental formatting regressions from this change since the formatting isn't as obvious from looking at the code.
### Additional information
Traditionally, cargo has disabled clap's styled output. My assumed
reason is that cargo mixes custom help output with auto-generated and
you couldn't previously make it all styled.
Clap 4.2 allowed users to pass in strings styled using ANSI escape
codes, allowing us to pass in styled text that matches clap, unblocking this. In clap
4.4.1, clap gained the ability for the user to override the style.
In this PR, I decided to use the new 4.4.1 feature to style clap's
output to match the rest of cargo's output. Alternatively, we could use
a more subdue style that clap uses by default.
I used the `color-print` crate to allow something almost html-like for styling `&static str`. Alternatively, we could directly embed the ANSI escape codes harder to get write, harder to inspect), or we could do the styling at runtime and enable the `string` feature in clap.
I decided to *not* style `Arg::help` messages because
- It might be distracting to have the descriptions lit up like a
christmas tree
- It is a lot more work
The one exception I made was for `--list` since it is for a
psuedo-command (`...`) and I wanted to intentionally draw attention to
it.
#12593 made styling of `cargo -h` cleaner imo.
#12592 and #12594 were improvements I noticed while doing this.
Ues strip_prefix for cleaner code
### What does this PR try to resolve?
In https://github.com/rust-lang/cargo/pull/12629#pullrequestreview-1614154046 Ed pointed out how much cleaner the code can be using `strip_prefix`, so I found a bunch more places where we should be using it.
### How should we test and review this PR?
Internal refactor and test still pass.
fix: improve warning for both token & credential-provider
Cargo issues a warning when both a `credential-provider` and a `token` are configured for a registry.
This change removes the warning if the `credential-provider` is `cargo:token` since that *will* use the token. The warning message text is also tweaked to include the name of the `credential-provider` that's overriding the token.
fix(resolver): Make resolver behavior independent of package order
This address one of the problems mentioned in #12599
The intent behind the `path_pkg` check is to make sure we update
workspace members in the lockfile if their version number changed.
In this case, we don't need to recursively walk, because the change
doesn't affect dependencies. However, we also shouldn't *prevent*
recursive walks which is what we are doing today, both for packages
marked to keep and for packages that have been "poisoned". So we fix
this by moving that call after all recursive adds are complete so as to
not interfere with them.
This should not affect `Cargo.lock` at rest, so no upgrade compatibility concerns.
This just allows more packages to be considered available to change which can prevent unclear failures.
The main case I can think of that this does something "undesirable" is when wanting to prevent another "bug" from manifesting: the updating of git dependencies when updating workspace members (#12599). I think I'm ok with that as that needs to be looked into separately.
fix(help): Provide better commands heading for styling
In working on #12578, I felt it would be weird to style the entire statement about commands but it also felt weird to not style it. So this change explores an alternatively way of communicating the information.
This addresses the ordering issue of for one of the problems from #12599.
The intent behind the `path_pkg` check is to make sure we update
workspace members in the lockfile if their version number changed.
In this case, we don't need to recursively walk, because the change
doesn't affect dependencies. However, we also shouldn't *prevent*
recursive walks which is what we are doing today, both for packages
marked to keep and for packages that have been "poisoned". So we fix
this by moving that call after all recursive adds are complete so as to
not interfere with them.
In working on #12578, I felt it would be weird to style the entire
statement about commands but it also felt weird to not style it. So
this change explores an alternatively way of communicating the
information.
fix: add error for unsupported credential provider version
Cargo currently ignores the version in the `CredentialHello` message, and proceeds to use version `1` regardless of what the credential provider claims it can support.
This change does the following:
* Adds a new error if Cargo doesn't support any of the supported protocol versions offered by the provider.
* Kills the credential provider subprocess if it fails. This prevents it from hanging or printing spurious errors such as "broken pipe" when it's attempting to read the next JSON message.
* Adds a new test for an unsupported credential provider protocol.
In working on #12578, I'm focusing on each help string to decide how it
should be handled and I noticed this. It feels weird to explain
something in terms of another command's CLI, so I took `rustc --help`s
message and added `rustc` to clarify it.
Looking back, the flag was added in #2551 with the message we have
today. Nothing seems to really be said about it.
In reflecting on this, I'm not 100% convinced and am open to other
opinions.
fix(help): Remove redundant information from new/init
Auditing all of the `--help` in prep for #12578 and noticed that we list the VCS information twice, once on our end and once by clap.
fix(lints): Fail when overriding inherited lints
### What does this PR try to resolve?
Overriding of inherited lints was reserved for the future but as pointed out in https://github.com/rust-lang/cargo/issues/12115#issuecomment-1695293006, we aren't failing on these when we should but silently ignoring the overrides.
This turns it into a hard error.
In fixing this, I had to add a `#[serde(expecting)]` attribute to maintain behavior on an error case (otherwise it would say "expecting struct WorkspaceLints"). Since this drew the error message to my attention, I also tweaked it to make it more specific.
### How should we test and review this PR?
Commits are broken down by the relevant tests and fixes to make the intended behavior changes obvious.
cargo install: suggest --git when package name is url
### What does this PR try to resolve?
Improve the error message when specifying a URL for a package name in `cargo install`.
Fixes#10485
### How should we test and review this PR?
Just cargo test and trying a common case like `cargo install https://github.com/rust-lang/cargo`
### Additional information
I found this PR after finishing this one: #10522
But it seems have a larger scope to refactor some of the related code.
Perhaps this one would be easier to merge and that one could focus on the refactor, otherwise sorry for the noise and feel free to close.
I assume the reason these aren't all individual tests is test-time but
if we divide by success/failure, I'll need to duplicate things to handle
partial versions.
Overall, I feel like this makes the tests make more sense.
When working on cargo-upgrade, I found the meaning of `--aggressive`
confusing and named it `--recursive` there.
Renaming this in `cargo update` (with a backwards compatible alias) was
referenced in #12425.
refactor: Pull out cargo-add MSRV code for reuse
### What does this PR try to resolve?
#12078 added MSRV code in `cargo add`. Our assumption when writing it is that we'd need to generalize the code before reusing it in other places, like `cargo install`. This PR focused purely on that refactor because I'm hopeful it will be useful for other work I'm doing. Despite not having a user for this yet, I think the `cargo install` case is inevitable and I feel this does a bit to clean up MSRV related code by using a more specific type everywhere.
### How should we test and review this PR?
Each commit gradually progresses things along
fix(toml): Improve parse errors
### What does this PR try to resolve?
When we adopted `toml_edit`, we got TOML syntax errors that showed the context for where the error occurred. However, the work was not done to extend this to semantic errors reported by serde.
This updates `Cargo.toml` and `Cargo.lock` code to provide that context on semantic errors. `config.toml` is not done because the schema is decentralized.
In theory, this will also improve performance because we aren't having to allocate a lot of intermediate data to then throw away for every `Cargo.toml` we read.
### How should we test and review this PR?
Check by commit to see this change gradually.
- The `package.cargo-features` change was made to drop out dependence on `toml::Table` so we could do the direct deserialization
Create dedicated unstable flag for asymmetric-token
Asymmetric tokens are gated by `-Zcredential-process`. Since we're considering stabilizing that soon, this moves asymmetric token support to have its own unstable flag.
It was previously gated by `-Zregistry-auth`, and some of the docs were not updated when it moved.
r? `@Eh2406`
Before, we'd render the source for TOML syntax errors but not semantic errors.
Now we render for both.
Originally I changed `parse_document` to returned `T: DeserializeOwned`
but that adds an extra "could not parse TOML" which is both redundant
and makes it sound like its a syntax issue.
Generally, cargo avoids positional arguments. Mostly for the commands
that might forward arguments to another command, like `cargo test`.
It also allows some flexibility in turning flags into options.
For `cargo add` and `cargo remove`, we decided to accept positionals
because the motivations didn't seem to apply as much (similar to `cargo
install`).
This applies the pattern to `cargo update` as well which is in the same
category of commands as `cargo add` and `cargo remove`.
As for `--help` formatting, I'm mixed on whether `[SPEC]...` should be at the top like
other positionals or should be relegated to "Package selection". I went
with the latter mostly to make it easier to visualize the less common
choice.
Switching to a positional for `cargo update` (while keeping `-p` for
backwards compatibility) was referenced in #12425.
feat(resolver): **Very** preliminary MSRV resolver support
### What does this PR try to resolve?
A bare bones implementation of an MSRV resolver that is good enough for people running on nightly when they really need it but is not ready for general use.
Current limitations
- Does not honor `--ignore-version`
- Gives terrible error messages
- Nothing is done yet regarding `cargo install`
- Doesn't inform the user when choosing non-latest
These will be noted in #9930 on merge.
Implementation wise, this is yet another hack (sorry `@Eh2406).` Our expectation to get this GA is to refactor the resolver to make the cargo/resolver boundary look a little more like the cargo/pubgrub boundary so we can better control policy without any of these hacks which will also make having all of the policy we need for this easier to maintain.
This is a part of #9930
### How should we test and review this PR?
Per commit
Improve deserialization errors of untagged enums
### What does this PR try to resolve?
```toml
# .cargo/config.toml
[http]
ssl-version.min = false
```
**Before:**
```console
$ cargo check
error: data did not match any variant of untagged enum SslVersionConfig
```
**After:**
```console
$ cargo check
error: error in /path/to/.cargo/config.toml: could not load config key `http.ssl-version`
Caused by:
error in /path/to/.cargo/config.toml: `http.ssl-version.min` expected a string, but found a boolean
```
### How should we test and review this PR?
The first commit adds tests showing the pre-existing error messages — mostly just _"data did not match any variant of untagged enum T"_ with no location information. The second commit replaces all `#[derive(Deserialize)] #[serde(untagged)]` with Deserialize impls based on https://docs.rs/serde-untagged/0.1, showing the effect on the error messages.
Tested with `cargo test`, and by handwriting some bad .cargo/config.toml files and looking at the error produced by `rust-lang/cargo/target/release/cargo check`.
config: merge lists in precedence order
When merging configuration lists, the current order does not match the expected precedence. This makes merged lists follow precedence order, with higher precedence items merged later in lists.
When a list in configuration exists in multiple places, Cargo merges the lists together. The ordering of this merging is unexpected and does not follow the precedence rules that non-list configuration uses.
The current merging order appears to be:
* project-specific `config.toml`
* global `config.toml`
* command-line (`--config`)
* environment variable (`CARGO_*`)
This PR changes the order to follow the precedence rules with higher precedence configuration merging later in the lists.
* global `config.toml`
* project-specific `config.toml`
* environment variable (`CARGO_*`)
* command-line (`--config`)
This aligns with config such as `build.rustflags` where later flags take precedence over earlier ones.
Since `--config` is relatively new, it's unlikely to cause too much breakage by making it come after environment variables.
Switching global and project-specific ordering is more likely to cause breakage, since it's been around longer (reported as an issue in #8128). Projects relying on global configuration flags (in `$CARGO_HOME\config.toml` or in `.cargo/config.toml` further from the project) being merged first in lists will be broken.
For most uses of merged lists (such as `build.rustflags`), if the flags do not conflict with each other, there will be no impact.
Fixes#12506Fixes#8128
login: allow passing additional args to provider
As part of moving asymmetric token support to a credential provider in #12334, support for passing `--key-subject` to `cargo login` was removed.
This change allows passing additional arguments to credential providers when running `cargo login`. For example:
`cargo login -- --key-subject foo`.
The asymmetric token provider (`cargo:paseto`) is updated to take advantage of this and re-enables setting `--key-subject` from `cargo login`.
r? `@Eh2406`
cc #8933
When merging configuration lists, the current order does not match
the expected precedence. This makes merged lists follow precedence
order, with higher precedence items merged later in lists.
Crate checksum lookup query should match on semver build metadata
Since crates.io allows crate versions to differ only by build metadata, a query using `OptVersionReq::exact` + `next()` can return nondeterministic results.
This change fixes the issue by adding an additional `filter` that ensures the version is equal (including build metadata).
It still feels somewhat wrong that a query using `exact` can match multiple crates, so an alternative fix would be to add a new variant of `OptVersionReq` that also matched on build metadata.
Fixes#11412
It confuses people that both `--no-fail-fast` and `--keep-going` exist
on `cargo test` and `cargo bench` but with slightly different behavior.
The intended use cases for `--keep-going` involve build commands like
`build`/`check`/`clippy` but never `test`/`bench`.
Hence, this commit removes `--keep-going` from `test`/`bench` and
provides guidance of `--no-fail-fast` instead.
If people really want to build as many tests as possible, they can also
do it in two steps:
cargo build --tests --keep-going
cargo test --test --no-fail-fast
prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal.
Fixes#10855
As per the discussion on this issue, we want to prompt the user to use `--nocapture` if a test is terminated abnormally. The motivation for this change is described in the issue.
We check for 3 things before we display this flag. -
- `!is_simple` (if the test ended with a non 101 status code)
- `harness` (if the standard test harness was used), and
- `!nocapture` (whether or not the `--nocapture` flag was already passed to the test)
There's further tests added to `test::nonzero_exit_status` that check that the `stderr` is correct for the various combinations possible when a test ends with a non-101 status code.
The new expected behavior is -
- Display `--nocapture` note for only non-zero exit statuses, when the `--nocapture` flag is not passed.
- Only display the note if we use a standard test harness since custom test harnesses do not implement the `--nocapture` flag.
To implement the check for the `--nocapture` flag, the function definition for `report_test_errors` was changed to add the `test_args: &[&str]` parameter. This parameter is passed from the immediate calling function. This private function is only called twice change and is not causing regression after making the appropriate changes to both the places it's called in.
Fix cargo remove incorrectly removing used patches
### What does this PR try to resolve?
Fixes an issue where patches are being removed when member dependencies don't explicitly contain the patched crate.
Fixes#12419
### How should we test and review this PR?
- Created a test for the failing use case
- Verify passing test
<!--
### Additional information
Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
fix(cli): Make `--help` easier to browse
This mirrors some of the categories from `cargo help` (the man pages) using [`clap::Arg::help_heading`](https://docs.rs/clap/latest/clap/struct.Arg.html#method.help_heading).
There are fewer categories to avoid extra vertical space. Instead, they
are left int the `Options` category but put first.
The goal is to try to make it easier to scan `--help` output as the list of flags can get quite long and its easy to miss what features are there.
This mirrors some of the categories from `cargo help` (the man pages).
There are fewer categories to avoid extra vertical space. Instead, they
are left int the `Options` category but put first.
Fix printing multiple warning messages for unused fields in [registries] table
Cargo currently prints the same warning message multiple times for unexpected fields in the `[registries.<registry>]` or `[registry]` tables. This is because Cargo warns each time that the structure is deserialized from `Config`. Depending on which code path is taken I've seen the warning printed up to 6 times.
* A cache of deserialized registry configurations is added to the `Config` struct.
* Registry authentication is changed to directly read the config when searching for a registry name, rather than deserializing each registry configuration.
A test is added to ensure both `[registries]` and `[registry]` only warn once for unexpected fields.
In 1.71, `.cargo-ok` changed to contain a JSON `{ v: 1 }` to indicate
the version of it. A failure of parsing will result in a heavy-hammer
approach that unpacks the `.crate` file again. This is in response to a
security issue that the unpacking didn't respect umask on Unix systems.
Without this, an attacker can leverage globally writable files buried
in the `.crate` file. After a user downloaded and unpacked the file,
the attacker can then write malicous code to the downloaded sources.
`#[allow(internal_features)]` in RUSTC_BOOTSTRAP test
This will be required in the future (where "the future" is rust-lang/rust#108955 which fails CI because of cargo here).
This does emit the unknown lints lint right now but that doesn't matter as it's just warn-by-default - internal_features is deny-by-default though, so it causes errors.
fix(update): Tweak CLI behavior
### What does this PR try to resolve?
When looking at `cargo update` for #12425, I noticed that the two flags related to `--package` were not next to it or each other. I figured grouping them like that would make things easier to browse.
When looking into that, I noticed that the two flags conflict and figured we'd provide a better error message if we did that through clap.
### How should we test and review this PR?
Looking per commit will help show the behavior changes.
### Additional information
I wanted to scope this to being simple, non-controversial, low effort, incremental improvements with this change so I did not look into the history of `--aggressive` not requiring `--package` like `--precise` does and figure out if there is any consistency we can be working towards.
Use thiserror for credential provider errors
### What does this PR try to resolve?
Errors from credential providers currently must a single string. This leads to a lot of `.map_err(|e|cargo_credential::Error::Other(e.to_string())`, which loses the `source()` of these errors.
This changes the `cargo_credential::Error` to use `thiserror` and adds a custom serialization for `std::error::Error` that preserves the source error chain across serialization / deserialization.
A unit test is added to verify serialization / deserialization.
Per my proposal here: https://github.com/rust-lang/cargo/issues/6669#issuecomment-1658593167
I tried to keep the edit minimalistic to match the surrounding style.
If the maintainers are amenable to it, I think it could also be useful to do one or more of:
- Offer concrete guidance on what to do to run actually-all tests (`--all-targets` then separately `--doc`).
- Link to the issue at: https://github.com/rust-lang/cargo/issues/6669
- Mention that `cargo test` without `--all-targets` runs doctests by default, which seems not immediately obvious.
I'd be happy to attempt to add any of the above that the maintainers feel would be a good fit here.
Git only assumes a submodule URL is a relative path if it starts with `./`
or `../` [^1]. To fetch the correct repo, we need to construct an aboslute
submodule URL.
At this moment it comes with some limitations:
* GitHub doesn't accept non-normalized URLs wth relative paths.
(`ssh://git@github.com/rust-lang/cargo.git/relative/..` is invalid)
* `url` crate cannot parse SCP-like URLs.
(`git@github.com:rust-lang/cargo.git` is not a valid WHATWG URL)
To overcome these, this patch always tries `Url::parse` first to normalize
the path. If it couldn't, append the relative path as the last resort and
pray the remote git service supports non-normalized URLs.
See also rust-lang/cargo#12404 and rust-lang/cargo#12295.
[^1]: <https://git-scm.com/docs/git-submodule>
To keep things simple, especially in getting a `Hash` implementation
correct, I'm leveraging `unicase` for case-insensitive
comparisons which is an existing dependency and I've been using for
years on other projects.
This also opens the door for us to add cross-platform compatibility
hazard warnings about multiple paths that would write to the same
location on a case insensitive file system. I held off on that because
I assume we would want #12235 first.
This does mean we can't test the "no manifest" case anymore because the
one case (no pun intended) I knew of for hitting it is now gone.
refactor(test): Move cargo-config into a dir
This is split out of #11912 and is prep for adding more UI tests.
Generally our UI tests are in a directory named after the full cargo command (`cargo config`). These tend to use `snapbox`.
Here we are tests for the `cargo config` command not written by `snapbox` in a `cargo_config.rs` file. This conflicts with adding snapbox UI tests later in a `cargo_config/` folder. Upon looking at this file, it appears to be UI tests, so I think it would make sense to move them into the `cargo_config/` folder. Definitely wouldn't make sense to move them into `config.rs` since that is general config testing.
This is split out of #11912 and is prep for adding more UI tests.
Generally our UI tests are in a directory named after the full cargo
command (`cargo config`). These tend to use `snapbox`.
Here we are tests for the `cargo config` command not written by
`snapbox` in a `cargo_config.rs` file. This conflicts with adding
snapbox UI tests later in a `cargo_config/` folder. Upon looking at this
file, it appears to be UI tests, so I think it would make sense to move
them into the `cargo_config/` folder. Definitely wouldn't make sense to
move them into `config.rs` since that is general config testing.
Currently, the UI tests are
- `cargo add`
- `cargo new`
- `cargo remove`
- `init`
One of these is not like the others. This change renames `init` to
`cargo_init` to suggest it is the UI tests for the `cargo init` command,
rather than `init` functionality.
feat(crates-io): expose HTTP headers and Error type
### What does this PR try to resolve?
This is part of #11521.
[RFC 3231] mentions the authentication process could have an additional **challenge-response mechanism** to prevent potential replay attacks. The challenge usually comes from HTTP `www-authenticate` header as a opaque string. When a client gets a 401/403 response with such a challenge, it may attach the `challenge` to the payload and request again to anwser the challenge.
```
➡️ cargo requests
⬅️ server responds with `www-authenticate` containing some opaque challenge string
➡️ cargo automatically requests again without any user perception
⬅️ server responds ok
```
However, `crates-io` crate doesn't expose HTTP headers. There is no access to `www-authenticate` header.
This PR make it expose HTTP headers and the custom `Error` type, so `cargo` can access and do further on the authentication process.
[RFC 3231]: https://rust-lang.github.io/rfcs/3231-cargo-asymmetric-tokens.html#the-authentication-process
`parent_remote_url` used to be `&str` before #12244. However, we changed
the type to `Url` and it started failing to parse scp-like URLs since
they are not compliant with WHATWG URL spec.
In this commit, we change it back to `&str` and construct the URL
manually. This should be safe since Cargo already checks if it is a
relative URL for that if branch.
fix(embedded): Always generate valid package names
### What does this PR try to resolve?
The sanitization logic uses a placeholder for the first character that isn't valid in the first character position. #12329 took the approach of always using `_` which has the problem of mixing separators if the user used `-` or we had other placeholders to insert. Instead, this takes the approach of stripping the leading invalid characters and using a placeholder name if nothing is left.
Fixes#12330
### How should we test and review this PR?
Per-commit. The first adds tests so the change in behavior can be observed over each additional commit.
### Additional information
I was also hoping to make the binary name not use placeholders by setting `bin.name` to `file_stem` but then I got
```
Compiling s-h-w-c- v0.0.0 (/home/epage/src/personal/cargo/target/tmp/cit/t133/foo)
error: invalid character `'.'` in crate name: `s_h.w§c!`
error: invalid character `'§'` in crate name: `s_h.w§c!`
error: invalid character `'!'` in crate name: `s_h.w§c!`
error: could not compile `s-h-w-c-` (bin "s-h.w§c!") due to 3 previous errors
```
I decided to not get into what are or aren't valid characters according to rustc.
- `cargo pkgid` is unsupported because we can't (yet) generate valid
pkgids for embedded manifests. Adding support for this would be a
step towards workspace support
- `cargo package` and `cargo publish` are being deferred. These would
be more important for `[lib]` support. `cargo install` for `[[bin]]`s
is a small case and As single-file packages are fairly restrictive, a
`[[bin]]` is a lower priority.
The goal is that we shouldn't interefere with end-user output when
"cargo script"s are used programmatically. The only way to detect this
is when piping. CI will also look like this.
My thought is that if someone does want to do `#!/usr/bin/env -S cargo -v`, it
should have a consistent meaning between local development
(`cargo run --manifest-path`) and "script mode" (`cargo`), so I
effectively added a new verbosity level in these cases. To get normal
output in all cases, add a `-v` like the tests do. Do `-vv` if you want
the normal `-v` mode. If you want it always quiet, do `--quiet`.
I want to see the default verbosity for interactive "script mode" a bit
quieter to the point that all normal output cargo makes is cleared before
running the built binary. I am holding off on that now as that could
tie into bigger conversations / refactors
(see https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Re-thinking.20cargo's.20output).
fix(script): Process config relative to script, not CWD
### What does this PR try to resolve?
This is part of the work for #12207.
When you put in your path `foo.rs`:
```rust
#!/usr/bin/env cargo
fn main() {}
```
You expect it to build once and then repeatedly run the same version. However, `.cargo/config.toml` doesn't work like that (normally). It is an environment file, like `.env`, and is based on your current working directory. So if you run `foo.rs` from within a random project, it might rebuild due to RUSTFLAGS in `.cargo/config.toml`.
I had some concern about whether this current behavior is right or not and [noted this in the Pre-RFC](https://github.com/epage/cargo-script-mvs/blob/main/0000-cargo-script.md#unresolved-questions). This came up again while we were [discussing editions on zulip](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/cargo.20script.20and.20edition). In looking further into this, it turns out we already have precedence for this with `cargo install --path <path>`.
### How should we test and review this PR?
The second commit has the fix, the docs, and a change to a test (from the first commit) to show that the fix actually changed behavior.
This is to avoid possible name collisions. For example, a user
creates a file called `.cargo/cache`, and then in the future
cargo wants to create a directory called `.cargo/cache/`, that
would collide with what the user specified. Restricting to `.toml`
extensions would avoid that since we won’t make a directory named
with a `.toml` extension.
fix: Allow embedded manifests in all commands
### What does this PR try to resolve?
This is a part of #12207.
One of the goals is for embedded manifests to be a first class citizen. If you have a script, you should be able to run tests on it, for example.
This expands the error check from just `Cargo.toml` to also single-file packages so you can use it in `--manifest-path`.
This, however, does mean that these *can* be used in places that likely won't work yet, like `cargo publish`.
### How should we test and review this PR?
By commit. We introduce tests for basic commands and then implement and refine the support for this.
### Additional information
Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
feat(cli): Support `cargo Cargo.toml`
### What does this PR try to resolve?
This is making the assumption that we want full unity between places accepting both single-file packages and `Cargo.toml` for #12207. This has not been brought up before in any of the discussions (Internals, eRFC), so I can understand if there are concerns about this and we decide to hold off.
We might want to resolve symlinks before this so people can have a prettier name for these.
### How should we test and review this PR?
The test for this was added in a commit before the actual change, letting people see how the behavior changed.
I originally centralized the error reporting until I realized it likely
is intentionally not centralized so we report errors in terms of the
arguments the user provided.
This puts the lockfile back into a target directory in the users home,
like before #12268.
Another idea that came up was to move the workspace root to be in the
target directory (which would effectively be like pre-#12268) but I
think that is a bit hacky / misleading.
This does mean that the lockfile is buried away from the user and they
can't pass it along with their script. In most cases I've dealt with,
this would be fine. When the lockfile is needed, they will also most
likely have a workspace, so it shoud have a local lockfile in that case.
The inbetween case is something that needs further evaluation for
whether we should handle it and how.