Suppress metadata warnings for non–crates.io publishable packages #16241

Amberley-Sz · 2025-11-11T18:21:09Z

What does this PR try to resolve?

Related issue: #4840

With this change, Cargo still performs full metadata checks for all packages to preserve validation consistency, but it suppresses warnings for crates that are not publishable to crates.io, such as those with publish = false or custom registries (publish = ["my-registry"]).

How to test and review this PR?

Unit tests added in tests/testsuite/package.rs, can be tested locally by running cargo test --test package.

rustbot · 2025-11-11T18:21:13Z

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

tests/testsuite/package.rs

epage · 2025-11-11T18:56:09Z

src/cargo/ops/cargo_package/mod.rs

    if !missing.is_empty() {
-        let mut things = missing[..missing.len() - 1].join(", ");
-        // `things` will be empty if and only if its length is 1 (i.e., the only case
-        // to have no `or`).
-        if !things.is_empty() {
-            things.push_str(" or ");
+        let should_warn = match pkg.publish() {
+            Some(registries) if registries.is_empty() => true, // publish = false, still warn
+            Some(registries) => {
+                // Only warn if crates.io is allowed
+                registries.iter().any(|r| r == CRATES_IO_REGISTRY)
+            }
+            None => true, // No publish field means can publish anywhere, including crates.io
+        };
+
+        if should_warn {
+            let mut things = missing[..missing.len() - 1].join(", ");
+            // `things` will be empty if and only if its length is 1 (i.e., the only case
+            // to have no `or`).
+            if !things.is_empty() {
+                things.push_str(" or ");
+            }
+            things.push_str(missing.last().unwrap());
+
+            gctx.shell().print_report(&[
+                Level::WARNING.secondary_title(format!("manifest has no {things}"))
+                    .element(Level::NOTE.message("see https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info"))
+             ],
+                 false
+            )?
        }


This is checking against what we can publish to. I was assuming we'd check against what we are publishing to (PackageOpts::reg_or_index). Is there a reason you went this direction?

FYI the following code is where that gets evaluated to something useful

cargo/src/cargo/ops/cargo_package/mod.rs

Lines 281 to 298 in 4b0e263

let mut local_reg = {

// The publish registry doesn't matter unless there are local dependencies that will be

// resolved,

// so only try to get one if we need it. If they explicitly passed a

// registry on the CLI, we check it no matter what.

let sid = if (deps.has_dependencies() && (opts.include_lockfile || opts.verify))

|| opts.reg_or_index.is_some()

{

let sid = get_registry(ws.gctx(), &just_pkgs, opts.reg_or_index.clone())?;

debug!("packaging for registry {}", sid);

Some(sid)

} else {

None

};

let reg_dir = ws.build_dir().join("package").join("tmp-registry");

sid.map(|sid| TmpRegistry::new(ws.gctx(), reg_dir, sid))

.transpose()?

};

Thanks for the pointer! I can see that PackageOpts::reg_or_index is accessible for both publish and package, but the resolution from the code you shared is what publish uses to determine where to upload. For cargo package, this field is typically None (unless a resolved SourceId is present).

Since check_metadata runs for both package and publish, I chose the manifest's package.publish field so the behavior remains consistent when there isn't an actual target registry.
I was following this suggestion but I see there's another approach here that checks the SourceId to determine if it's crates.io.

How about a hybrid approach:

when running publish (a resolved target exists): ask the resolved SourceId: is this crates.io? If so emit warnings, otherwise suppress.

when package (no target exists): fall back to checking the manifest’s publish field to see if crates.io is allowed.

The suggestion for package.publish was in the context of moving the warning to cargo check where we don't know where it will be published to in practice. Here, we know where the user is actively publishing to. For the [name1, name2] case, that is less important because the user showed intent for what registry they intend to publish to. for the publish = true case, we don't know the users intent and they may never publish to crates.io. We could say "but when we get to cargo check, we'll display it anyways" but for us to emit this for cargo check, we'd need to have #12235, allowing users to disable it.

when package (no target exists): fall back to checking the manifest’s publish field to see if crates.io is allowed.

I think the question here is about workflows. The warning is intended for crates.io. When is someone using cargo package and while still intending it for crates.io. It could be as a dry-run but we have cargo publish --dry-run. However, it was specifically added with an opt-out, so that shows intent for it to be displayed. In the end, this is a reverseable decision, so let's keep things simple and have just one approach.

Thanks for clarifying! That makes sense, I’ll simplify the logic to check only the resolved SourceId when publishing.

src/cargo/ops/cargo_package/mod.rs

rustbot · 2025-11-12T23:13:36Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

src/cargo/ops/cargo_package/mod.rs

tests/testsuite/package.rs

weihanglo · 2025-11-12T23:31:45Z

src/cargo/ops/cargo_package/mod.rs

+        let should_warn = match reg_or_index {
+            Some(RegistryOrIndex::Registry(reg_name)) => reg_name == CRATES_IO_REGISTRY,
+            None => true,                             // Default is crates.io
+            Some(RegistryOrIndex::Index(_)) => false, // Custom index, not crates.io


Actually with --index "https://github.com/rust-lang/crates.io-index", this could also be crates.io registry (see #16231). Unsure if we want to check that or not.

We can probably have a separate PR resolving that issue.

src/cargo/ops/cargo_package/mod.rs

tests/testsuite/package.rs

epage · 2025-11-12T23:46:00Z

Looks like other tests need to be updated for this.

epage

Thanks!

View changes since this review

Amberley-Sz · 2025-11-13T22:06:44Z

Thanks!

View changes since this review

Thanks for all the communication and bearing with the back and forth!

rustbot assigned ehuss Nov 11, 2025

rustbot added Command-package S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 11, 2025

epage reviewed Nov 11, 2025

View reviewed changes

tests/testsuite/package.rs Outdated Show resolved Hide resolved

epage reviewed Nov 11, 2025

View reviewed changes

src/cargo/ops/cargo_package/mod.rs Outdated Show resolved Hide resolved

Amberley-Sz force-pushed the master branch from b58ea6a to b47a161 Compare November 12, 2025 06:45

This comment has been minimized.

Sign in to view

Amberley-Sz force-pushed the master branch from b47a161 to 4a0345b Compare November 12, 2025 23:13

Amberley-Sz force-pushed the master branch from 4a0345b to 97baf8c Compare November 12, 2025 23:15

weihanglo reviewed Nov 12, 2025

View reviewed changes

epage reviewed Nov 12, 2025

View reviewed changes

src/cargo/ops/cargo_package/mod.rs Show resolved Hide resolved

epage reviewed Nov 12, 2025

View reviewed changes

tests/testsuite/package.rs Outdated Show resolved Hide resolved

Amberley-Sz force-pushed the master branch from 97baf8c to 29c8916 Compare November 13, 2025 19:10

Amberley-Sz added 2 commits November 13, 2025 20:46

test(package): add baseline tests for global metadata warnings

fdc4f95

fix: check sourceId and generate warning accordingly

f73f174

Amberley-Sz force-pushed the master branch from 29c8916 to f73f174 Compare November 13, 2025 20:53

epage approved these changes Nov 13, 2025

View reviewed changes

epage enabled auto-merge November 13, 2025 20:57

epage added this pull request to the merge queue Nov 13, 2025

Merged via the queue into rust-lang:master with commit 8363559 Nov 13, 2025
26 checks passed

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 13, 2025

weihanglo mentioned this pull request Nov 19, 2025

Update cargo submodule rust-lang/rust#149073

Open

	let mut local_reg = {
	// The publish registry doesn't matter unless there are local dependencies that will be
	// resolved,
	// so only try to get one if we need it. If they explicitly passed a
	// registry on the CLI, we check it no matter what.
	let sid = if (deps.has_dependencies() && (opts.include_lockfile \|\| opts.verify))
	\|\| opts.reg_or_index.is_some()
	{
	let sid = get_registry(ws.gctx(), &just_pkgs, opts.reg_or_index.clone())?;
	debug!("packaging for registry {}", sid);
	Some(sid)
	} else {
	None
	};
	let reg_dir = ws.build_dir().join("package").join("tmp-registry");
	sid.map(\|sid\| TmpRegistry::new(ws.gctx(), reg_dir, sid))
	.transpose()?
	};

Suppress metadata warnings for non–crates.io publishable packages #16241

Suppress metadata warnings for non–crates.io publishable packages #16241

Conversation

Amberley-Sz commented Nov 11, 2025

What does this PR try to resolve?

How to test and review this PR?

Uh oh!

rustbot commented Nov 11, 2025

Uh oh!

Uh oh!

epage Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

epage Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Amberley-Sz Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

epage Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Amberley-Sz Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Nov 12, 2025

Uh oh!

Uh oh!

Uh oh!

weihanglo Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Amberley-Sz Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

epage commented Nov 12, 2025

Uh oh!

epage left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Amberley-Sz commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

epage left a comment •

edited by rustbot

Loading