eopb / cargo-override

The quickest way to override dependencies with Cargo
Apache License 2.0
34 stars 4 forks source link

Ensure that patch version is selected by lockfile after patching #106

Open eopb opened 2 weeks ago

eopb commented 2 weeks ago

Problem

Quoting from https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#testing-a-bugfix:

The way [patch] works is that it’ll load the dependency at ../path/to/uuid and then whenever crates.io is queried for versions of uuid it’ll also return the local version.

This means that the version number of the local checkout is significant and will affect whether the patch is used. Our manifest declared uuid = "1.0" which means we’ll only resolve to >= 1.0.0, < 2.0.0, and Cargo’s greedy resolution algorithm also means that we’ll resolve to the maximum version within that range. Typically this doesn’t matter as the version of the git repository will already be greater or match the maximum version published on crates.io, but it’s important to keep this in mind!

...

...If you don’t see the local path version getting built then you may need to run cargo update uuid --precise $version where $version is the version of the locally checked out copy of uuid.

This means that it's possible for a patch created with cargo-override to not be selected, despite the version compatibility checks we do. This is due to cargo selecting the latest version with little regard to where it comes from.

Suggested solution

First we need to reproduce this issue. I don't want to risk trying to fix something without demonstrating that cargo works in ways that match our assumptions. This test could just be done on a local machine, and once done, a summary posted here.

For the solution, we can automate the guidance from the article above. We already have the exact version of the patch, so a call to cargo update <pkgid> --precise <version> should be possible. We shouldn't need to check the lockfile before doing this, since the precise update should be a no-op on correct lockfiles. We can do this for every call to cargo-override.

Open questions

What should we use to call cargo update?

eopb commented 2 weeks ago

Note, we have a --locked flag which "Assert that Cargo.lock will remain unchanged".

If this flag is passed, we should not attempt to update the lockfile. Ideally, we should emit a warning if the versions don't align, and --locked is passed, but we can keep that outside of the scope of this issue.

epage commented 1 week ago

Technically, a patch will change Cargo.lock, so --locked should always fail unless the patch is already applied.