Warnings for implicit tests and examples on cargo package/publish causes extra noise

[ia0]

Problem

With nightly-2024-05-01-x86_64-unknown-linux-gnu, we don't get any warning during cargo package or cargo publish that implicit (i.e. not listed in the Cargo.toml) examples and tests are not included (when using package.include in the Cargo.toml).

But when using nightly-2024-05-24-x86_64-unknown-linux-gnu, we do get warnings:

warning: ignoring example `baz` as `examples/baz.rs` is not included in the published package
warning: ignoring test `bar` as `tests/bar.rs` is not included in the published package

This might have been caused by #13713.

Steps

cd /tmp
cargo new foo --lib
cd foo
mkdir tests
touch tests/bar.rs
mkdir examples
touch examples/baz.rs
sed -i 's#^$#include = ["/src/"]#' Cargo.toml
git add .
git commit -mx
cargo package

Possible Solution(s)

Partially revert #13713 to not warn for implicit examples and tests (possibly also benches) like it was before.

Notes

Help was provided in https://users.rust-lang.org/t/rationale-behind-warning-for-examples-and-tests-in-cargo-publish/114840 to figure out the issue and culprit.

Version

# Good version

cargo 1.80.0-nightly (b60a15551 2024-04-26)
release: 1.80.0-nightly
commit-hash: b60a1555155111e962018007a6d0ef85207db463
commit-date: 2024-04-26
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Debian n/a (rodete) [64-bit]

# Bad version

cargo 1.80.0-nightly (84dc5dc11 2024-05-20)
release: 1.80.0-nightly
commit-hash: 84dc5dc11a9007a08f27170454da6097265e510e
commit-date: 2024-05-20
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Debian n/a (rodete) [64-bit]

[epage]

Could you give an idea of what the impact to you is?

This isn't a regression but intentional new-ish behavior. In fact, the behavior is in the name of #13713. We need to weigh the intent of #13713 against the user impact of those not interested in that intent.

Before #13713, excluding a target could lead to different results

• error if its required (need at least a bin or a lib)
• error if specified in Cargo.toml with only name or path
• silently exclude if specified in Cargo.toml with name and path
• silently exclude if implicitly discovered

There also wasn't a good way to tell what targets are excluded. You could go through hoops to open up a .crate file and see what files are missing. Or you could run cargo metadata --no-deps | jq inside of the extracted .crate.

#13713 turned all of the last three cases into "warn that its being excluded". Users who explicitly specify a target may be surprised that it is missing. By explicitly listing each target, you only need to look at your Cargo.toml in your .crate file to see what was done. The warning is intended to make this part of the publish workflow so people can verify their include/exclude behavior is working as intended and to know when cargo is overriding things.

[ia0]

Could you give an idea of what the impact to you is?

cargo publish prints warnings that are expected and can't be silenced, thus reducing the usefulness of actual warnings for events that are not expected and must be addressed (for example forgetting to include build.rs).

In fact, the behavior is in the name of #13713.

That's not how I read that name. For me, "Warn, rather than fail publish, if a target is excluded" means that a previous error turned into a warning, and not that a new warning is introduced. But that's no really relevant if the actual behavior was to introduce the warning, that's fine by me, and we can discuss ways to allow users to disable it.

Users who explicitly specify a target may be surprised that it is missing.

I completely agree with that. The particular issue I have is about implicit targets (i.e. not listed in [[test]] or [[example]]) that I don't want to publish.

By explicitly listing each target, you only need to look at your Cargo.toml in your .crate file to see what was done.

That's probably the part I'm missing to understand the rationale. Do you mean that it is expected to never have implicit tests or examples (i.e. tests and examples should always be listed in the Cargo.toml as [[test]] and [[example]])? This is surprising to me, and if that's the case, then I'll probably simply move my tests and examples in a different crate that is not published, such that the published crate only contains the library (or the parts I want to publish). This is fine by me but I would like to know that this is the recommended approach.

The warning is intended to make this part of the publish workflow so people can verify their include/exclude behavior is working as intended and to know when cargo is overriding things.

I see 2 mechanisms to verify the include/exclude behavior that I believe are superior and working:

• Use cargo package --list
• Use cargo publish --dry-run which should fail compilation if a file is missing (although I agree that this might not test as much as needed, in particular when multiple target architectures are supported or different features may select different files)

[epage]

By explicitly listing each target, you only need to look at your Cargo.toml in your .crate file to see what was done.

That's probably the part I'm missing to understand the rationale.

Sorry, I was talking within the context of #13713 and .crate files. cargo package transforms your Cargo.toml to explicitly list all build-targets.

Use cargo package --list

Identifying the absence of something is not always obvious.

Use cargo publish --dry-run which should fail compilation if a file is missing

Our intention was to demote the existing error for "build-target is specified but the file is missing".

[ia0]

cargo package transforms your Cargo.toml to explicitly list all build-targets.

I might be missing something here. It seems like before #13713 (or at least with nightly-2024-05-01) the build-targets are not explicitly listed. And it seems that after #13713 (or at least with nightly-2024-05-24) only the [lib] is explicitly listed (at least in the reproduction case I gave in the issue description). However, the following package fields are added:

autoexamples = false
autotests = false

So it looks like cargo package indeed makes things explicit, but it doesn't list all build-targets. It is somehow smart and takes include into account, or at least that's my guess based on the behavior I observe.

Note that I like this new behavior (expliciting the build-targets and only listing those meant to be published based on include). What I don't like is really just the warnings. And thanks to looking into this, I found out how to disable those warnings. I just need to explicitly add autotests = false.

So from my side, we can close this issue. The behavior is as I would expect:

• cargo package make things explicit based on user information (include, autotests, etc).
• Warnings are only emitted if user information is missing.

Thanks for the rubber-ducking!

[ia0]

I spoke too fast, using autotests = false is not a good solution, because it prevents using cargo test --test=bar.

Actually, if there was a publish field in target configuration, then I could have something like that:

[package]
include = ["/src/"]
autotests = false

[[test]]
name = "bar"
path = "tests/bar.rs"
publish = false # make it explicit that we don't want to publish this

[epage]

So it looks like cargo package indeed makes things explicit, but it doesn't list all build-targets. It is somehow smart and takes include into account, or at least that's my guess based on the behavior I observe.

Yes, we only include a build-target in the published Cargo.toml if its root is included in the .crate

[ia0]

Actually, I would argue that having this publish field (defaults to true for backward compatibility) is useful even for originally explicit build-targets (not just implicit built-targets that must be made explicit). Because I don't see the reasonning why an explicit build-target is meant to be published. This seems like conflating 2 different concepts.

Taking a step back, as more time passes, the more I believe that crates are not meant to carry more than one build-target. This has similar issues as cargo workspaces (which I'm not using for many reasons). This particular noisy warning is rather minor, but see #13844 (comment) for example. You can get quite surprised to see that cargo check --all-targets succeeds but cargo check --lib (which should be a subset) fails, and so simply because checking a test (which enables dev-depedencies) enabled a feature of a direct dependency of the library that is needed for the library to compile. So I would understand if the conclusion of this issue is that crates should not have more than one built-target. I'm already working around this issue by always explicitly selecting a single build-target for all my cargo invocations: google/wasefire#490 and google/wasefire#491. I wouldn't be against either if package.publish could (in addition to taking a boolean) also take a string which could be lib or bin=foo for example, to explicitly select the build-target to publish (thus enforcing that only one build-target can be published).