Skip to content
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

update Cargo easyblock to extract crates into a vendor subdir and overwrite git repo URLs with local paths #3118

Merged
merged 7 commits into from Feb 7, 2024

Conversation

lexming
Copy link
Contributor

@lexming lexming commented Jan 31, 2024

This evolved into a 2x1 PR, apologies for the added complexity.

Fix 1: extract crates into vendor subdir

Currently, the sources of the main Cargo package and the sources of all crates are all extracted into builddir. This is known to make some installations fail. The presence of the main sources alongside its dependencies can make cargo trip in identifying availability of vendored sources. See related comment below.

Not all Cargo packages are affected, but it seems to happen on more complex packaging. For instance, known failure is a package with a virtual workspace distributing multiple crates plus a another package in a subdir.

This PR solves this issue by extracting the list of crates into a separate vendor subdir without any sources of the main packages (i.e. items in the sources list of the easyconfig). This follows the same approach carried out by the cargo vendor command.

Fix 2: overwrite crates from git repos with local paths

I hit another issue where cargo was failing to locate the extracted sources of a dependency pulled from a git repo. The sources are properly downloaded and extracted from the git repo and the checksuming of the local sources is done properly. However, Cargo.lock of the main package being installed does not list any checksum for those dependencies pulled from a git repo, so cargo has no way to correlate the extracted folder to this dependency.

This PR solves this issue by overwriting the repo URL of each package pulled from a git repo with the local paths to the corresponding extracted folders. This is done in the generated .cargo/config.toml file by using the [patch.URL] directive as describe in the rust docs instead of [source.URL].

For instace,

[source."https://github.com/ritchie46/jsonpath"]
git = "https://github.com/ritchie46/jsonpath"
rev = "24eaf0b4416edff38a4d1b6b17bc4b9f3f047b4b"

becomes

[patch."https://github.com/ritchie46/jsonpath"]
jsonpath_lib = { path = "/user/brussel/101/vsc10122/easybuild/install/skylake/build/polars/0.19.19/gfbf-2023a/easybuild_vendor/jsonpath" }

Changelog

  • extract crates into self.builddir.easybuild_vendor
  • add static methods to generate filenames for downloaded tarballs and local tarballs
  • be smarter in setting finalpath of extracted sources by detecting
  • use global templates to generate .cargo/config.toml file
  • replace [source.URL] with [patch.URL] to overwrite git repos with local paths to extracted sources

@lexming
Copy link
Contributor Author

lexming commented Jan 31, 2024

@boegelbot please test @ generoso
EB_ARGS="maturin-1.1.0-GCCcore-12.3.0.eb maturin-1.4.0-GCCcore-12.3.0-Rust-1.75.0.eb cryptography-41.0.1-GCCcore-12.3.0.eb bcrypt-4.0.1-GCCcore-12.3.0.eb pydantic-2.5.3-GCCcore-12.3.0.eb"

@boegelbot
Copy link

@lexming: Request for testing this PR well received on login1

PR test command 'EB_PR=3118 EB_ARGS="maturin-1.1.0-GCCcore-12.3.0.eb maturin-1.4.0-GCCcore-12.3.0-Rust-1.75.0.eb cryptography-41.0.1-GCCcore-12.3.0.eb bcrypt-4.0.1-GCCcore-12.3.0.eb pydantic-2.5.3-GCCcore-12.3.0.eb" EB_CONTAINER= EB_REPO=easybuild-easyblocks /opt/software/slurm/bin/sbatch --job-name test_PR_3118 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 12784

Test results coming soon (I hope)...

- notification for comment with ID 1919188427 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link

Test report by @boegelbot

Overview of tested easyconfigs (in order)

  • SUCCESS maturin-1.1.0-GCCcore-12.3.0.eb
  • SUCCESS maturin-1.4.0-GCCcore-12.3.0-Rust-1.75.0.eb
  • SUCCESS cryptography-41.0.1-GCCcore-12.3.0.eb
  • SUCCESS bcrypt-4.0.1-GCCcore-12.3.0.eb
  • SUCCESS pydantic-2.5.3-GCCcore-12.3.0.eb

Build succeeded for 5 out of 5 (5 easyconfigs in total)
cns2 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/boegelbot/2154f0c6595af34a3e0feb6a1610a3b7 for a full test report.

@lexming lexming requested a review from Micket January 31, 2024 16:28
@lexming lexming changed the title Place extracted dirs of crates into a vendor subdir without main package sources Extract crates into a vendor subdir and overwrite git repo URLs with patch directive Feb 1, 2024
@lexming lexming changed the title Extract crates into a vendor subdir and overwrite git repo URLs with patch directive Extract crates into a vendor subdir and overwrite git repo URLs with local paths Feb 1, 2024
@lexming
Copy link
Contributor Author

lexming commented Feb 1, 2024

@boegelbot please test @ generoso
EB_ARGS="maturin-1.1.0-GCCcore-12.3.0.eb maturin-1.4.0-GCCcore-12.3.0-Rust-1.75.0.eb cryptography-41.0.1-GCCcore-12.3.0.eb bcrypt-4.0.1-GCCcore-12.3.0.eb pydantic-2.5.3-GCCcore-12.3.0.eb"

@boegelbot
Copy link

@lexming: Request for testing this PR well received on login1

PR test command 'EB_PR=3118 EB_ARGS="maturin-1.1.0-GCCcore-12.3.0.eb maturin-1.4.0-GCCcore-12.3.0-Rust-1.75.0.eb cryptography-41.0.1-GCCcore-12.3.0.eb bcrypt-4.0.1-GCCcore-12.3.0.eb pydantic-2.5.3-GCCcore-12.3.0.eb" EB_CONTAINER= EB_REPO=easybuild-easyblocks /opt/software/slurm/bin/sbatch --job-name test_PR_3118 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 12797

Test results coming soon (I hope)...

- notification for comment with ID 1920915926 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link

Test report by @boegelbot

Overview of tested easyconfigs (in order)

  • SUCCESS maturin-1.1.0-GCCcore-12.3.0.eb
  • SUCCESS maturin-1.4.0-GCCcore-12.3.0-Rust-1.75.0.eb
  • SUCCESS cryptography-41.0.1-GCCcore-12.3.0.eb
  • SUCCESS bcrypt-4.0.1-GCCcore-12.3.0.eb
  • SUCCESS pydantic-2.5.3-GCCcore-12.3.0.eb

Build succeeded for 5 out of 5 (5 easyconfigs in total)
cns1 - Linux Rocky Linux 8.9, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/boegelbot/44d1ead4c616ace3bf7fa7da4143f94f for a full test report.

@lexming
Copy link
Contributor Author

lexming commented Feb 1, 2024

@lexming
Copy link
Contributor Author

lexming commented Feb 1, 2024

Test case for fix2 - polars:

polars has jsonpath_lib pulled from a git repo

@lexming
Copy link
Contributor Author

lexming commented Feb 1, 2024

@Micket this is ready on my side, not touching it any more

@boegel boegel changed the title Extract crates into a vendor subdir and overwrite git repo URLs with local paths update Cargo easyblock to extract crates into a vendor subdir and overwrite git repo URLs with local paths Feb 1, 2024
Copy link
Contributor

@Micket Micket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Micket Micket merged commit 836f08d into easybuilders:develop Feb 7, 2024
47 checks passed
@Micket Micket modified the milestones: 4.x, release after 4.9.0 Feb 7, 2024
@lexming lexming deleted the cargo-vendor branch February 8, 2024 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants