anza-xyz / agave

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://www.anza.xyz/
Apache License 2.0
361 stars 163 forks source link

Circular dependency check in CI is wrong about dev-dependencies #2428

Closed kevinheavey closed 1 month ago

kevinheavey commented 2 months ago

Problem

It's fine to have apparent circular dependencies if they're just dev-dependencies [UPDATE: they must also be path dependencies], but order-crates-for-publishing.py will still fail in this case. ~Unfortunately cargo metadata doesn't yet have a way to ignore dev-dependencies: https://github.com/rust-lang/cargo/issues/10718~

Proposed Solution

~I'll open this one up to the floor, but I see the Python script has a dependency['kind'] == 'dev' check in there - maybe we can just use this check to filter out all dev deps?~ Filter out path dev-dependencies in order-crates-for-publishing.py

kevinheavey commented 2 months ago

I assumed circular dev deps were fine because cargo test is fine with them but looks like cargo publish is not fine with them? https://github.com/rust-lang/cargo/issues/4242

kevinheavey commented 2 months ago

More investigation reveals that cargo publish is fine with circular dev deps if those dev deps are path dependencies

Example repo here: https://github.com/kevinheavey/circular-dev-dep

joncinque commented 1 month ago

Was this addressed with #2578?

kevinheavey commented 1 month ago

Yes