-
Notifications
You must be signed in to change notification settings - Fork 10
Add settings for EasyBuild 5+ to EESSI-extend #45
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
Changes from all commits
c7d8371
988975c
bfe4704
7625ff4
fd7ed92
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 |
---|---|---|
|
@@ -98,21 +98,26 @@ if (eessi_accelerator_target ~= nil) then | |
end | ||
end | ||
|
||
-- Some environment variables affect behaviour, let's gather them once | ||
local eessi_cvmfs_install = os.getenv("EESSI_CVMFS_INSTALL") ~= nil | ||
local eessi_site_install = os.getenv("EESSI_SITE_INSTALL") ~= nil | ||
local eessi_project_install = os.getenv("EESSI_PROJECT_INSTALL") ~= nil | ||
local eessi_user_install = os.getenv("EESSI_USER_INSTALL") ~= nil | ||
|
||
-- Use an installation prefix that we _should_ have write access to | ||
if (os.getenv("EESSI_CVMFS_INSTALL") ~= nil) then | ||
if eessi_cvmfs_install then | ||
-- Make sure no other EESSI install environment variables are set | ||
if ((os.getenv("EESSI_SITE_INSTALL") ~= nil) or (os.getenv("EESSI_PROJECT_INSTALL") ~= nil) or (os.getenv("EESSI_USER_INSTALL") ~= nil)) then | ||
if (eessi_site_install or eessi_project_install or eessi_user_install) then | ||
LmodError("You cannot use EESSI_CVMFS_INSTALL in combination with any other EESSI_*_INSTALL environment variables") | ||
end | ||
eessi_cvmfs_install = true | ||
easybuild_installpath = os.getenv("EESSI_SOFTWARE_PATH") | ||
-- enforce accelerator subdirectory usage for CVMFS installs (only if an accelerator install is requested) | ||
if (eessi_accelerator_target ~= nil) and (cuda_compute_capability ~= nil) and (os.getenv("EESSI_ACCELERATOR_INSTALL") ~= nil) then | ||
easybuild_installpath = pathJoin(easybuild_installpath, eessi_accelerator_target) | ||
end | ||
elseif (os.getenv("EESSI_SITE_INSTALL") ~= nil) then | ||
elseif eessi_site_install then | ||
-- Make sure no other EESSI install environment variables are set | ||
if ((os.getenv("EESSI_PROJECT_INSTALL") ~= nil) or (os.getenv("EESSI_USER_INSTALL") ~= nil)) then | ||
if (eessi_project_install or eessi_user_install) then | ||
LmodError("You cannot use EESSI_SITE_INSTALL in combination with any other EESSI_*_INSTALL environment variables") | ||
end | ||
easybuild_installpath = os.getenv("EESSI_SITE_SOFTWARE_PATH") | ||
|
@@ -122,35 +127,35 @@ elseif (os.getenv("EESSI_SITE_INSTALL") ~= nil) then | |
end | ||
else | ||
-- Deal with user and project installs | ||
project_install = os.getenv("EESSI_PROJECT_INSTALL") | ||
project_install_dir = os.getenv("EESSI_PROJECT_INSTALL") | ||
project_modulepath = nil | ||
if (project_install ~= nil) then | ||
if eessi_project_install then | ||
-- Check the folder exists | ||
if not isDir(project_install) then | ||
LmodError("The location of EESSI_PROJECT_INSTALL (" .. project_install .. ") does not exist or is not a folder") | ||
if not isDir(project_install_dir) then | ||
LmodError("The location of EESSI_PROJECT_INSTALL (" .. project_install_dir .. ") does not exist or is not a folder") | ||
end | ||
if (mode() == "load") then | ||
LmodMessage("Configuring for use of EESSI_PROJECT_INSTALL under " .. project_install) | ||
LmodMessage("Configuring for use of EESSI_PROJECT_INSTALL under " .. project_install_dir) | ||
end | ||
easybuild_installpath = string.gsub(os.getenv("EESSI_SOFTWARE_PATH"), os.getenv("EESSI_CVMFS_REPO"), project_install) | ||
easybuild_installpath = string.gsub(os.getenv("EESSI_SOFTWARE_PATH"), os.getenv("EESSI_CVMFS_REPO"), project_install_dir) | ||
project_modulepath = pathJoin(easybuild_installpath, 'modules', 'all') | ||
end | ||
user_install = os.getenv("EESSI_USER_INSTALL") | ||
user_install_dir = os.getenv("EESSI_USER_INSTALL") | ||
user_modulepath = nil | ||
if (user_install ~= nil) then | ||
if eessi_user_install then | ||
-- Check the folder exists | ||
if not isDir(user_install) then | ||
LmodError("The location of EESSI_USER_INSTALL (" .. user_install .. ") does not exist or is not a folder") | ||
if not isDir(user_install_dir) then | ||
LmodError("The location of EESSI_USER_INSTALL (" .. user_install_dir .. ") does not exist or is not a folder") | ||
end | ||
elseif (user_install == nil) and (project_install == nil) then | ||
elseif (user_install_dir == nil) and (project_install_dir == nil) then | ||
-- No need to check for existence when we use a HOME subdir | ||
user_install = pathJoin(os.getenv("HOME"), "eessi") | ||
user_install_dir = pathJoin(os.getenv("HOME"), "eessi") | ||
end | ||
if (user_install ~= nil) then | ||
if (user_install_dir ~= nil) then | ||
if (mode() == "load") then | ||
LmodMessage("Configuring for use of EESSI_USER_INSTALL under " .. user_install) | ||
LmodMessage("Configuring for use of EESSI_USER_INSTALL under " .. user_install_dir) | ||
end | ||
easybuild_installpath = string.gsub(os.getenv("EESSI_SOFTWARE_PATH"), os.getenv("EESSI_CVMFS_REPO"), user_install) | ||
easybuild_installpath = string.gsub(os.getenv("EESSI_SOFTWARE_PATH"), os.getenv("EESSI_CVMFS_REPO"), user_install_dir) | ||
user_modulepath = pathJoin(easybuild_installpath, 'modules', 'all') | ||
end | ||
end | ||
|
@@ -196,10 +201,29 @@ elseif (project_modulepath ~= nil) then | |
-- configure MODULEPATH | ||
prepend_path("MODULEPATH", project_modulepath) | ||
end | ||
|
||
-- Make sure EasyBuild itself is loaded | ||
-- need to also handle the unload behaviour where the version is defined only before we unload | ||
easybuild_version = os.getenv("EBVERSIONEASYBUILD") | ||
if not ( isloaded("EasyBuild") ) then | ||
load(latest("EasyBuild")) | ||
end | ||
easybuild_version = os.getenv("EBVERSIONEASYBUILD") or easybuild_version | ||
eessi_version = os.getenv("EESSI_VERSION") or "2023.06" | ||
|
||
-- Set environment variables that are EasyBuild version specific | ||
if convertToCanonical(easybuild_version) > convertToCanonical("4") then | ||
setenv ("EASYBUILD_STRICT_RPATH_SANITY_CHECK", "1") | ||
setenv ("EASYBUILD_CUDA_SANITY_CHECK_ERROR_ON_FAILED_CHECKS", "1") | ||
setenv ("EASYBUILD_FAIL_ON_MOD_FILES_GCCCORE", "1") | ||
setenv ("EASYBUILD_LOCAL_VAR_NAMING_CHECK", "error") | ||
-- Set environment variables that are EESSI version specific | ||
if convertToCanonical(eessi_version) > convertToCanonical("2023.06") then | ||
setenv ("EASYBUILD_PREFER_PYTHON_SEARCH_PATH", "EBPYTHONPREFIXES") | ||
setenv ("EASYBUILD_MODULE_SEARCH_PATH_HEADERS", "include_paths") | ||
setenv ("EASYBUILD_SEARCH_PATH_CPP_HEADERS", "include_paths") | ||
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. Somewhat similar, why don't we just stick to the default (using 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 think this one is a motivated change IIRC, something related to the DPC++ compiler...will need to dig it out 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. The issue and resolution is in easybuilders/easybuild-framework#3331 (comment) , but that doesn't really help me to be any clearer on what we should do. @Flamefire You were involved in that discussion, what do you think? Is it enough to only change 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 have seen issues fixed with the new I'd say this one is less problematic. If stuff really does break we can unset/change 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. Same here, not opposing it, just making sure we understand the potential implications (which includes confusing users by using a not-so-common approach to specifying the paths to where header files can be found). It's also worth being aware that In this case, we can actually divert from it relatively easy where needed I think... 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. 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.
The variables used in module files are changed with 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. Ah, so @Flamefire 's earlier comment that this is more easily revertable
applies to 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. Yes, like 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. Same here, worst case we can actually back out later by patching or re-generating the module files, but it likely won't come to that. To let's move forward by configuring EasyBuild so it doesn't fiddle with |
||
end | ||
end | ||
""" | ||
|
||
moduleclass = 'devel' |
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.
I don't have strong feelings about this, and I do see the appeal to use
$EBPYTHONPREFIXES
rather than$PYTHONPATH
(works better when using Python virtual environments), but this is in some sense also uncharted territory, so we're bound to run into surprises/problems because we're using this.Also, not relying on the standard mechanism (
$PYTHONPATH
) is no doubt going to confuse end users at some point.So, we should clearly motivate why we're going down this road, since we can't change our mind in the middle, this is an all-or-nothing choice...
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.
The motivation is exactly that, better
venv
behaviour, but I agree with the main concern about how hard it is to roll back. I was actually hoping you would be the one to sooth me.End users can still use PYTHONPATH, but we wouldn't. Or are you more concerned about those using
EESSI-extend
?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.
I'm all for
EBPYTHONPREFIXES
. We have yet to find issues and it allows using venvs which is essential at least to our users.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.
I'm mostly concerned about people being confused as to how Python packages can be imported from non-standard locations while we're not using
$PYTHONPATH
.The
$EBPYTHONPREFIXES
approach seems robust, but it's very custom.That said, I'm not opposing it, just making sure we're aware that we're (yet again) taking a non-standard route here, and there may be dragons along that route...
We should probably add some documentation on the different ways in which we divert from default EasyBuild configuration, and explain the motivation behind those settings => EESSI/docs#519
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.
EBPYTHONPREFIXES
isn't the default? I thought we would change that for EB 5There 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.
It's not, we kept
$PYTHONPATH
as default search path (pretty much for the same concerns as I raised here):That option is there so you can opt-in to using it though
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.
After discussing this: let's go through with
EBPYTHONPREFIXES
.Although painful, we can actually change our mind later (by patching the module files), but hopefully it won't come to that.