fix(toml)!: Disallow source-less dependencies
### What does this PR try to resolve?
This is part of #13629 addressing “dependency without path, version, git, workspace specified”.
This turns deps like
```toml
foo = { optional = true }
```
from `version="*"` deps with a warning into errors. This breaking change was deemed acceptable because this behavior has been considered a bug from the beginning.
We have gotten little to no feedback about people wanting this behavior.
This improves our forwards-compatibility story as we can add new dependency sources and they won't be considered a wildcard registry dependency on older cargo.
### How should we test and review this PR?
I removed the `cargo_add` test because it is redundant.
We no longer get the “unused key” warnings because those warnings get deferred to after this error gets reported.
### Additional information
This is part of #13629
This turns deps like
```toml
foo = { optional = true }
```
from `version="*"` deps with a warning into errors.
This breaking change was deemed acceptable because this behavior has
been considered a bug from the beginning.
We have gotten little to no feedback about people wanting this behavior.
This improves our forwards-compatibility story as we can add new
dependency sources and they won't be considered a wildcard registry
dependency on older cargo.
This is a part of #13540 which is a party of #9930.
The config is `resolver.something-like-precedence` with values:
- `something-like-maximum` (default)
- `something-like-rust-version`
This is punting on the actual config schema so we can implement
`package.resolver` and `edition = "2024"` support as we want the
MSRV-aware resolver available without `cargo_features`.
As we switch to MSRV-aware resolver, this will help users work out why
MSRV-aware resolving isn't helping them.
This will also make it more obvious if we breaking things when
developing the MSRV-aware resolver.
feat(update): Include a Locking message
### What does this PR try to resolve?
This extends #13561 to `cargo update`. I previously left it out because the locking message was redundant. However the `Locking` message has been extended in #13754 to include the resolving policy which can be a useful point of interest (e.g. "why does `cargo update` do nothing? Oh, `-Zminimal-versions` is enabled").
I still left out the message for `--precise` because the user is overriding all of that.
I'd still like to extend all of this to `cargo install` (and maybe all resolves) but that is taking more investigation.
### How should we test and review this PR?
### Additional information
feat(resolve): Tell the user the style of resovle done
### What does this PR try to resolve?
This is to help with https://github.com/rust-lang/cargo/issues/9930
Example changes:
```diff
-[LOCKING] 4 packages
+[LOCKING] 4 packages to latest compatible version
-[LOCKING] 2 packages
+[LOCKING] 2 packages to latest Rust 1.60.0 compatible versions
-[LOCKING] 2 packages
+[LOCKING] 2 packages to earliest compatible 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.
### How should we test and review this PR?
### Additional information
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.
Judging by the commit history, the original sentence was written before the `[patch]` and `[profile]` sections were added and since those sections do not go underneath the workspace table the original sentence needed to be updated.
This shouldn't change the behavior but makes it safer if
- We add new fields where it will matter
- Copy/paste these for new structs
I did not change things related to the Index because we are already
stuck with that case (whether we want it or not)
feat(cli): Add --ignore-rust-version to update/generate-lockfile
### What does this PR try to resolve?
This is part of #9930 and extends `--ignore-rust-version` to `cargo update` and `cargo generate-lockfile`
### How should we test and review this PR?
First commit sets up tests
### Additional information
`cargo package -p no-exist` emitt error when the -p `package` not found
### What does this PR try to resolve?
Fixes#13719
If `-p` is used, and the spec doesn't match any member, we emit an error like `cargo publish -p` does.
### How should we test and review this PR?
The first commit add a test to show the issue, the next commit add the check logic to fix it.
### Additional information
feat(reslve): Respect '--ignore-rust-version'
### What does this PR try to resolve?
This is a part of #9930.
### How should we test and review this PR?
I had considered several ways of implementing this. I first looked at passing this into `ops::resolve*`.
.This would get a bit annoying with the function signature, so I considered moving it to a builder..
Each of the entry points is slightly different with different ownership needs, making it hard to have a common abstraction.
In doing this, I noticed we currently pass some state around to the resolver via `Workspace`, so I mirrored that.
The nice thing about this location is it provides a good place to hook in config and `package.resolve` so they affect this.
### Additional information
This assumes that if any of the machine applicable fixes in
a diagnostic suggestion is a duplicate, we should see the
entire suggestion as a duplicate.
fix(package): Normalize paths in `Cargo.toml`
### What does this PR try to resolve?
On the surface, this resolves problems that aren't too big of a deal
- Clean up redundant information in paths (e.g. `.////.//foo.rs` being `foo.rs`) which is just visual
- Switch paths with `\` in them to `/`
However, this is prep for #13713 where these will be a much bigger deal
- If the user does `./foo.rs`, we might fail to compare that with the list of files included in the package
- We'll generate paths with `\` and need to make sure the packages can still be used on Windows
### How should we test and review this PR?
### Additional information
For now, this is more for visual consistency.
However, this blocks #13713 as we need to be able to make these paths
comparable to what is included in the package.
feat(add): Stabilize MSRV-aware version req selection
### What does this PR try to resolve?
This is part of #9930 for rust-lang/rfcs#3537
This will make it easier to maintain an MSRV-compliant `Cargo.toml` but leaves validation up to the user as the resolver will pick newer versions.
This helps the MSRV-aware workflows enumerated in
rust-lang/rfcs#3537
### How should we test and review this PR?
As for determining if this is ready for stabilization:
By stabilizing this without the MSRV-aware resolver, this could be confusing to the workflow with an MSRV-compatible lockfile.
PR #13561 at least raises awareness of that discrepancy.
In general there was interest in the RFC discussions to stabilize this ASAP, regardless of what resolver direction we went.
There is an unresolved question on differences in the resolver vs `cargo add` for dealing with an unset `rust-version` (noted in the tracking issue). However, we are only stabilizing the `cargo add` side which is a very light two-way door as this is a UX-focused command and not a programmatic command.
This hasn't gotten much end-user acceptance testing but, as its UX focused, that seems fine (light, two way door)
As such, this seems like it is ready to stabilize.
### Additional information
Switch to using gitoxide by default for listing files
### What does this PR try to resolve?
Uses gitoxide by for listing the contents of a git repository by default. Fixes#10150
It's possible out-opt of this change with the environment variable `__CARGO_GITOXIDE_DISABLE_LIST_FILES=1`. This opt-out mechanism is temporary and will be removed before the next release.
### How should we test and review this PR?
The newly added test fails with the `git2` implementation.
Allow precise update to prerelease.
### What does this PR try to resolve?
This is a feature that attempts to support updates to pre-release versions via `cargo update --precise`.
when `precise-pre-release` used, the prerelase version will be taking consider as compatible version. That said, we can update to any compatible pre-release version. The logic of checking the compatibility of pre-release versions is currently tentative and does not take many conditions into account, this part of the logic makes more sense when implemented in semver.
Use `-Zunstable-options` instead of `-Zprecise-pre-release`.
### How should we test and review this PR?
### Additional information
Part of https://github.com/rust-lang/cargo/issues/13290
refactor(toml): Split out an explicit step to resolve `Cargo.toml`
### What does this PR try to resolve?
This builds on #13664 and #13666. Currently, once we have deserialized `Cargo.toml`, we pass it to a large machinery (`to_real_manifest`, `to_virtual_manifest`) so that
- `Cargo.toml` is resolved
- `Summary` is created
- `Manifest` is created
This splits out the resolving of `Cargo.toml` which is mostly workspace inheritance today.
While splitting logic conjoined like this can be a bit messy in the short term, the hope is that overall this makes the logic easier to follow (more condensed, focused sections to view; more explicit inputs/outputs).
In particular, I hope that this will make it clearer and easier to shift more logic into the resolving step, specifically the inferring of build targets for #13456.
### How should we test and review this PR?
This is broken up into very small steps in the hope that it makes it easier to analyze a step.
### Additional information
Normally, I prefer directly constructing something but I feel this works
better in this case so I can limit a lot of work that is coupled to a
`package` being present.
Since we still directly construct, just with `None`, I think this still
works.
With the shell in `nounset` mode, the intended fallback to filename
completion provokes an error:
```
$ cargo foo <TAB>bash: !opt_var: unbound variable
```
fix(toml): Warn on unused workspace.dependencies keys on virtual workspaces
### What does this PR try to resolve?
This splits out refactors that build on #13589 in preparation for resolving #13456.
As part of those refactors, I noticed an inconsistency on when we warn for unused keys. We have parallel code paths between `to_virtual_manifest` and `to_real_manifest` and only one got updated on a change. This syncs them up. Hopefully the end state this builds to will reduce duplication.
### How should we test and review this PR?
### Additional information
fix(add): Preserve comments when updating simple deps
### What does this PR try to resolve?
Fixes https://github.com/rust-lang/cargo/issues/13645
### How should we test and review this PR?
A case the tests showed but isn't covered here is when a `[features]`
table is created, the dependencies-end comment gets attached to that,
e.g. see cargo_add/overwrite_optional
### Additional information
This is the only error we do this for and we have the context for what
manifest we are parsing.
By removing this, it makes it easier to adjust lifetimes in the short
term.
A case the tests showed but isn't covered here is when a `[features]`
table is created, the dependencies-end comment gets attached to that,
e.g. see cargo_add/overwrite_optional
Fixes#13645
This intentionally borrows from `RefCell`s before conditional code
to try to prevent regressions like #13646 without exhaustively
covering every case that could hit these `BorrowMutError`s with
complicated tests.
I tested this by ensuring the registry code path panicked before
rebasing on top of #13647.
Once I rebased, the panic went away.
Co-authored-by: Ed Page <eopage@gmail.com>
Do not strip debuginfo by default for MSVC
This PR disables the logic which enables debuginfo stripping by default in release mode (https://github.com/rust-lang/cargo/pull/13257) for MSVC, since it causes problems there (https://github.com/rust-lang/rust/issues/122857).
I'm not sure if this is the correct way to gate the logic on a specific target triple.
The root of the issue is that `-Cstrip=debuginfo` currently behaves like `-Cstrip=symbols` on MSVC, which causes the original optimization to break backtraces on Windows. Ultimately, this should probably be fixed in `rustc`, but that might take some further design work, therefore a faster solution would be to just special case this in Cargo for now, and remove the special case later, once it gets fixed on the `rustc` side.
Related issue: https://github.com/rust-lang/rust/issues/122857
refactor: Make lint names snake_case
When working on #13621, I somehow missed that lint names should be `snake_case` according to the [`rustc-dev-guide`](https://rustc-dev-guide.rust-lang.org/diagnostics.html#lint-naming) as well as [`RFC #344`](https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints).
This PR renames:
- `implicit-features` => `implicit_featires`
- `rust-2024-compatibility` => `rust_2024_compatibility`.
<hr>
Note: We should probably have some tooling to enforce this, but I was unsure if it belonged to this PR or another one. One solution would be to use a macro to create the `const LINT_NAME: Lint = {...}`, where `LINT_NAME` would be the `ident` as well as the `name: &'static str` and then have a method on `Lint` to make it lowercase as needed. This is what `rustc` does, and it could work well here. It would ensure snake case as `const` names need to be [`SCREAMING_SNAKE_CASE`](https://rust-lang.github.io/rfcs/0430-finalizing-naming-conventions.html#general-naming-conventions), or a warning is shown.
Use `gitoxide` for `list_files_git`
Related to #10150.
### Tasks
* [x] update `gix` to v0.60
* [x] assure this is tested (currently only git-tests run with `git2` and `gitoxide`)
* [x] allow `list_files_git` to use `gitoxide` if it is enabled as feature.
* [x] use dirwalk iterator
* [x] use new release of `gix` with necessary updates
### Review Notes
As this PR has come a long way, I decided to keep a few of the steps leading up to the final state, showing the PR's evolution in the hope it helps the review.
* Would it be better to simply use `gitoxide` for this without a switch? I don't think
it will cause more trouble than `git2`, and if there is an issue I will fix it with priority.
* In one test, the walk resolves a symlink to a submodule to individual files, including the `.git/*` folder contained in the submodule which is ignored by the walk, i.e. `submodule/*` does not contain it, but `submodule-link/*` does. This is fixed in the gitoxide version, and the `git2` version.
* I noticed that symlinks are resolved for packaging *and* are allowed to point to anywhere, even outside of package root. I left it, but felt that maybe this should be reconsidered.
### Remarks
* I love the test-suite! It's incredibly exhaustive to the point where it uncovers shortcomings in `gitoxide`, which I greatly appreciate.
* I also love `git2` as it's API for many things leads to pretty idiomatic code, and sometimes I really have to work to match it. The example here is the initial `dirwalk()` method which requires a delegate as it doesn't just collect into a `Vec` like `git2` does (for good reason). Turning that into an iterator via `dirwalk_iter()` makes it far more usable, and will definitely be good for performance as the dirwalk work is offloaded into its own thread.
feat: Add a basic linting system
This PR adds a basic linting system, the first step towards [User control over cargo warnings](https://github.com/rust-lang/cargo/issues/12235). To demonstrate the system, a lint for #12826 is added. It supports controlling cargo lints via the `[lints.cargo]` table. Lints can be controlled either by a group or individually.
This is meant to lay down some fundamental infrastructure for the whole linting system and is not meant to be fully featured. Over time, features will be added that will help us reach a much more solid state.
Top-level pathspecs are needed to assure they are not affected by
the CWD. The trailing `/` in `'target` is needed to assure excluded
items are in a folder, and that only entries in that folder are extracted
from the index.
Fix debuginfo strip when using `--target`
This fixes an issue where automatic `strip` of debuginfo added in https://github.com/rust-lang/cargo/pull/13257 wasn't working when the `--target` flag was used.
The problem is that the adjustment code was only running in the optimization pass that is done when `--target` is *not* specified.
The solution is to just always run the unit graph rebuild. I believe it should be safe to do so, since the adjustments it makes should be conditional on just the scenarios that matter when `--target` is not specified. The downside is that this might be a small performance hit when `--target` is used. Trying to avoid that I think would be quite challenging.
Fixes#13617
This is part of #9930 for rust-lang/rfcs#3537
This will make it easier to maintain an MSRV-compliant `Cargo.toml` but
leaves validation up to the user as the resolver will pick newer
versions.
This helps the MSRV-aware workflows enumerated in
rust-lang/rfcs#3537 though it could be confusing to the workflow with an
MSRV-compatible lockfile.
PR #13561 at least raises awareness of that discrepancy.
There is an unresolved question on differences in the resolver vs
`cargo add` for dealing with an unset `rust-version`.
However, we are only stabilizing the `cargo add` side which is a very
light two-way door as this is a UX-focused command and not a
programmatic command.
This hasn't gotten much end-user testing but, as its UX focused, that
seems fine.
As such, this seems like it is ready to stabilize.
With a traditional walk, `.git` will be picked up, and so will be
ignored directories. This commit also doesn't give submodules special
treatment - instead it just tries to open directories as repositories,
or walks them if that fails.
Scott Schafer
bors
Ed Page
Arlo Siemsen
renovate[bot]
Naglis Jonaitis
David Tolnay
jw013
Ralf Jung
Lin Yihai
Weihang Lo
heisen-li
Eric Huss
guqicun
Ville Skyttä
Flowrey
Paul Mabileau
Jakub Beránek
Sebastian Thiel
