unrelated dependency changed in Cargo.lock when updating another dependency

[xxchan]

Problem

This dependabot PR updates lz4, but Cargo.lock for (one of the) opendal is also updated. risingwavelabs/risingwave#17207

From cargo tree -i lz4, they seem to be not related.

I thought it's dependabot's issue, but I can also reproduce this locally.
I also found that only modifying the Cargo.toml will trigger this change, but not cargo update -p lz4

Steps

1. git clone https://github.com/risingwavelabs/risingwave (HEAD as of the post is af8f9a5815388c6d3147bebda77a1830a05f8c7e)
2. modify src/storage/Cargo.toml lz4 1.24.0 -> 1.25.0

Possible Solution(s)

No response

Notes

No response

Version

❯ cargo version --verbose
cargo 1.78.0-nightly (a4c63fe53 2024-03-06)
release: 1.78.0-nightly
commit-hash: a4c63fe5388beaa09e5f91196c86addab0a03580
commit-date: 2024-03-06
host: aarch64-apple-darwin
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.4.0 (sys:0.4.72+curl-8.6.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Mac OS 14.4.1 [64-bit]

[xxchan]

It happens again 🤔

risingwavelabs/risingwave#17162

[weihanglo]

Could you provide the exact Git revision in Steps for completeness?

[weihanglo]

I suspect the problem happened like this:

1. icelake depends on opendal with the requirement >=0.40.0.
2. Cargo.toml was edited manually so the entire crates.io source is “unlocked”, to allow Cargo to pick a newer version of lz4 without being locked in Cargo.lock. See this comment, which is a heavy hammer that you can consider it is the culprit.
3. Cargo.lock in https://github.com/risingwavelabs/risingwave/commits/af8f9a5815388c6d3147bebda77a1830a05f8c7e contains two version of opendal — 0.45.1 and 0.47.0. When the crates.io source was locked, Cargo prefers MaximumVersionsFirst so picked 0.47.0.

I'll try building a minimal repro.

[weihanglo]

BTW, there is a manual way to prevent this from happening:

1. Run cargo update lz4 --precise 1.25.0 first.
2. Edit Cargo.toml to lz4 = "1.25.0" later.

I also wonder whether the unstable feature cargo update --breaking can handle this flawlessly.

[weihanglo]

This is a minimal repro. As you can see, [email protected] is missing in the late lockfile, due to an edit of lz4 in Cargo.toml.

#[cargo_test]
fn manually_edit_manifest() {
    Package::new("lz4", "0.1.0").publish();
    Package::new("lz4", "0.2.0").publish();
    Package::new("opendal", "0.45.1").publish();
    Package::new("icelake", "0.0.10")
        .dep("opendal", ">=0.40.0")
        .publish();
    Package::new("risingwave_object_store", "1.0.0")
        .dep("opendal", "0.45.1")
        .publish();
    let p = project()
        .file(
            "Cargo.toml",
            r#"
                [package]
                name = "foo"
                edition = "2015"

                [dependencies]
                lz4 = "0.1.0"
                icelake = "0.0.10"
                risingwave_object_store = "1.0.0"
            "#,
        )
        .file("src/lib.rs", "")
        .build();

    p.cargo("fetch").run();

    assert_e2e().eq(p.read_lockfile(), str![[r##"
# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 3

[[package]]
name = "foo"
version = "0.0.0"
dependencies = [
 "icelake",
 "lz4",
 "risingwave_object_store",
]

[[package]]
name = "icelake"
version = "0.0.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0ced47b8e3f022dbdd8e45550820642f9bb85531b8c5ccd4f609fee3a1c51631"
dependencies = [
 "opendal",
]

[[package]]
name = "lz4"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "58d613dfd0b895188c526409f83e59a2e4e886fcdf1784e1ec26803c410081c0"

[[package]]
name = "opendal"
version = "0.45.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bfeff569181f1f3614b3d297f6ddb9ff510d07128832af820e3fa01fe6d38d21"

[[package]]
name = "risingwave_object_store"
version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c33db1e82767ae8b74d1ddb9eefc2e1b65992bb0f7bcef8b3300a0a3ee768bd7"
dependencies = [
 "opendal",
]

"##]]);

    Package::new("opendal", "0.47.0").publish();
    Package::new("risingwave_object_store", "1.1.0")
        .dep("opendal", "0.47.0")
        .publish();

    // This is to simulate duplicate versions.
    p.cargo("update risingwave_object_store").run();

    // This is a slimmed-down version of the current state of lockfile in
    // https://github.com/risingwavelabs/risingwave/commits/af8f9a5815388c6d3147bebda77a1830a05f8c7e
    // * contain duplicate versions of opendal
    // * [email protected] still depends on [email protected]
    assert_e2e().eq(p.read_lockfile(), str![[r##"
# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 3

[[package]]
name = "foo"
version = "0.0.0"
dependencies = [
 "icelake",
 "lz4",
 "risingwave_object_store",
]

[[package]]
name = "icelake"
version = "0.0.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0ced47b8e3f022dbdd8e45550820642f9bb85531b8c5ccd4f609fee3a1c51631"
dependencies = [
 "opendal 0.45.1",
]

[[package]]
name = "lz4"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "58d613dfd0b895188c526409f83e59a2e4e886fcdf1784e1ec26803c410081c0"

[[package]]
name = "opendal"
version = "0.45.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bfeff569181f1f3614b3d297f6ddb9ff510d07128832af820e3fa01fe6d38d21"

[[package]]
name = "opendal"
version = "0.47.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9bc28c53e2d46f9af0f2c43ce4f5df096521e80f63470e06ecf3c1947f0b3347"

[[package]]
name = "risingwave_object_store"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "21b5ce67766299c7e9e7947d33e64fb7d55b7efe4d6920a5387a8eb3d1108b4f"
dependencies = [
 "opendal 0.47.0",
]

"##]]);

    // Update a completely unrelated package from the same source.
    p.change_file(
        "Cargo.toml",
        r#"
            [package]
            name = "foo"
            edition = "2015"

            [dependencies]
            lz4 = "0.2.0"
            icelake = "0.0.10"
            risingwave_object_store = "1.0.0"
        "#,
    );

    p.cargo("fetch").run();

    assert_e2e().eq(p.read_lockfile(), str![[r##"
# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 3

[[package]]
name = "foo"
version = "0.0.0"
dependencies = [
 "icelake",
 "lz4",
 "risingwave_object_store",
]

[[package]]
name = "icelake"
version = "0.0.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0ced47b8e3f022dbdd8e45550820642f9bb85531b8c5ccd4f609fee3a1c51631"
dependencies = [
 "opendal",
]

[[package]]
name = "lz4"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "feccb9b0fa40e850b1e70920bdf6ba7e10017c2c240bda19f9619da336126f25"

[[package]]
name = "opendal"
version = "0.47.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9bc28c53e2d46f9af0f2c43ce4f5df096521e80f63470e06ecf3c1947f0b3347"

[[package]]
name = "risingwave_object_store"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "21b5ce67766299c7e9e7947d33e64fb7d55b7efe4d6920a5387a8eb3d1108b4f"
dependencies = [
 "opendal",
]

"##]]);

}

[xxchan]

So this happens only when some crate uses >= dependency versions, do I understand it correctly? (Perhaps also <=, as I sometimes also see unrelated dependencies got downgraded?)

[xxchan]

I also wonder whether the unstable feature cargo update --breaking can handle this flawlessly.

It has the same problem. I just tried cargo +nightly -Zunstable-options update -p duration-str --breaking in the risingwave repo. opendal is updated in this case.

[weihanglo]

So this happens only when some crate uses >= dependency versions, do I understand it correctly? (Perhaps also <=, as I sometimes also see unrelated dependencies got downgraded?)

I believe it happens when the lockfile contains more than one version of a packages. As Cargo only allows one major version per package, yes to trigger this bug, you'll need to have a quite relax version requirement could pick across major versions.

[epage]

@weihanglo I was confused by this because of this comment:

cargo/src/cargo/ops/resolve.rs

Lines 568 to 571 in 6e23650

	// however, that we still want conservative updates. This currently happens
	// because the first candidate the resolver picks is the previously locked
	// version, and only if that fails to activate to we move on and try
	// a different version. (giving the guise of conservative updates)

It seems like we should just keep to the same version. However, I think I was overlooking how the resolver would just blindly see two compatible versions of the same package in the lockfile and pick one of the two. Either I forgot or hadn't considered that ramification of wider version requirements.

@xxchan

So this happens only when some crate uses >= dependency versions, do I understand it correctly? (Perhaps also <=, as I sometimes also see unrelated dependencies got downgraded?)

From my very brief look at icelake, I feel like the version requirements it uses are dubious within the cargo ecosystem. The ecosystem is built on maintaining semver guarantees but that package seems to be saying that it and its dependents don't care about semver.

btw we have guidelines on version requirements at:
https://doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html#multiple-requirements

[weihanglo]

@epage If you change the last cargo fetch call to cargo fetch -Z minimal-versions in the minimal repro, then the lower version 0.45.1 would be picked. That's the third point.

[xxchan]

From my very brief look at icelake, I feel like the version requirements it uses are dubious within the cargo ecosystem. The ecosystem is built on maintaining semver guarantees but that package seems to be saying that it and its dependents don't care about semver.

@epage I understand what you mean. However, I would say such usage is not that uncommon in the ecosystem.

e.g., prost-build uses itertools = { version = ">=0.10, <=0.12"} (itertools is indeed another dependency I observed got upgraded or downgraded randomly, same as the original issue).

https://github.com/tokio-rs/prost/blob/ba776540834b0cecbbaa6ca98bd79d9682cd7e92/prost-build/Cargo.toml#L20

Another one is arrow-rs. It does frequent major releases, and used in public APIs. And @Xuanwo proposed to use >= versions. You can see related discussions here: apache/arrow-rs#5368 (comment)

[epage]

itertools case is different that it has a semver upper bound. That is saying that it knows the breaking changes don't affect it. This case is even called out in the linked document, particularly for -sys crates.

Unbounded reqs are not common. I'm surprised these weren't banned with *.

[xxchan]

Thanks for clarifying the differences. BTW that seems a little off topic, i.e., whenever we have a "quite relax version requirement" (whether it's >=10 or ">=0.10, <=0.12"), the issue can happen.

[xxchan]

It seems cargo update can also trigger this.

Edit: oh, at least it since the changed part is used by opendal (#5529?)

> git rev-parse HEAD
c22c4265052c2a4f2876132a10a0b522ec7c03c9
> cargo update [email protected] --precise 0.47.2
> git diff

diff --git a/Cargo.lock b/Cargo.lock
index 9197e73c27..2b6dbe90ae 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2101,7 +2101,7 @@ dependencies = [
  "bitflags 2.5.0",
  "cexpr",
  "clang-sys",
- "itertools 0.12.1",
+ "itertools 0.10.5",
  "lazy_static",
  "lazycell",
  "log",
@@ -8440,9 +8440,9 @@ checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381"

 [[package]]
 name = "opendal"
-version = "0.47.0"
+version = "0.47.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5c3ba698f2258bebdf7a3a38862bb6ef1f96d351627002686dacc228f805bdd6"
+checksum = "ff159a2da374ef2d64848a6547943cf1af7d2ceada5ae77be175e1389aa07ae3"
 dependencies = [
  "anyhow",
  "async-trait",
@@ -9961,7 +9961,7 @@ dependencies = [
  "indoc",
  "libc",
  "memoffset",
- "parking_lot 0.12.1",
+ "parking_lot 0.11.2",
  "portable-atomic",
  "pyo3-build-config",
  "pyo3-ffi",