fix(toml)!: Disallow source-less dependencies #13775

epage · 2024-04-18T17:58:55Z

What does this PR try to resolve?

This is part of #13629 addressing “dependency without path, version, git, workspace specified”.

This turns deps like

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 rust-lang#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.

rustbot · 2024-04-18T17:59:00Z

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

weihanglo · 2024-04-18T19:44:06Z

src/cargo/util/toml/mod.rs

@@ -1672,14 +1672,11 @@ fn detailed_dep_to_dependency<P: ResolveToPath + Clone>(
    kind: Option<DepKind>,
 ) -> CargoResult<Dependency> {
    if orig.version.is_none() && orig.path.is_none() && orig.git.is_none() {
-        let msg = format!(
-            "dependency ({}) specified without \
+        anyhow::bail!(


Question: What's the criteria of turning this into

a normal warning

a critical warning that bails out later

a fatal error bailing out here

turning this into a normal warning

This was already a warning, so no change there

a fatal error bailing out here

When we discussed #13629 as a team, we seemed in favor of a hard error as this was deemed a bug that people haven't been clamoring for us to continue to support.

a critical warning that bails out later

I honestly haven't touched that system much. I think those "recoverable errors", right? As in, we continue moving forward, seeing other messages, and but fail after all of that?

While I would want to move more towards error recovery, this has some weird properties

These only show up when emit_warnings is called which looks to only be compile commands and cargo fetch

The overall value of error recovery seems relatively low (not much more is really going to be shown but instead people might see other unrelated warnings)

Thanks for the clarification. I understand this being a fatal error as a team decision. The recovery error is something I am also thinking about, yet it doesn't apply to this situation.

weihanglo · 2024-04-18T21:59:40Z

@bors r+

Thanks!

bors · 2024-04-18T21:59:43Z

📌 Commit cf23e4b has been approved by weihanglo

It is now in the queue for this repository.

bors · 2024-04-18T22:02:08Z

⌛ Testing commit cf23e4b with merge 7e9c2ef...

bors · 2024-04-18T22:31:32Z

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 7e9c2ef to master...

Update cargo 8 commits in 6f06fe908a5ee0f415c187f868ea627e82efe07d..80d5b607dde6ef97dfff4e23923822c01d2bb036 2024-04-16 18:47:44 +0000 to 2024-04-19 18:39:22 +0000 - fix 13773 - 'cargo build' fails when list_files() with gix is triggered (rust-lang/cargo#13777) - fix(toml): Don't crash on parse errors that point to multi-byte character (rust-lang/cargo#13780) - fix(toml)!: Disallow source-less dependencies (rust-lang/cargo#13775) - fix(msrv): Put MSRV-aware resolver behind a config (rust-lang/cargo#13769) - fix(msrv): Error, rather than panic, on rust-version 'x' (rust-lang/cargo#13771) - fix(credential): trim newlines in tokens from stdin (rust-lang/cargo#13770) - test(msrv): Re-organize MSRV tests (rust-lang/cargo#13767) - feat(install): Including Locking message (rust-lang/cargo#13764) r? ghost

As of cargo 1.79.0, our fake_root Cargo.toml is no longer valid due to the change in rust-lang/cargo#13775 which disallows a dependency without a version (or path, or something). Previously, these were implicitly a "wildcard registry dependency" but no longer. So do this explicitly to set the feature on the dependency without any version requirement. The version will be specified by std. R=aeubanks@google.com Bug: 339172099 Change-Id: Ifbb78ced06d520755b683f49a6daca2cd0bcd333 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5525961 Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: Arthur Eubanks <aeubanks@google.com> Cr-Commit-Position: refs/heads/main@{#1298307}

As of cargo 1.79.0, our fake_root Cargo.toml is no longer valid due to the change in rust-lang/cargo#13775 which disallows a dependency without a version (or path, or something). Previously, these were implicitly a "wildcard registry dependency" but no longer. So do this explicitly to set the feature on the dependency without any version requirement. The version will be specified by std. R=aeubanks@google.com Bug: 339172099 Change-Id: Ifbb78ced06d520755b683f49a6daca2cd0bcd333 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5525961 Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: Arthur Eubanks <aeubanks@google.com> Cr-Commit-Position: refs/heads/main@{#1298307} NOKEYCHECK=True GitOrigin-RevId: b71d1630b5f82988db7c26c0675648aa1a4563fd

rustbot assigned ehuss Apr 18, 2024

rustbot added A-manifest Area: Cargo.toml issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2024

epage mentioned this pull request Apr 18, 2024

Next step for deprecations in Cargo #13629

Open

13 tasks

weihanglo approved these changes Apr 18, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2024

bors merged commit 7e9c2ef into rust-lang:master Apr 18, 2024
23 checks passed

epage deleted the incomplete-dep branch April 19, 2024 01:26

weihanglo mentioned this pull request Apr 19, 2024

Update cargo rust-lang/rust#124177

Merged

rustbot added this to the 1.79.0 milestone Apr 20, 2024

weihanglo mentioned this pull request May 3, 2024

Configuration specific dependencies break cargo tree #13854

Closed

epage mentioned this pull request May 15, 2024

In Cargo.toml I missspelled "version" and it still worked, but after I corrected it I now get compile errors. #13918

Closed

epage mentioned this pull request Oct 1, 2024

bdf regress because bit-set 0.3.0 disappeared in the recent few months #14643

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(toml)!: Disallow source-less dependencies #13775

fix(toml)!: Disallow source-less dependencies #13775

epage commented Apr 18, 2024 •

edited by weihanglo

Loading

rustbot commented Apr 18, 2024

weihanglo Apr 18, 2024 •

edited

Loading

epage Apr 18, 2024

weihanglo Apr 18, 2024

weihanglo commented Apr 18, 2024

bors commented Apr 18, 2024

bors commented Apr 18, 2024

bors commented Apr 18, 2024

fix(toml)!: Disallow source-less dependencies #13775

fix(toml)!: Disallow source-less dependencies #13775

Conversation

epage commented Apr 18, 2024 • edited by weihanglo Loading

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Apr 18, 2024

weihanglo Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

epage Apr 18, 2024

Choose a reason for hiding this comment

weihanglo Apr 18, 2024

Choose a reason for hiding this comment

weihanglo commented Apr 18, 2024

bors commented Apr 18, 2024

bors commented Apr 18, 2024

bors commented Apr 18, 2024

epage commented Apr 18, 2024 •

edited by weihanglo

Loading

weihanglo Apr 18, 2024 •

edited

Loading