patch was not used message does not adequately describe reason

[ryankurte]

Heya, not sure whether we consider usability issues bugs, but, it seemed the closest option.

When configuring [patch]es there are a variety of reasons the patch may not be used, resulting in a message like the following:

warning: Patch `SOMETHING` 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.

This gives a list of reasons that can be particularly difficult to debug, as well as the recommendation that cargo update may help, which is particularly helpful when this message is printed by cargo update. As an alternative, it would be great to print the specific reason the [patch] wasn't used, and a proposed resolution, which I believe is in-line with other compiler error improvements.

Some possible examples:

warning: Patch `SOMETHING` was not used in the crate graph.
Patch version "0.14.0" from `../SOMETHING` does not meet package constraint ">0.15.0",
this may be resolved by updating the `SOMETHING` package version

warning: Patch `SOMETHING` was not used in the crate graph.
Patch version "0.14.0" does not match the package version in Cargo.lock,
this may be resolved by running `cargo update` to update the cargo lock file.

Related to: #8400

[jyn514]

This is a cargo bug, not a rust bug.

[jyn514]

@ehuss could you transfer the issue?

[ehuss]

Transferred.

@ryankurte it would be helpful if you could put together an example that doesn't print a helpful error message. Please also include which version of Cargo you are using. The messages were improved recently (#8248), and the example where the patch does not satisfy the dependency requirement should say something like this:

[ERROR] failed to resolve patches for `https://github.com/rust-lang/crates.io-index`
Caused by:
  patch for `bar` in `https://github.com/rust-lang/crates.io-index` failed to resolve
Caused by:
  The patch location `[..]/foo/bar` contains a `bar` package with version `0.1.0`, 
  but the patch definition requires `^0.1.1`.
  Check that the version in the patch location is what you expect, and update the 
  patch definition to match.

[ryankurte]

oh hey that's super exciting, maybe this is solved (though i am still seeing the old messages on 1.48.0-nightly?)

it would be helpful if you could put together an example that doesn't print a helpful error message

