search

paritytech / pipeline-scripts

Shared scripts for usage in CI pipelines
Apache License 2.0
2 stars 3 forks source link

check_dependent_project: Failed to detect ambiguous dependency reference in companion PR #61

Closed joao-paulo-parity closed 1 year ago

joao-paulo-parity commented 2 years ago

Problem: In https://github.com/paritytech/polkadot/pull/5731, the initial commit's lockfile had two windows-sys

[[package]]
name = "windows-sys"
version = "0.32.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3df6e476185f92a12c072be4a189a0210dcdcf512a1891d6dff9edb874deadc6"
dependencies = [
 "windows_aarch64_msvc 0.32.0",
 "windows_i686_gnu 0.32.0",
 "windows_i686_msvc 0.32.0",
 "windows_x86_64_gnu 0.32.0",
 "windows_x86_64_msvc 0.32.0",
]

[[package]]
name = "windows-sys"
version = "0.36.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ea04155a16a59f9eab786fe12a4a450e75cdb175f9e0d80da1e17db09f55b8d2"
dependencies = [
 "windows_aarch64_msvc 0.36.1",
 "windows_i686_gnu 0.36.1",
 "windows_i686_msvc 0.36.1",
 "windows_x86_64_gnu 0.36.1",
 "windows_x86_64_msvc 0.36.1",
]

However one of the packages didn't qualify which one of the two versions to use, as shown in https://github.com/koute/polkadot/blob/8ff2627f00c88e5f0997b44f444b67d3a83d3444/Cargo.lock#L6054

[[package]]
name = "parking_lot_core"
version = "0.9.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "28141e0cc4143da2443301914478dc976a61ffdb3f043058310c70df2fed8954"
dependencies = [
 "cfg-if 1.0.0",
 "libc",
 "redox_syscall",
 "smallvec",
 "windows-sys", # HERE
]

The follow-up commit had to fix that in https://github.com/paritytech/polkadot/pull/5731/commits/2abfa1c7435b57155aa28ecc106f547539b454c7#diff-13ee4b2252c9e516a0547f2891aa2105c3ca71c6d7a1e682c69be97998dfc87eR6054.

The problem here is that the integration check didn't catch the invalid lockfile before merge.

Solution: check the lockfile more thoroughly for ambiguous dependencies. Need to figure out why neither cargo check nor cargo metadata complained in that case.

joao-paulo-parity commented 2 years ago

windows-sys didn't show up in the logs of https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1651556/raw (from https://github.com/paritytech/substrate/pull/11722) despite sc-executor-wasmtime being used. I infer it's because it's a dependency for Windows and the CI runner was running Ubuntu. That being said, something inside of cargo_lock - which I still haven't figured out what it is exactly - caught a problem which neither cargo metadata nor cargo check ran into, so there must be a lead there. If for some reason cargo-lock doesn't suffice, another option would be to use cargo's APIs directly.

So far I haven't been able to reproduce that problem, but at the same time I didn't spend a lot of time on it; I still expect to make some progress on this soon.

joao-paulo-parity commented 2 years ago

Related cargo_lock ticket: https://github.com/rustsec/rustsec/issues/606

V2+ lockfiles need to resolve dependencies based on the minimum amount of the (package_name, version, source_id) tuple which is needed to unambiguously identify them.

More context for a possible solution was provided in https://github.com/rustsec/rustsec/issues/606#issuecomment-1189472257

mordamax commented 1 year ago

IMO Not a job of check-dependent