[patch] in config.toml triggers unhelpful warnings

[alecmocatta]

Problem

#9269 introduced [patch] in .cargo/config.toml. If dependencies patched in this way are used in some workspaces but not others, unnecessary warnings are emitted:

warning: Patch `headless_chrome v0.9.0 (https://github.com/alecmocatta/rust-headless-chrome#db63e48b)` was not used in the crate graph.
Patch `optional_struct v0.2.0 (https://github.com/alecmocatta/OptionalStruct#58d1f03a)` was not used in the crate graph.
Patch `tokio-tar v0.3.0 (https://github.com/alecmocatta/tokio-tar#01af794b)` was not used in the crate graph.
Check that the patched package version and available features are compatible
with the dependency requirements. If the patch has a different version from
what is locked in the Cargo.lock file, run `cargo update` to use the new
version. This may also occur with an optional dependency that is not enabled.

We use the xtask pattern, with workspaces for different targets.

Possible Solution(s)

Silence warnings about unused patches that come from config.tomls

Version

cargo 1.65.0-nightly (efd4ca3dc 2022-08-12)

[weihanglo]

Hmm… interesting. IIRC, [patch] follows SemVer when matching which dep to patch. That means the error message is quite useful if one configure the [patch] wrong — it won't fail silently.

I noticed you used plural workspaces. Did you mean you have a .cargo/config.toml shared across several workspaces? However, Cargo doesn't know which workspace is going to use this [patch] from .cargo/config.toml. The best effort it could make is to warn every unused patches, in order not to miss any actual misconfigured [patch]s. It's a false-positive indeed, though I feel like it is better than true-negative. We could probably ignore all warnings but that sounds a bit dangerous.

Since Cargo configurations follow a hierarchical structure for their discovery. That mean you can have multiple .cargo/config.toml in you filesystem. Is it possible to put the actual shared part only under a certain workspace? So that only the workspace using the [patch] get the config.

The other way round is abusing --config <path>. You can put the shared config file outside the hierarchical config discovery, and load it on-demand with --config <path>.

There is also a brand new -C <dir> coming in 1.69, which change to the given directory before any command happens. As the path of the current directory affects which configs Cargo picks up, it might be useful if you want to tweak the discovery.

[alecmocatta]

I have mostly-duplicate [patch] sections in multiple workspaces in a single repo. It would be ideal to specify a [patch] section in one place, i.e. the .cargo/config.toml at the top of the repo. The problem is we then we get false positive warnings about them not being used (because they're used for one workspace but not another).

For a [patch] section in a Cargo.toml, it's a very useful warning. In .cargo/config.toml, where there are potentially multiple workspaces it's applying to, it's quite noisy to have an insuppressible false positive warning when building different workspaces.

[weihanglo]

FWIW, with #12115 we open a possibility that Cargo can have its own lint rules. This issue could benefit from that.

[weihanglo]

@rustbot label +Z-lints-table

Labelled as such to indicate it is currently blocked on -Zlints unstable feature.

[weihanglo]

OTOH, people may also want to deny this warning to ensure every patch is applied. See https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Force.20error.20on.20.22Patch.20.2E.2E.2E.20was.20not.20used.20in.20the.20crate.20graph.22.3F.

So we need it to be configurable.

[epage]

So far, we've only had manifest, and not config, lints in scope for being controlled by the user as lint control is only allowed in Cargo.toml and not .cargo/config.toml or the CLI. We'd have to decide what the right framing of this is, whether its a lint against the config or a lint against the Cargo.lock, which could block whether this can move forward. In most cases, I see this as a lint against the config as that is the half that is usually "in the wrong". We'd then have to decide if we're willing to compromise on that for the sake of doing this.