cargo publish has bad error message when explicit [[bench]] isn't in package.include

[dhardy]

Problem

cargo publish fails

Steps

Using the latest rand commit:

$ cargo publish --dry-run
    Updating crates.io index
   Packaging rand v0.9.0-alpha.0 (/home/dhardy/projects/rand/rand)
    Updating crates.io index
   Verifying rand v0.9.0-alpha.0 (/home/dhardy/projects/rand/rand)
error: failed to verify package tarball

Caused by:
  failed to parse manifest at `/home/dhardy/projects/rand/rand/target/package/rand-0.9.0-alpha.0/Cargo.toml`

Caused by:
  can't find `uniform` bench at `benches/uniform.rs` or `benches/uniform/main.rs`. Please specify bench.path if you want to use a non-default path.

Possible Solution(s)

No response

Notes

The relevant part of the Cargo.toml:

[[bench]]
name = "uniform"
harness = false

This could have a path specified... but as far as I can tell, cargo publish is the only thing not happy to discover the default path (benches/uniform.rs).

Despite this, the benchmark works fine:

$ ls -l benches/uniform.rs 
-rw-r--r--. 1 dhardy dhardy 2696 Feb 13 11:39 benches/uniform.rs
$ cargo bench --bench uniform --features small_rng
   Compiling rand v0.9.0-alpha.0 (/home/dhardy/projects/rand/rand)
    Finished `bench` profile [optimized] target(s) in 2.28s
     Running benches/uniform.rs (target/release/deps/uniform-38f8da4b7c78a479)
samplei8/SmallRng/single
                        time:   [1.5141 ns 1.5143 ns 1.5145 ns]
[snip]

Steps

• refactor(package): Simplify getting of published Manifest #13666
• refactor(toml): Flatten manifest parsing #13589
• fix(toml): Warn on unused workspace.dependencies keys on virtual workspaces #13664
• refactor(toml): Split out an explicit step to resolve Cargo.toml #13693
• refactor(toml): Decouple target discovery from Target creation #13701
• fix(toml): Warn, rather than fail publish, if a target is excluded #13713

Version

cargo 1.78.0-nightly (fc1d58fd0 2024-02-09)
release: 1.78.0-nightly
commit-hash: fc1d58fd0531a57a6b942a14cdcdbcb82ece16f3
commit-date: 2024-02-09
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.6.0-DEV (sys:0.4.71+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Fedora 39 [64-bit]

[epage]

rand has package.include set:

include = ["src/", "LICENSE-*", "README.md", "CHANGELOG.md", "COPYRIGHT"]

This doesn't include benches/.

When you use auto-detected benches, cargo publish works just fine because it will auto-detect among the bundled files and be fine.

When you add [[bench]] tables, cargo knows to look for them but they were not included, and so things error.

Possible ways to improve the experience for this:

• Make the error clearer by explicitly checking whether targets were included
• Auto-strip certain types of targets, like [[bench]] and [[test]]

[weihanglo]

Auto-strip certain types of targets, like [[bench]] and [[test]]

I might be wrong, but doesn't crater run rely on tests being published to crates.io?

[epage]

I meant "strip them if they aren't included", not "unconditionally strip them". It could also likely be a user-controlled lint so people can control whether they want to error or allow it (#12235).

[dhardy]

Isn't inclusion in the package a separate issue?

I would expect

[[bench]]
name = "uniform"
harness = false

to behave identically to

[[bench]]
name = "uniform"
path = "benches/uniform.rs"
harness = false

where benches/uniform.rs exists, given that the former is already supported by cargo bench.

[epage]

Interesting, I hadn't realized that the presence of path makes a difference.

The reason is that without path, we have to do target auto-detection which fails. With path, we skip that and only error if the target is activated.

This makes me think we'll only be able to handle this by stripping these target as erroring would likely break some existing cases. We could still warn though and once that is user controllable, they could turn that into an error.

[vincentdephily]

I think I'm hitting a variant of this, testing with cargo package. My Cargo.toml contains

[package]
exclude = ["benches"]
[[bench]]
# `benches/exec_compare.rs` is not meant to be run from cargo
name = "exec_compare"
bench = false

Which gives me

can't find `exec_compare` bench at `benches/exec_compare.rs` or `benches/exec_compare/main.rs`.
Please specify bench.path if you want to use a non-default path.

I can work around this by not excluding benches (but that's not the desired behavior), or by setting path="anything". The fact that even a bogus path value makes cargo happy is interesting.

[heisen-li]

At the time of upload，setting path but not verifying the validity of the path seems reasonable to me. But if bench = false is set, should existential checks be eliminated?

cargo book:

Setting targets to bench = false will stop them from being bencharmked by default.

I understand the implication here is that with bench = false set, Cargo will not execute benchmarks for crates with name=xx. Am I understanding this wrong?

[weihanglo]

@heisen-li have not checked the code but I believe this is about target path auto-discovery inconsistency when packaging, not compiling default targets.

[epage]

btw i have a solution in work. I am working to make it so published packages have all targets explicitly enumerated.