-
Notifications
You must be signed in to change notification settings - Fork 312
enhance Cargo easyblock to merge Cargo.toml file with workspace file
#4016
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enhance Cargo easyblock to merge Cargo.toml file with workspace file
#4016
Conversation
Implement a more complete TOML parser in Cargo easyblock Loosely parse everything that could be in a Cargo.toml file Use it to resolve backreferences like `authors.workspace = true`
EasyBuild contains facilities to read and write TOML files so remove the manual TOML parser and implement the workspace/crates merging using those. This allows for a lot more flexibility and avoids errors caused by unexpected formats of a TOML file.
When inheriting from a workspace value we need to take the original dict and replace any keys that are present in the child dict.
See https://doc.rust-lang.org/cargo/reference/workspaces.html#the-lints-table Factor out the merge function to avoid duplicating it
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 118 out of 120 (total: 18 hours 39 mins 36 secs) (115 easyconfigs in total) |
|
SnapATAC2 won't work, see also: |
|
@boegelbot please test @ jsc-zen3 |
|
@boegel: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3649590336 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 123 out of 126 (total: 17 hours 31 mins 45 secs) (120 easyconfigs in total) |
|
As for SnapATAC: easybuilders/easybuild-easyconfigs#24643 Would be good to get that and dependencies in too |
|
|
@boegelbot please test @ jsc-zen3 |
|
@boegel: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3650689258 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (total: 15 mins 50 secs) (1 easyconfigs in total) |
|
Problem with I suspect that the changes there unmasked another bug, because the edit: Since the problem also pop up when not using this updated |
boegel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Cargo easyblock to merge Cargo.toml file with workspace file
@boegel, for some reason, the current directory is set to # Shell for the command: 'cd helix-term && cargo install --profile=release --root=/tmp/software/helix/25.07.1-GCCcore-14.3.0 --path=.'
# Use command history, exit to stop
eb-shell> pwd
/data/EasyBuild-develop/build/helix/25.07.1/GCCcore-14.3.0/helix-stdx
eb-shell> ls ../helix-term/
build.rs Cargo.toml src tests
eb-shell> cd ../helix-term/
eb-shell> cargo install --profile=release --root=/tmp/software/helix/25.07.1-GCCcore-14.3.0 --path=.
Installing helix-term v25.7.1 (/data/EasyBuild-develop/build/helix/25.07.1/GCCcore-14.3.0/helix-term)
[...]Also confirmed in the EasyBuild logs: |
|
@Flamefire, @boegel The final path is then set to the non-deterministic subfolder, therefore breaking parts which rely on being in one of the specific folders, or in the top level. |
Resolve backreferences like
authors.workspace = truehttps://doc.rust-lang.org/cargo/reference/workspaces.html#the-package-table
For ruff-0.14.3 the salsa crate is using the
[workspace.package]section where one can define values that can be inherited by membersSo the members
Cargo.tomlfile contains e.g.For the offline builds we need to copy the members to standalone packages which has the side-effect that no "workspace" exists anymore causing
cargoto fail withChecking
cargo vendoroutput: It replacesversion.workspace = truebyversion = "1.2.3"Hence we need to parse both TOML files, "merge" them to replace those references and write the result to the member
Cargo.tomlThis PR supersedes #3988 by using:
tomllibfrom Vendor a copy oftomliineasybuild.tools.tomllibeasybuild-framework#5063dump_tomlfrom Vendor copy of tomli-w 1.2.0, use it to implementdump_tomlfunction ineasybuild.tools.filetoolseasybuild-framework#5071For future reference I kept that PR as a single, squashed commit.
With that it supports all examples from the docs including merging dictionaries and sections which isn't feasible without a full parser and serializing to TOML
The tests check the parsed results, not their textual representation as e.g. comments and formatting get lost