Skip to content

Fix fake_module dependencies for ExtensionsEasyblock with --sanity-check-only#5182

Open
Crivella wants to merge 2 commits into
easybuilders:developfrom
Crivella:fix-exts_sanityonly_deps
Open

Fix fake_module dependencies for ExtensionsEasyblock with --sanity-check-only#5182
Crivella wants to merge 2 commits into
easybuilders:developfrom
Crivella:fix-exts_sanityonly_deps

Conversation

@Crivella

Copy link
Copy Markdown
Contributor

Currently the sanity_check_step of ExtensionEasyBlock uses

with self.fake_module_environment(extra_modules=extra_modules):

to run the sanity check on the main bundle before doing the sanity checks for each individual extension

At this point when running with --sanity-check-only the prepare_step is skipped and hence the call to

self.toolchain.prepare(onlymod=self.cfg['onlytcmod'], deps=self.cfg.dependencies(),

which causes self.toolcahain.dependencies to be an empty list.

This causes problems when the sanity check commands of the main bundle requires one of the dependencies to be loaded, see for example easybuilders/easybuild-easyblocks#4122 (comment)

As far as i can tell toolchain.prepare_step is always called passing Easyconfig.dependencies() (in general self.cfg) and this only does some checks on the deps passed without modifying them.

I think self.cfg.dependencies() should be used here directly unless there was ever a reason to not load the Bundle dependencies when running the main sanity_check_step of a Bundle with extensions

Tested this w.r.t. easybuilders/easybuild-easyblocks#4122 (comment) and it fixes the problem

@Crivella

Copy link
Copy Markdown
Contributor Author

The only error I am seeing in the CI is due to test_extensions_templates not calling eb.prepare_step() which would cause the same error to pop up.
Not calling eb.prepare_step() i guess is fine for just testing templates, but is not realistic in an actual run where extensions_step is called and we should cover this case.

We should probably fix this test by also creating a fake module for Python if we want to test for the python templates similar to what is done in

https://github.com/easybuilders/easybuild-easyblocks/blob/17d584071c73bbb51ce0ff9f58d17f201027e074/test/easyblocks/module.py#L454-L455

@Flamefire

Copy link
Copy Markdown
Contributor

IMO the actual error there is using the fake module at all: It should use the "sanity_check_module", which for module-only is the real module.

As I can't find the PR I though I made with that change I split up some changes to sanity checks I had and #5185 implements the fix.

However this might be relevant for --module-only. Did you see the same issue there too?

@Crivella

Copy link
Copy Markdown
Contributor Author

--module-only does go through the prepare step so it does work without this.

Still think that we should either use self.cfg.dependencies() or make sure that self.toolchain.dependencies was actually set

@Flamefire

Copy link
Copy Markdown
Contributor

When --sanity-check-only does not use this anymore and in all other uses self.toolchain.dependencies is set, it should be fine, shouldn't it?

I'm not sure we actually ever need self.toolchain.dependencies. From my understanding this is simply self.ec.dependencies() but hidden inside a function that cannot return anything else unless _check_dependencies was overwritten to have a different behavior

@Crivella

Crivella commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

In theory yes but to avoid confusion we should either have a check on make_module_dep that it runs only if something that set self.toolchain.dependencies has ran before hand, or use this fix.

This PR also does fix the problem of fake_module not working in --sanity-check-only.
Using the real module is probably a different discussion, but one worth having. Will try to look into #5185 as well

@Flamefire

Copy link
Copy Markdown
Contributor

In theory yes but to avoid confusion we should either have a check on make_module_dep that it runs only if something that set self.toolchain.dependencies has ran before hand, or use this fix.

I think we cannot check that: It is an empty list initially which is a valid value for a SYSTEM EC. We could potentially init it with None instead and watch for potential fallout.

This PR also does fix the problem of fake_module not working in --sanity-check-only. Using the real module is probably a different discussion, but one worth having. Will try to look into #5185 as well

With --sanity-check-only we only want to test the real module, so we don't need to create a fake module. So we don't actually need to create a fake module and it failing would be a good thing. See

# skip loading of fake module when using --sanity-check-only, load real module instead

So that one place in ExtensionEasyBlock where it is used is clearly a bug.

IIUC the failure though is not clear (enough) so the check mentioned would still be good, but with a clear comment that this is (likely) an internal usage error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants