-
Notifications
You must be signed in to change notification settings - Fork 20
Update site package behavior #232
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
base: main
Are you sure you want to change the base?
Changes from all commits
0fcaacb
a3229cf
a226976
e1a6d5d
8151942
7b44e69
dc32c97
22c9c9b
3af956c
2ed0a52
0e95025
046bfde
f0dafeb
9849e35
7b328cc
96f0d1d
8e78e0c
b34dbd7
d17e970
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,7 +134,16 @@ local eessi_modules_subdir = pathJoin("modules", "all") | |
| -- eessi_module_path is the location of the _CPU_ module files, e.g., | ||
| -- /cvmfs/software.eessi.io/versions/<EESSI_VERSION>/software/linux/x86_64/amd/zen3/modules/all | ||
| local eessi_module_path = pathJoin(eessi_software_path, eessi_modules_subdir) | ||
| local eessi_site_software_path = string.gsub(eessi_software_path, "versions", "host_injections") | ||
| local eessi_site_software_path | ||
| -- If EESSI_SITE_SOFTWARE_PREFIX is defined, replace /cvmfs/software.eessi.io (or more generally EESSI_CVMFS_REPO) | ||
| -- by that prefix. This ensures that the directory still contains the os/vendor/arch/micro-arch/accelerator etc | ||
| -- If it is not defined, default to a site installation prefix under host_injections | ||
| site_prefix = os.getenv("EESSI_SITE_SOFTWARE_PREFIX") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love this approach, there is value in having the path in a fully defined location.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about it more, and I am coming around. I was a bit stuck on having the fixed path because we require a fixed path to be able to do MPI injection. However, these are actually separate issues and this change doesn't affect that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will require some documentation updates, and some additional checks in Issues for all these are enough for now, but they should be addressed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Caspar and I discussed this quite a bit yesterday. In some way I also still prefer to have everything in a single and fixed location, but I also see Caspar's point, and being able to do the software installations elsewhere does make sense to me. I still have some concerns, e.g. about where to put the Lmod site hooks: Caspar wanted to keep them under host_injections, as they (may) also affect EESSI modules, and though that makes sense, it does mean that you may have two locations with this The current situation, with just a single host_injection directory, also has some drawbacks, e.g. in case you want to do site installations on a local CVMFS repo, while having MPI/GPU libraries on a local disk. You can do it by doing some symlink trickery, but that's not ideal either. Anyway, since the default behavior won't change, I was okay with adding the possibility for overriding the software installation prefix.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to hear you've come around - I do feel that at least having the option as a site to configure it is something we should offer. Whether a site uses it, or prefers redirecting the host-injections symlink (or making e.g. I'd personally like to use the environment variable. And I'd probably set it in an internal lmod hook, so that it's set at the moment users load the EESSI module - that way users can't unset it either. Companion PR for EESSI-extend is here #235 I'll make issues for the rest. I think we should do the documentation updates once we've actually taken the whole new functionality for practical test - I'm hoping the idea is more clear after the webinar :)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
And yes, if this works well, I think it's nice to make dev.eessi.io use this, as it is essentially also 'building on top' of EESSI.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docs issue: EESSI/docs#768
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minimal docs update here EESSI/docs#769 (we should still have more extensive docs on how to build on top of EESSI as a site, but that's what EESSI/docs#768 is for). |
||
| if site_prefix then | ||
| eessi_site_software_path = string.gsub(eessi_software_path, eessi_repo, site_prefix) | ||
| else | ||
| eessi_site_software_path = string.gsub(eessi_software_path, "versions", "host_injections") | ||
| end | ||
| -- Site module path is the same as the EESSI one, but with `versions` changed to `host_injections`, e.g., | ||
| -- /cvmfs/software.eessi.io/host_injections/<EESSI_VERSION>/software/linux/x86_64/amd/zen3/modules/all | ||
| local eessi_site_module_path = pathJoin(eessi_site_software_path, eessi_modules_subdir) | ||
|
|
@@ -169,8 +178,14 @@ if ( mode() ~= "spider" ) then | |
| prepend_path("MODULEPATH", eessi_module_path) | ||
| eessiDebug("Adding " .. eessi_module_path .. " to MODULEPATH") | ||
| end | ||
|
|
||
| -- Make sure the EESSI cache is found, this is specified in the lmodrc.lua in the eessi_software_path | ||
| prepend_path("LMOD_RC", pathJoin(eessi_software_path, ".lmod", "lmodrc.lua")) | ||
| eessiDebug("Adding " .. pathJoin(eessi_software_path, ".lmod", "lmodrc.lua") .. " to LMOD_RC") | ||
| -- Make sure that a cache for site installations can also be found | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How to generate the cache also needs an update, but I need to check that works first. We need to let Lmod know that the module path requires a gateway module, then the hierarchy can be represented in the cache. It's a separate issue, this just reminds me that I need to look into it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we currently even support caches for local installation? Caspar and I looked into that a bit yesterday, but we couldn't find anything. We plan to cover this in the tutorial, but we should indeed also add documentation for this. With Caspar's proposed change, sites can easily call the |
||
| prepend_path("LMOD_RC", pathJoin(eessi_site_software_path , ".lmod", "lmodrc.lua")) | ||
| eessiDebug("Adding " .. pathJoin(eessi_site_software_path , ".lmod", "lmodrc.lua") .. " to LMOD_RC") | ||
|
|
||
| -- Use pushenv for LMOD_PACKAGE_PATH as this may be set locally by the site | ||
| pushenv("LMOD_PACKAGE_PATH", pathJoin(eessi_software_path, ".lmod")) | ||
| eessiDebug("Setting LMOD_PACKAGE_PATH to " .. pathJoin(eessi_software_path, ".lmod")) | ||
|
|
@@ -210,11 +225,23 @@ end | |
| -- prepend the site module path last so it has priority | ||
| prepend_path("MODULEPATH", eessi_site_module_path) | ||
| eessiDebug("Adding " .. eessi_site_module_path .. " to MODULEPATH") | ||
| if isDir(eessi_module_path_accel) then | ||
| eessi_module_path_site_accel = string.gsub(eessi_module_path_accel, "versions", "host_injections") | ||
| setenv("EESSI_SITE_MODULEPATH_ACCEL", eessi_module_path_site_accel) | ||
| prepend_path("MODULEPATH", eessi_module_path_site_accel) | ||
| eessiDebug("Using site accelerator modules at: " .. eessi_module_path_site_accel) | ||
|
|
||
| -- If EESSI_SITE_SOFTWARE_PREFIX is defined, replace /cvmfs/software.eessi.io (or more generally EESSI_CVMFS_REPO) | ||
| -- by that prefix to get the site accelerator path. This ensures that the directory still contains the | ||
| -- os/vendor/arch/micro-arch/accelerator etc. If it is not defined, default to a site installation prefix under | ||
| -- host_injections | ||
| -- Note that we need the eessi_module_path_accel to construct either of these site installation accelerator paths | ||
| if eessi_module_path_accel then | ||
| if site_prefix then | ||
| eessi_module_path_site_accel = string.gsub(eessi_module_path_accel, eessi_repo, site_prefix) | ||
| else | ||
| eessi_module_path_site_accel = string.gsub(eessi_module_path_accel, "versions", "host_injections") | ||
| end | ||
| if isDir(eessi_module_path_site_accel) then | ||
| setenv("EESSI_SITE_MODULEPATH_ACCEL", eessi_module_path_site_accel) | ||
| prepend_path("MODULEPATH", eessi_module_path_site_accel) | ||
| eessiDebug("Using site accelerator modules at: " .. eessi_module_path_site_accel) | ||
| end | ||
| end | ||
|
|
||
| -- allow sites to add a family directive to the EESSI module, | ||
|
|
||
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.
Change this in sync with the change in
2023.06.lua, which now conditionally adds the site accerator path if that accelerator path exists - ie this check: