Location of patch information

[kurtlawrence]

Problem

If there are errors in a [patch] entry, it can be hard to locate where the patch was specified.

I work on a large code base that spans multiple workspace crates and cross repo crates. I have experienced a few times that we migrate a set of dependencies, and get caught with resolution errors. If the error is because of a [patch] entry which is no longer applicable, it only gives the crate name that is failing. Most of the time we define patches in the workspace Cargo.toml, however there have been occasion to put them into the .cargo/config.toml for various reasons.

The .cargo/config.toml is not a common location to look for patches, it would be helpful if the error message could point to where the patch is being specified.

Proposed Solution

Cargo gives more detail for error messages involving [patch]es. Specifically the location of the [patch] entry would quickly help pinpoint where to fix the issue.

Notes

No response

[epage]

Could you provide reproduction steps for an error, including an example error message?

[kurtlawrence]

I wasn't able to get the ambiguity error I was seeing, but I can trivially produce an error if I specify a patch version that doesn't match:

[patch.crates-io.serde]
git = "https://github.com/serde-rs/serde"
version = "1.0.203"

Error:

    Updating git repository `https://github.com/serde-rs/serde`
error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index`

Caused by:
  patch for `serde` in `https://github.com/rust-lang/crates.io-index` failed to resolve

Caused by:
  The patch location `https://github.com/serde-rs/serde` contains a `serde` package with version `1.0.202`, but the patch definition requires `^1.0.203`.
  Check that the version in the patch location is what you expect, and update the patch definition to match.

The main thing that I think is lacking is where the patch is specified:

This would be ideal, but would depend if there is span info available. Otherwise just the location of the TOML file would be useful.

    Updating git repository `https://github.com/serde-rs/serde`
error: failed to resolve patches for `https://github.com/rust-lang/crates.io-index`

Caused by:
  patch for `serde` in `https://github.com/rust-lang/crates.io-index` failed to resolve

Caused by:
  The patch location `https://github.com/serde-rs/serde` contains a `serde` package with version `1.0.202`, but the patch definition requires `^1.0.203`.
  Check that the version in the patch location is what you expect, and update the patch definition to match.

Help:
  The patch entry is specified here:
in `.cargo/config.toml`
3 | [patch.crates-io.serde]
4 | git = "https://github.com/serde-rs/serde"
5 | version = "1.0.203"
               ^^^^^^^

[epage]

As I said, could you provide a complete reproduction?

[kurtlawrence]

My apologies

Cargo.toml

[package]
name = "tmp"
version = "0.1.0"
edition = "2021"

[dependencies]
serde = "1"

.cargo/config.toml

[patch.crates-io.serde]
git = "https://github.com/serde-rs/serde"
version = "1.1.0"

src/main.rs

fn main() {
    println!("Hello, world!");
}

[weihanglo]

The error was emitted from this piece of code:

https://github.com/rust-lang/cargo/blob/ac8c6aba13c07e8e94940a3c465f1f52294832ea/src/cargo/core/registry.rs#L993-L1011

It might be possible to attach a span information with orig_patch, though that would be quite challenging.

• [patch] can be defined in both Carg.toml and Cargo configurations. Currently Cargo has only a rudimentary diagnostics system. It's around the parsing of Cargo.toml at this moment. .cargo/config.toml hasn't yet covered.
• The error is not emitted during parsing those TOML files. It was done during the phase of the dependency resolution preparation. The current diagnostics system is not available in that phase.

[epage]

Not having looked at the code, would it be possible to track any of

• If it came from config or manifest
• Which config
• Which manifest

Those might be smaller steps that could offer most of what is needed here without going to the full extent of tracking spans.

[weihanglo]

Not having looked at the code, would it be possible to track any of

• If it came from config or manifest
• Which config
• Which manifest

The patch dep given to registry::patch() is directly from ws.root_patch().

https://github.com/rust-lang/cargo/blob/cabd5eb2762a87aa8e6657dbd349166def0f0f06/src/cargo/ops/resolve.rs#L804-L810

We could extend Workspace::root_patch() to preserve the location information (something like context::Definition + manifest path), pass it down to registry::patch(). Within registry::patch() it tracks the relationship between a patch dep and the location information. A patch Dependency value should be unique. If not, it might be duplicate and we should report all of them regardless the location.

[weihanglo]

I have the idea just by looking at the code. Mark this as S-accepted but mind that it needs some more code explorations and a brave heart. Patching implementation in Cargo is notoriously messy.

@rustbot label S-accepted

[omskscream]

I've noticed that the pull request for this issue has been closed without merging about a year ago. Is this issue still relevant and needs work?

[weihanglo]

#14473 was not finished. This issue is resolved yet.