Relax dcou dep. in order-crates-for-publishing.py #32432
Conversation
ci/order-crates-for-publishing.py
Outdated
| def is_special_cased_self_dep(package, dependency): | ||
| is_special_cased = False | ||
|
|
||
| # Sometimes self-dev-dep with dev-context-only-utils is needed and this dep |
There was a problem hiding this comment.
The use of "self-dev-dep" in this comment throws me off. Maybe it can be expanded or more context added.
It looks like is_special_cased_self_dep only cares about the dev-context-only-util crate, so I'd rather it be more explicit in the function name that this is really only about that one crate
There was a problem hiding this comment.
haha, no wonder for throwing you off. the proper commenting resulted in quite a high wall of text: 15cf9c4
yeah, i needed fresh eyes to spell the hidden context around dev-context-only-utils out. thanks for review. :)
yeah, we can. fyi, the new code in this pr is restricting to the However, i wonder the merit of the extra hassle towards generalization... as a matter of fact, seems no one complained for years. also, note that this script is best-effort, namely it can only detect A <=> B; it can't for A => B => C => A.... I'd like to blind here. ;)
heh, you hate python? no objection for the port for fun. but i want to ship this firstly to unblock #32428 and #31239.... |
ryoqun
force-pushed
the
dcou-order-crates-for-publishing
branch
from
e237615 to
15cf9c4
Compare
ci/order-crates-for-publishing.py
Outdated
| # it's likely `{ workspace = true, ... }` is used, which implicitly pulls the | ||
| # version in... | ||
| sys.exit( | ||
| 'Error: wrong dev-context-only-utils circular dependency. try: ' + | ||
| '{} = {{ path = ".", features = {} }}\n' | ||
| .format(dependency['name'], json.dumps(dependency['features'])) | ||
| ) |
There was a problem hiding this comment.
can we collect the crates that fail this check and print them at the end instead of early-returning? spare someone the demoralizing check-fail-fix cycles 😉
There was a problem hiding this comment.
phew, you rightfully spotted my laziness: 44e6513
Problem
Recently,
order-crates-for-publishing.pybite us (or me, specifically, lol) for rejecting a valid circular dep which has been legalized at rust-lang/cargo#7333 (at rust 1.40)as for the underlying problem, unfortunately, there's a years-long extremely muddying affair regarding circular dev-dep. and lack of grouped (i.e.
workspace-d)cargo publish, our tendency of bench code placement (under./benches) and#[cfg(test)]interaction, never-stabilizedbenches, no official equivalent to#[cfg(feature = "dev-context-only-utils")], andcargo hack-based publish and its drawbacks..... YES, I'm tortuning myself for high standards of our coding practice. ;)I don't spend writing all the findings. for bored, you can start your rabbit hole starting at these links:
rust-lang/cargo#4242
rust-lang/cargo#7333
rust-lang/rust#84629
rust-lang/rust#67295
rust-lang/crates.io#1789
getsentry/sentry-rust#374
https://users.rust-lang.org/t/conditional-compilation-for-benchmarks/47227/2
https://users.rust-lang.org/t/what-are-the-rules-for-cfg-test/54122/2
https://stackoverflow.com/questions/57607182/equivalent-of-cfgtest-for-benchmarks
tldr: addressing this correctly isn't viable. there's a hope for quite acceptable yet another work-around, if you ignore all the unsatisfying realities. :)
This actually
cargo publish-safe dep pattern is needed as one ofdev-context-only-utilsusage, specifically blocking #32428.Summary of Changes
Actually, we just need to relax
order-crates-for-publishing.pyin rather very conditioned basis (to reduce any risks incargo publish-ing). in a say, the script was too strict, causing false-positive errors.