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.

Sign up for GitHub

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

Jump to bottom

enhance cargo easyblock for sources from git repositories #3483

Merged
merged 2 commits into from
Nov 7, 2024
Merged

enhance cargo easyblock for sources from git repositories #3483

merged 2 commits into from
Nov 7, 2024

Conversation

Flamefire
Copy link
Contributor

This combines #3481 and #3482 to test that they work together for EasyConfigs that would fail without.

They can be merged individually which might be easier to review or just this one.

Closes #3481
Closes #3482

@Flamefire
avoid name clash in cargo crates downloaded from git repositories
6bd93b5 
Crates referenced by their git revision have a version but might have
different revisions.
The source filename ignored the revision such that sources from
different revisions were considered the same file which either fails the
checksum verification or the build.
Append the revision if specified to disambiguate them.
@Flamefire
Handle cargo workspaces in git repositories
b88ab29 
Crates downloaded from crates.io can just be put into the
vendored-sources directory.
However a referenced git repository might contain multiple crates in a
so-called "workspace".
If we put such a workspace into the vendor folder, cargo fails with
> found a virtual manifest at [...]Cargo.toml instead of a package manifest

This solution uses a separate folder for sources downloaded from git
repositories and checks them for workspaces.
If they are "regular", i.e. don't have a workspace, they are put to the
vendored sources from crates.io.
Otherwise they are left in the git-vendor directory.

Additionally refine the config.toml entry for sources vendored from git.
This allows to override a specific git URL and revision with the path to
the extracted repository.
Otherwise we would need entries for each crate in workspaces but then
cargo tries to verify the revision and fails with
> Unable to update https://github.com/[...]?rev=[...]
> Caused by: can't checkout from 'https://github.com/[...]': you are in the offline mode (--offline)
@Flamefire
Copy link
Contributor Author

Test report by @Flamefire

Overview of tested easyconfigs (in order)

  • SUCCESS uv-0.2.30-GCCcore-13.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
i7016 - Linux Rocky Linux 8.9 (Green Obsidian), x86_64, AMD EPYC 7702 64-Core Processor (zen2), Python 3.8.17
See https://gist.github.com/Flamefire/d91ecc4728b15a21cdad664178436018 for a full test report.

This was referenced Oct 14, 2024
Open
Open
Micket
Micket approved these changes Nov 7, 2024
View reviewed changes
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.

it seems you thought this through, so looks fine to me. I will run some tests.

@Micket Micket added this to the release after 4.9.4 milestone Nov 7, 2024
@Micket
Copy link
Contributor

Micket commented Nov 7, 2024

Hm, i'm having trouble with the uv package you link

== 2024-11-07 15:59:09,477 easyblock.py:815 INFO Considering directory in which easyconfig file is located when searching for reqwest-middleware-0.3.2-21ceec9a5fd2e8d6f71c3ea2999078fecbd13cbe.tar.gz: /dev/shm/eb-04l76vze/files_pr21637/u/uv
== 2024-11-07 15:59:09,477 tools.py:282 INFO Found location to 'eb' script via $EB_SCRIPT_PATH: /apps/Common/software/EasyBuild/4.9.4/bin/eb
== 2024-11-07 15:59:09,477 tools.py:296 INFO Also considering installation prefix /apps/Common/software/EasyBuild/4.9.4 (determined via path to 'eb' script)...
== 2024-11-07 15:59:09,480 run.py:251 INFO Using /bin/bash as shell for running cmd: git clone --no-checkout https://github.com/astral-sh/reqwest-middleware.git
== 2024-11-07 15:59:09,480 run.py:260 INFO running cmd: git clone --no-checkout https://github.com/astral-sh/reqwest-middleware.git
== 2024-11-07 15:59:10,105 run.py:700 INFO cmd "git clone --no-checkout https://github.com/astral-sh/reqwest-middleware.git" exited with exit code 0 and output:
Cloning into 'reqwest-middleware'...

== 2024-11-07 15:59:10,105 run.py:251 INFO Using /bin/bash as shell for running cmd: git checkout 21ceec9a5fd2e8d6f71c3ea2999078fecbd13cbe
== 2024-11-07 15:59:10,105 run.py:260 INFO running cmd: git checkout 21ceec9a5fd2e8d6f71c3ea2999078fecbd13cbe
== 2024-11-07 15:59:10,134 build_log.py:171 ERROR EasyBuild crashed with an error (at easybuild/base/exceptions.py:126 in __init__): cmd "git checkout 21ceec9a5fd2e8d6f71c3ea2999078fecbd13cbe" exited with exit code 128 and output:
fatal: reference is not a tree: 21ceec9a5fd2e8d6f71c3ea2999078fecbd13cbe

Not sure what to make of that

@Micket
Copy link
Contributor

Micket commented Nov 7, 2024

Looks to me that maybe this is just the developers over at astral-sh/reqwest-middleware.git just forced pushed new stuff, abandoning the commit 21ceec9a5fd2e8d6f71c3ea2999078fecbd13cbe maybe?
Probing
astral-sh/reqwest-middleware@21ceec9
i see the warning

This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

If so, then I don't want to let that block this PR, but I want to confirm with you @Flamefire

@Micket
Copy link
Contributor

Micket commented Nov 7, 2024

Test report by @Micket

Overview of tested easyconfigs (in order)

  • SUCCESS tokenizers-0.15.2-GCCcore-12.3.0.eb
  • SUCCESS fastparquet-2023.4.0-gfbf-2022b.eb
  • SUCCESS Safetensors-0.4.3-gfbf-2023a.eb
  • SUCCESS rustworkx-0.15.1-gfbf-2023a.eb
  • SUCCESS maturin-1.4.0-GCCcore-12.2.0-Rust-1.75.0.eb
  • SUCCESS kyber-0.4.0-GCC-12.3.0.eb
  • SUCCESS cryptography-42.0.8-GCCcore-13.3.0.eb

Build succeeded for 7 out of 7 (7 easyconfigs in total)
vera-skylake-build - Linux Rocky Linux 8.9, x86_64, Intel Xeon Processor (Skylake, IBRS, no TSX), Python 3.6.8
See https://gist.github.com/Micket/ffa48aff3847feba053d5373367a2bc1 for a full test report.

@Micket
Copy link
Contributor

Micket commented Nov 7, 2024

I hadn't seen easybuilders/easybuild-framework#4680

OK, so it's not this PR's fault. I'm merging it as is, as the two failures are unrelated.

@Micket Micket merged commit 8551f99 into easybuilders:develop Nov 7, 2024
41 checks passed
@Flamefire Flamefire deleted the cargo-for-uv branch November 8, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Micket Micket Micket approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
release after 4.9.4
Development

Successfully merging this pull request may close these issues.
2 participants
@Flamefire @Micket