Fix build system hanging due to conflicting version features #362
Conversation
|
Thanks for the contribution! |
contrib/update-lock-files.sh
Outdated
| (cd node && cargo check --features=29_0) | ||
| # verify crate is excluded from workspace, handle separately | ||
| (cd verify && cargo check) | ||
| # Skip integration_test as it cannot use --all-features |
There was a problem hiding this comment.
integration_test can be checked the same as node (and for the same reason).
What about using lists of crate names and then a for loop, might be cleaner? I don't see the reason to have verify separate.
justfile
Outdated
| cargo check -p corepc-types --all-targets --all-features | ||
| cargo check -p jsonrpc --all-targets --all-features | ||
| cargo check -p corepc-node --all-targets --features=29_0 | ||
| (cd verify && cargo check --all-targets) |
There was a problem hiding this comment.
We could use cargo check --manifest-patch verify/Cargo.toml instead to save cd'ing into verify.
0xZaddyy
force-pushed
the
fix-build-features
branch
from
b8b7fdf to
493bf2f
Compare
0xZaddyy
force-pushed
the
fix-build-features
branch
2 times, most recently
from
d6f6bbb to
471e0e1
Compare
|
Although I can't reproduce the issue I still agree it's better to remove It's not a great idea having the version hard coded in the scripts, could we instead define a feature Also remove the comment from |
|
I like the |
There was a problem hiding this comment.
Looks good, but can you change the order of the two commits so that the latest feature is added to the Cargo.toml file first and then in the second commit you can use latest in the changes to the scripts. Otherwise you set it to 29.0 then immediately change it to latest.
0xZaddyy
force-pushed
the
fix-build-features
branch
from
7fca458 to
4f1f741
Compare
The previous build configuration used `--all-features` at the workspace level, which caused conflicts with mutually exclusive version features in the node and integration_test crates (e.g., v29_0, v28_2, etc.).
0xZaddyy
force-pushed
the
fix-build-features
branch
from
4f1f741 to
d267de8
Compare
|
Hey @jamillambert good now? |
There was a problem hiding this comment.
The idea was to have the first commit define the latest feature and the second use it. Currently the two commits are mixed up in what they do. The first makes changes to both the update-lock-file.sh script and the justfile, neither of which are described in the commit log.
Can you try again?
justfile
Outdated
|
|
||
| # Cargo build everything. | ||
| build: | ||
| <<<<<<< HEAD |
There was a problem hiding this comment.
What is this from? your merge editor?
|
You still have merge conflicts in patch 1 mate. |
This PR addresses #361
The
just update-lock-filescommand was hanging because it used--all-featureswhich enabled mutually exclusive version features in the node crate simultaneously (29_0, 28_2, 28_1, etc.), causing Cargo to get stuck resolving impossible dependencies.Changes:
--all-featuresfeatures=29_0for node crate (latest supported version)cdcommands