-
Notifications
You must be signed in to change notification settings - Fork 50
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
Print LmodError when loading GCCcore-12.2.0-based modules on zen4
#841
base: 2023.06-software.eessi.io
Are you sure you want to change the base?
Print LmodError when loading GCCcore-12.2.0-based modules on zen4
#841
Conversation
…dError when they are loaded
Instance
|
Instance
|
Instance
|
…e of the step-hooks, so we can unset it at the end
Ok, this PR is more or less ready, but we should create a known issues page on the zen4 tree missing |
Ok, I'll need EESSI/docs#357 to be merged first. Then, I'll put a link to that part of the docs in the LmodError. |
EESSI_IGNORE_ZEN4_GCC1220_ENVVAR="EESSI_IGNORE_LMOD_ERROR_ZEN4_GCC1220" | ||
|
||
def is_gcccore_1220_based(ecname, ecversion, tcname, tcversion): | ||
"""Checks if this easyconfig either _is_ or _uses_ a GCCcore-12.2.2 based toolchain""" |
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.
"""Checks if this easyconfig either _is_ or _uses_ a GCCcore-12.2.2 based toolchain""" | |
"""Checks if this easyconfig either _is_ or _uses_ a GCCcore-12.2.0 based toolchain""" |
# Need to escape newline character so that the newline character actually ends up in the module file | ||
# (otherwise, it splits the string, and a 2-line string ends up in the modulefile, resulting in syntax error) | ||
errmsg = "EasyConfigs using toolchains based on GCCcore-12.2.0 are not supported for the Zen4 architecture.\\n" | ||
errmsg += "See https://www.eessi.io/docs/known_issues/eessi-2023.06/" |
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.
errmsg += "See https://www.eessi.io/docs/known_issues/eessi-2023.06/" | |
errmsg += "See https://www.eessi.io/docs/known_issues/eessi-2023.06/#gcc-1220-and-foss-2022b-based-modules-cannot-be-loaded-on-zen4-architecture" |
# Make sure a single environment variable name is used for this throughout the hooks | ||
EESSI_IGNORE_ZEN4_GCC1220_ENVVAR="EESSI_IGNORE_LMOD_ERROR_ZEN4_GCC1220" | ||
|
||
def is_gcccore_1220_based(ecname, ecversion, tcname, tcversion): |
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.
This feels brittle, there's no sanity checking on the arguments. If you give the arguments in the wrong order, the function will happily proceed. Don't particularly want you to bend over backwards to check the arguments, I think kwargs with default None
would at least be clear and less error prone.
EESSI_FORCE_ATTR) | ||
|
||
|
||
# We do this as early as possible - and remove it all the way in the last step hook (post_testcases_hook) |
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.
This comment is no longer accurate (I think)
|
||
|
||
# We do this as early as possible - and remove it all the way in the last step hook (post_testcases_hook) | ||
def pre_prepare_hook_ignore_zen4_gcccore1220_error(self, *args, **kwargs): |
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.
Is there really any harm in collapsing this into your pre_fetch hook? It would be nice to keep the setting and unsetting unified for our future selves to understand better what would need to be changed.
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.
Or is the environment variable not persistent?
os.environ[EESSI_IGNORE_ZEN4_GCC1220_ENVVAR] = "1" | ||
|
||
|
||
def post_prepare_hook_ignore_zen4_gcccore1220_error(self, *args, **kwargs): |
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.
Same here, is there any harm to just adding this to post_module_hook?
Implements the idea from https://gitlab.com/eessi/support/-/issues/37#note_2159031831
But, not currently working, because the first module that gets installed that uses GCCcore-12.2.0 as dependency will try to load it (even with
--module-only
), which then fails:maybe we can make that pre-prepare hook do nothing. Or skip the prepare phase. NOt sure...