Skip to content

Remove redundant work in sanity check of extensions#5185

Open
Flamefire wants to merge 9 commits into
easybuilders:developfrom
Flamefire:sanity_check_module_environment
Open

Remove redundant work in sanity check of extensions#5185
Flamefire wants to merge 9 commits into
easybuilders:developfrom
Flamefire:sanity_check_module_environment

Conversation

@Flamefire

@Flamefire Flamefire commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

I identified a few things we do in the sanity check of extensions we should not:

  • check for things on the whole install tree: folders, rpaths, libraries, CUDA CCs
  • Create a module in --sanity-check-only mode

I'm fixing the first one in #5184

The main one here is the 2nd one with this simple diff in extensioneasyblock:

-               with self.fake_module_environment(extra_modules=extra_modules):
+               with self.sanity_check_module_environment(extra_modules=extra_modules):

Not only does this avoid the complex, time-consuming logic of creating a module and manipulating MODULEPATH it might fail if module creation requires information from the build process.
See e.g. easybuilders/easybuild-easyblocks#4122 where get_software_version was used, i.e. the dependency module was required to be loaded.
I added a sanity_check_module_environment context manager to load & unload the sanity check module (real module or fake module)

I'm also starting to remove the extension parameter to those methods, which I did in a PR prior to the 5.3.0 release but took out to get that PR merged without this change.
Reason is to avoid potential mismatches, e.g. by forgetting to pass the parameter.
Ultimately I want to use self.is_extension to allow multiple extension to fail their sanity check step and having a "parallel" source of truth makes that harder.
The cause here is that the failing exts_filter allows to check other extensions but failing commands does not as that aborts directly. Easy to fix when relying on self.is_extension

If there are no objections I'll also remove it from sanity_check_load_module which is similarly simple but requires an update to the easyblocks that use it. Just want to get feedback before doing that extra work.

@Flamefire Flamefire force-pushed the sanity_check_module_environment branch from 373c48b to 9b60663 Compare June 12, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant