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

adopt module_load_environment in Bundle easyblock #3513

Merged
merged 13 commits into from
Feb 5, 2025

Conversation

lexming
Copy link
Contributor

@lexming lexming commented Nov 22, 2024

Update of Bundle easyblock for #3527

Also fixes issue #3528

@lexming lexming added the EasyBuild-5.0 EasyBuild 5.0 label Nov 22, 2024
@lexming lexming added this to the 5.0 milestone Nov 22, 2024
@lexming lexming marked this pull request as draft November 22, 2024 13:13
@lexming lexming changed the title {WIP} replace make_module_req_guess with module_load_environment {WIP} replace make_module_req_guess with module_load_environment: PythonBundle, Bundle, FlexiBLAS, PerlModule Nov 28, 2024
@lexming lexming changed the title {WIP} replace make_module_req_guess with module_load_environment: PythonBundle, Bundle, FlexiBLAS, PerlModule {WIP} replace make_module_req_guess with module_load_environment: Bundle, FlexiBLAS, PerlModule, Perl, PythonPackage Nov 28, 2024
@lexming lexming changed the title {WIP} replace make_module_req_guess with module_load_environment: Bundle, FlexiBLAS, PerlModule, Perl, PythonPackage replace make_module_req_guess with module_load_environment: Bundle, FlexiBLAS, PerlModule, Perl, PythonPackage Nov 28, 2024
@lexming lexming marked this pull request as ready for review November 28, 2024 13:38
@boegel
Copy link
Member

boegel commented Dec 4, 2024

@lexming I suspect that #3509 will get merged soon, which will cause trouble/conflicts here.

Maybe we can revert the changes to the Bundle easyblock made here for now, and postpone that to another PR to avoid a complex conflict resolution situation?

@boegel boegel changed the title replace make_module_req_guess with module_load_environment: Bundle, FlexiBLAS, PerlModule, Perl, PythonPackage replace make_module_req_guess with module_load_environment: Bundle, FlexiBLAS, PerlModule, Perl, PythonPackage Dec 4, 2024
@Thyre
Copy link
Contributor

Thyre commented Dec 4, 2024

@lexming I suspect that #3509 will get merged soon, which will cause trouble/conflicts here.

Maybe we can revert the changes to the Bundle easyblock made here for now, and postpone that to another PR to avoid a complex conflict resolution situation?

I think that this should not cause a merge conflict. #3509 doesn't touch the lines changed by this PR and comp, used by this PR, is still defined.
However, having both module_load_environment and make_module_req_guess in the same EasyBlock is probably not a good idea.

I think there are two options:

@lexming lexming changed the title replace make_module_req_guess with module_load_environment: Bundle, FlexiBLAS, PerlModule, Perl, PythonPackage replace make_module_req_guess with module_load_environment: Bundle Dec 11, 2024
@lexming
Copy link
Contributor Author

lexming commented Dec 11, 2024

Having both module_load_environment and make_module_req_guess is indeed undesirable. I propose that #3509 gets merged first and then I'll update this PR to adapt those changes to the new approach.

I changed the scope of this PR to only concern Bundle.

@lexming lexming changed the title replace make_module_req_guess with module_load_environment: Bundle adopt module_load_environment: Bundle Dec 11, 2024
@boegel
Copy link
Member

boegel commented Dec 11, 2024

@lexming I think this PR should only get merged if all other easyblocks have been updated to use module_load_environment rather than customizing make_module_req_guess, since with the changes made here customized make_module_req_guess methods for components will be ignored?

@boegel
Copy link
Member

boegel commented Jan 8, 2025

@lexming Taking this one step further: shouldn't we be taking into account here that one of bundle components is being installed with an easyblock that has not migrated to module_load_environment yet?

We'll do so for all easyblocks under our control, but we also need to take into account that people may have custom easyblocks that are still using make_module_req_guess, so we should at least consider it too still (along with a deprecation warning, perhaps)?

@boegel
Copy link
Member

boegel commented Jan 20, 2025

@lexming You should sync up your branch with current 5.0.x, since the Bundle easyblock now uses make_module_req_guess in two places (after the merge of #3509, which was synced into 5.0.x in #3542)

…ake into account that component easyblocks may still be using make_module_req_guess
complete migration of `Bundle` easyblock to `module_load_environment` + take into account that component easyblocks may still be using `make_module_req_guess`
@lexming
Copy link
Contributor Author

lexming commented Jan 24, 2025

@boegel LGTM, will send some tests...

@lexming
Copy link
Contributor Author

lexming commented Jan 24, 2025

Test report by @lexming

Overview of tested easyconfigs (in order)

  • SUCCESS buildenv-default-foss-2023a.eb
  • SUCCESS OpenSSL-3.eb
  • SUCCESS Perl-bundle-CPAN-5.36.1-GCCcore-12.3.0.eb
  • SUCCESS gfbf-2023a.eb
  • SUCCESS jiter-0.4.1-GCCcore-12.3.0.eb
  • SUCCESS X11-20230603-GCCcore-12.3.0.eb

Build succeeded for 6 out of 6 (6 easyconfigs in total)
obelia - Linux Fedora Linux 41 (KDE Plasma), x86_64, 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, Python 3.9.21
See https://gist.github.com/lexming/974c98d6234323edaa157681d887c484 for a full test report.

@lexming
Copy link
Contributor Author

lexming commented Jan 24, 2025

Checked the resulting module files and they have the same variables as before this PR.

@Thyre
Copy link
Contributor

Thyre commented Jan 24, 2025

Might be also worth checking FlexiBLAS, where we ran into #2733 before #3509

@Thyre
Copy link
Contributor

Thyre commented Jan 27, 2025

Testing one of the EasyConfigs from easybuilders/easybuild-easyconfigs#21582

Test report by @Thyre

Overview of tested easyconfigs (in order)

  • SUCCESS intel-compilers-2025.0.0-CUDA-12.8.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
Linux - Linux Arch Linux UNKNOWN, x86_64, AMD Ryzen 7 7800X3D 8-Core Processor, 1 x NVIDIA NVIDIA GeForce RTX 3070, 565.77, Python 3.13.1
See https://gist.github.com/Thyre/ea3d5e4bf30df680feabf4444567aae4 for a full test report.

@Thyre
Copy link
Contributor

Thyre commented Jan 27, 2025

Test report by @Thyre

Overview of tested easyconfigs (in order)

  • SUCCESS FlexiBLAS-3.4.4-GCC-13.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
Linux - Linux Arch Linux UNKNOWN, x86_64, AMD Ryzen 7 7800X3D 8-Core Processor, 1 x NVIDIA NVIDIA GeForce RTX 3070, 565.77, Python 3.13.1
See https://gist.github.com/Thyre/66a519ac64dc1e4dfcffef30fccfebb0 for a full test report.

@Thyre
Copy link
Contributor

Thyre commented Jan 27, 2025

Test report by @Thyre

Overview of tested easyconfigs (in order)

* **SUCCESS** _FlexiBLAS-3.4.4-GCC-13.3.0.eb_

Build succeeded for 1 out of 1 (1 easyconfigs in total) Linux - Linux Arch Linux UNKNOWN, x86_64, AMD Ryzen 7 7800X3D 8-Core Processor, 1 x NVIDIA NVIDIA GeForce RTX 3070, 565.77, Python 3.13.1 See https://gist.github.com/Thyre/66a519ac64dc1e4dfcffef30fccfebb0 for a full test report.

Module file paths looks good:

[...]
prepend_path("CMAKE_PREFIX_PATH", root)
prepend_path("CPATH", pathJoin(root, "include"))
prepend_path("CPATH", pathJoin(root, "include/flexiblas"))
prepend_path("LD_LIBRARY_PATH", pathJoin(root, "lib"))
prepend_path("LIBRARY_PATH", pathJoin(root, "lib"))
prepend_path("MANPATH", pathJoin(root, "share/man"))
prepend_path("PATH", pathJoin(root, "bin"))
prepend_path("PKG_CONFIG_PATH", pathJoin(root, "lib/pkgconfig"))
prepend_path("XDG_DATA_DIRS", pathJoin(root, "share"))
setenv("EBROOTFLEXIBLAS", root)
setenv("EBVERSIONFLEXIBLAS", "3.4.4")
setenv("EBDEVELFLEXIBLAS", pathJoin(root, "easybuild/FlexiBLAS-.3.4.4-GCC-13.3.0-easybuild-devel"))

-- Built with EasyBuild version 5.0.0beta1

@boegel boegel added the change label Feb 5, 2025
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel merged commit 320cea7 into easybuilders:5.0.x Feb 5, 2025
19 checks passed
@boegel boegel changed the title adopt module_load_environment: Bundle adopt module_load_environment in Bundle easyblock Feb 5, 2025
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.

3 participants