Cargo update ignores --precise in some cases when using `[patch]`

[64]

Problem
When [patch]ing a crate foo of version x.y.z (where x.y.z is not the most up-to-date version), running cargo update -p foo --precise x.y.z twice causes foo to be updated to the latest version (rather than x.y.z).

Steps

1. Clone libc to /tmp and downgrade to an old version (let's take 0.2.93) using: git checkout tags/0.2.93
2. cargo new foo
3. Modify foo/Cargo.toml:

[package]
name = "foo"
version = "0.1.0"
edition = "2018"

[dependencies]
libc = "0.2"

[patch.crates-io]
libc = { path = "/tmp/libc" } # Path to our 0.2.93 version of libc

4. Run:

$ cargo update -p libc --precise 0.2.93
    Updating crates.io index
warning: Patch `libc v0.2.93 (/tmp/libc)` 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.
      Adding libc v0.2.93 (/tmp/libc)
    Removing libc v0.2.94

5. Run again:

$ cargo update -p libc --precise 0.2.93
    Updating crates.io index
warning: Patch `libc v0.2.93 (/tmp/libc)` 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.
    Removing libc v0.2.93 (/tmp/libc)
      Adding libc v0.2.94

Note that the last two lines of output are different in steps 4 and 5.

Notes

Output of cargo version:

cargo 1.53.0-nightly (65d57e6f3 2021-04-04)

It appears that without [patch] everything behaves as expected - running the same update more than once has no effect.

This may seem like an odd set of circumstances. I would be happy to explain why we run into this situation if it helps.

[64]

Is anyone with more knowledge able to confirm that this is a bug, or is it somehow intended? I would be happy to prepare a fix, given a few pointers in the right direction.

[ehuss]

Removing the patch is a bug. I don't know exactly what is going on, but in general dealing with patch issues is notoriously difficult. However, I don't want to discourage you, just to set the expectations of how difficult it will be.

I would start investigating here where the precise versions are added to the to_avoid list. If I had to guess, it may have something to do with the way it is checking that the dep is a registry source. I would then continue the investigation into resolve_with_previous which takes the existing Cargo.lock and runs the resolver constrained to those versions. There is a bunch of subtle patch-related stuff going on there. Going further down the hole, you'll land in PackageRegistry. This is the "front end" for queries from the resolver, and where all the patches "overlay" the registry. The patch and query functions have a lot of patch handling code, and is where the final choice to use a patch is made.

[64]

Hm, it seems that keep returns false on the patch (even though the comment implies that -p should force keep to be true), but I suppose the issue is further up the chain. Will investigate more...

Nevermind -- that all seems to be working as expected. Though, I have a suspicion that this filter is responsible - it seems to give us far more candidates than we expect.

[64]

source_id.precise() here is None -- that seems wrong to me (when doing the update it for the first time in step 4, it is Some).

[64]

Alright, I'm going to give up trying to fix this bug (we will work around it downstream in the meantime), but here's what I suspect is causing the problem for future reference:

1. When a patch exists in the lockfile, we put precise() on the patched source here
2. When we try to resolve the dependency later, we grab the registry source (not the patched source) here
3. Since we have set precise() on the patched source rather than the registry, when we reach here the source_id.precise() is None and it effectively ignores our --precise.

I suppose the proper fix to this would be to completely ignore any patched sources when reading from the existing lockfile, but that may cause a lot of other breakages. The code is very complicated so I'll leave it for now and good luck to anyone who manages to tackle this :)

FWIW this seems to be related to @alexcrichton's comment on #8355.