The `clippy::perf` lint group is fairly useful for catching bad
practices that might hurt performance marginally.
This PR fixes most of them except `clippy::large_enum_variant`,
which doesn't feel right at this moment and need more researches.
Anyway, overall this PR should be good.
It's very common for users to attempt to use the pseudo-URLs that GitHub
or other providers provide in the form
`git@github.com:rust-lang/rust.git` as a source in Cargo.toml, which are
the default format accepted by OpenSSH. Unfortunately, these are not
actually URLs, and unsurprisingly, the `url` crate doesn't accept them.
However, our error message is unhelpful and looks like this:
invalid url `git@github.com:rust-lang/rust.git`: relative URL without a base
This is technically true, but we can do better. The user actually wants
to write a real SSH URL, so if the non-URL starts with `git@`, let's
rewrite it into a real URL for them to help them and include that in the
error message.
`git@` is the prefix used by all major forges, as well as the default
configuration for do-it-yourself implementations like Gitolite. While
other options are possible, they are much less common, and this is an
extremely easy and cheap heuristic that does not necessitate complicated
parsing, but which we can change in the future should that be necessary.
This also avoids the problem where users try to turn the pseudo-URL into
a real URL by just prepending `ssh://`, which causes an error message
about the invalid port number due to the colon which they have not
changed. Since they can just copy and paste the proposed answer,
they'll be less likely to attempt this invalid approach.
Personally I liked that the test was only dependent on what really matters, the lack of presence of a particular filename. Now the test would fail if Cargo one day adds more (generated) files to the package.
Co-authored-by: Weihang Lo <weihanglo@users.noreply.github.com>
This is a follow up to #14846 which changed `run` to return the
`RawOutput`.
Reasons I didn't "update" some code to the new `run` return value
- We were actually using `ProcessBuilder::exec_with_output` and I didn't
want to disentangle what it would take to switch to `Execs`
- We did processing on the `Result` and I didn't want to check how that
could be updated
### What does this PR try to resolve?
Fixes Issue : #14621
Adds more error context for fetching a commit that doesn't exists.
### How should we test and review this PR?
I've created two tests, one for fast_path and one for libgit2.
r? @weihanglo
Originally it was only included for packages that have executables or
examples for `cargo install`, however this causes inconsistencies and
is kind of unexpected nowadays, e.g. with cdylib crates.
Including it always only slightly increases the crate size and allows
for all crates to know a set of dependency versions that were working,
which can make regression tracking easier.
Fixes https://github.com/rust-lang/cargo/issues/13447
This commit makes lockfile version 4 the default version when Cargo
tries to write to a lockfile.
The lockfile version 4 has been stabilized since 1.78.0,
and will become default in 1.83.0.
the length of transition period is pretty similar as before.
One caveat is that in other output from Cargo,
e.g., `cargo metatada`, status messages,
`SourceID` will display in the v4 URL encoded format.
This shouldn't affect the majority of Rust users,
as `SourceId` representation should be opaque to them,
unless comparing `SourceId` across different version of toolchains.
This is for `cargo generate-lockfile` and when syncing the lockfile with
the manifest.
We still show it for `cargo update` because of `cargo update
--workspace`.
We hacked around this previously by filtering out the `num_pkgs==1` case
for single packages but this didn't help with workspaces.
I was considering moving this into `paths` and noticed `CargoPathExt`.
I figured if we had any extension traits for `Path`, then this is a
reasonable one to add.
We now include the prelude in so many places, this simplifies how we can
present how `cargo-test-support` works.
Yes, this included some `use` clean ups but its already painful enough
walking through every test file, I didn't want to do it twice.
While this is noisy and hides other deprecations, I figured deprecations would
make it easier for people to discover what tasks remain and allow us to
divide and conquer this work rather than doing a heroic PR.
In theory, this will be short lived and we'll go back to seeing
deprecations in our tests.
This is to help with #9930
Example changes:
```diff
-[LOCKING] 4 packages
+[LOCKING] 4 packages to latest version
-[LOCKING] 2 packages
+[LOCKING] 2 packages to latest Rust 1.60.0 compatible versions
-[LOCKING] 2 packages
+[LOCKING] 2 packages to earliest versions
```
Benefits
- The package count is of "added" packages and this makes that more
logically clear
- This gives users transparency into what is happening, especially with
- what rust-version is use
- the transition to this feature in the new edition
- whether the planned config was applied or not (as I don't want it to
require an MSRV bump)
- Will make it easier in tests to show what changed
- Provides more motiviation to show this message in `cargo update` and
`cargo install` (that will be explored in a follow up PR)
This does come at the cost of more verbose output but hopefully not too
verbose. This is why I left off other factors, like avoid-dev-deps.
fix(metadata): Stabilize id format as PackageIDSpec
### What does this PR try to resolve?
For tools integrating with cargo, `cargo metadata` is the primary interface. Limitations include:
- There isn't an unambiguous way to map a package entry from `cargo metadata` to a parameter to pass to other `cargo` commands. An `id` field exists but it is documented as an opaque string, useful only for comparisons with other `id`s within the document.
- There isn't an unambiguous way of taking user parameters (`--package`) and mapping them to `cargo metadata` entries. `cargo pkgid` could help but it returns a `PackageIdSpec` which doesn't exist within the `cargo metadata` output.
This attempts to solve these problems by switching the `id` field from `PackageId` to `PackageIdSpec` which is a [publicly documented format](https://doc.rust-lang.org/cargo/reference/pkgid-spec.html), can be generated by `cargo pkgid`, and is accepted by most commands via the `--package` flag.
As the `"id"` field is documented as opaque, this technically isn't a breaking change though people could be parsing it.
For `cargo_metadata` they do [use a new type that documents it as opaque but publicly expose the inner `String`](https://docs.rs/cargo_metadata/latest/cargo_metadata/struct.PackageId.html). The `String` wasn't publicly exposed due to a request by users but instead their `PackageId` type replaced using `String`s in the API in oli-obk/cargo_metadata#59 with no indication given as to why the `String` was still exposed. However, you'll note that before that PR, they had `WorkspaceMember` that parsed `PackageId`. This was introduced in oli-obk/cargo_metadata#26 without a motivation given.
**Note that `PackageIdSpec` has multiple representation that might uniquely identify a package and we can return any one of them.**
Fixes#7267
### How should we test and review this PR?
### Additional information
cc `@oli-obk`
`cargo update --precise` might pass in any arbitrary Git reference,
and `git2::Oid::from_str` would always zero-pad the given str if it is
not a full SHA hex string.
This introduces an enum `Revision` to represent `locked_rev`
that is either deferred or resolved to an actual object ID.
When `locked_rev` is a short ID or any other Git reference,
Cargo first performs a Git fetch to resolve it (without --offline),
and then locks it to an actual commit object ID.
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.
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.