the current iteration of this is in a non-public project and, (having seen / resolved this many a time prior) i have no idea why the resolution is broken in this case, which is a bit of an impediment to useful reproduction :-( i will try to dig further.

[ryankurte]

right, turns out to be a user error when you have a git dependency and try to patch with [patch.crates-io]... which is documented here. clearly a mistake on my part but, seems like it would be nice to provide a nudge in the right direction?

with an cargo new whatever project and a clone of https://github.com/rust-lang/log in the same directory.

the following works, as expected the crates.io version is replaced with the local version:

[dependencies]
log = "0.4.11"

[patch.crates-io]
log = { path = "./log" }

This does not work, which, perhaps should be obvious if you read [patch.**crates-io**] but, this took me... some weeks :-/

[dependencies]
log = { git = "https://github.com/rust-lang/log" }

[patch.crates-io]
log = { path = "./log" }

and this gives you the classic "not used in the crate graph" warning

warning: Patch `log v0.4.11 (/Users/ryankurte/projects/patch-test/log)` 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.

the correct approach of course is to:

[dependencies]
log = { git = "https://github.com/rust-lang/log" }

[patch."https://github.com/rust-lang/log"]
log = { path = "./log" }

imo it'd be excellent to have something like:

warning: Patch `log v0.4.11 (/Users/ryankurte/projects/patch-test/log)` was not used in the crate graph.
Crate `log` from "https://github.com/rust-lang/log" was patched from "crates-io", 
you may wish to use `[patch."https://github.com/rust-lang/log"]` to patch the included crate.
See https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#the-patch-section for more details
...

[ehuss]

Thanks for digging into that! Yea, that definitely seems like a scenario that could stand to have a better error message.

[matu3ba]

@ryankurte

Just writing a requirement list would be easier to read in the documentation.
Does this mean that I am forced to 1. either specify dependencies on git repos in [dependencies], 2.clone crates locally for usage (patching) or 3.only specify branches of the git repo in crates.io ?
The text feels very verbose, but leaves out the important parts.

Using the example

[dependencies]
uuid = "1.0.1"
[patch.crates-io]
uuid = { git = 'https://github.com/uuid-rs/uuid' }

or using uuid ="1.0.0" resulted in the same confusing message.
This kind of problem was abit more frequently asked.

[matu3ba]

My guess is that also the minor version 1.0.1 is significant, but this is not documented.

[ryankurte]

it appears that uuid does not have a version 1.0.0? maybe the example could be updated to something that works?

that said, i would suspect the similar situation is because git targets can't resolve -versions-, so you'd need to also supply a branch or tagargument that contains a compatible version of the crate to patch?

[skhameneh]

I ran into this issue with a slightly different problem, when the major does not match, the patch is ignored. This is not desired behavior as I do not see a way to control the patch apply/ignore logic.

[skhameneh]

Related #5640

[Joshuashen022]

I ran into same probelm I change the package verson and problem solved.
My local crate verson was 0.16.1 the offical version was 0.16.2
So I change my local version to 0.16.1 to 0.16.9 and other settings remain the same.
And when I run cargo update it switchs to 0.16.9.

[Wilfred]

I bumped into this issue today. How about wording like this?

Check that the patched package version and available features are compatible
with the version and features specified in Cargo.toml.

[weihanglo]

From this code section:

cargo/src/cargo/ops/resolve.rs

Lines 471 to 488 in 458d345

	if register_patches {
	// It would be good if this warning was more targeted and helpful
	// (such as showing close candidates that failed to match). However,
	// that's not terribly easy to do, so just show a general help
	// message.
	let warnings: Vec<String> = resolved
	.unused_patches()
	.iter()
	.map(|pkgid| format!("Patch `{}` was not used in the crate graph.", pkgid))
	.collect();
	if !warnings.is_empty() {
	ws.config().shell().warn(format!(
	"{}\n{}",
	warnings.join("\n"),
	UNUSED_PATCH_WARNING
	))?;
	}
	}

I feel that "close candidates that failed to match" cannot be simply defined. From my experience, [patch] is used on transitive dependencies more than direct ones. If we want to provide informative messages, we might need to take the whole dependency graph into account. then it would become somehow hard to display a proper message. Is a transitive patch with mismatched registry closer than a direct patch with incompatible version? Or should we list all of them? None looks good enough to me.

Moreover, since patch-in-config has been stabilized in 1.56, I am afraid that unused patches become more common. That is, more unused patches may lead to a slight performance loss in order to emitting better warnings by querying the graph.

Is there any better place or approach to improve this situation?

[ehuss]

@weihanglo There are many reasons why a patch could fail, so I would recommend trying to tackle individual examples instead of going for a general solution. I think the example above about using the wrong source ([patch.crates-io] instead of the git location) shouldn't be too hard to improve. I would iterate over the unused patches, and then query resolved if it contains anything close to the missing patch (such as looking for a package by name).

Here is an example that does that. That example only works within a single source, so something similar will need to be done inside resolve_with_previous (just look for a package by name, ignoring the source and version).

[weihanglo]

I am afraid that the information from resolved and unused_patches is not enough. Imagine a manifest like below:

[dependencies]
git2 = "0.13"

[patch.'https://github.com/rust-lang/bar']
git2 = { path = "bar" }

[patch.alt-registry]
git2 = { path = "bar" }

The unused_patches contains two identical PackageIds, and the warning message would show it twice. It would be great if we can inform users which [patch.<url>] uses a wrong source as #8690 (comment) described. However, the info is not sufficient to know which patch has been used or not, since it is determined by the containment of PackageId in the graph. OTOH, if one of these patches is used, the other is also considered as used, which is not correct.

The case is awkward and rare, but still valid, and it seems not trivial to me to deal with. Maybe we need to define what are "used patches" and then track them?

[ehuss]

I personally wouldn't worry too much about more unusual situations like that. It would be nice to have the information about the original patch definition, but that may be quite difficult to do (since the unused patches are part of Cargo.lock). If it is possible to provide an improvement for more common mistakes, I would focus on that (assuming that is on the easier side of implementation).