Skip to content

Update fake module for each extension installed #4868

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

Merged
merged 16 commits into from
May 18, 2025

Conversation

lexming
Copy link
Contributor

@lexming lexming commented May 5, 2025

The extension step generates a single fake module before the first extension is installed and uses that same fake module across the installation of all extensions.

This break the installations of easyconfigs with extensions that rely on files installed by previous extensions in the same easyconfig and that make use of search paths to find those files. Since the resolution of paths in module_load_environment only happens before extensions are installed, no search paths are added to the fake module as those paths/globs are empty.

We added a quick workaround in #4809 that skips path resolution for fake modules to avoid this issue. However, this workaround has broken installation of easyconfigs that have search paths with globs that must be expanded in their sanity checks. One example is MATSim-15.0-Java-17.eb, which has modextrapaths = {'CLASSPATH': 'libs/*'}.

== sanity checking...
  >> file 'matsim-15.0.jar' found: OK
  >> (non-empty) directory 'libs' found: OK
  >> (non-empty) directory 'examples' found: OK
  >> loading modules: MATSim/15.0-Java-17...
  >> running command 'java org.matsim.run.ReleaseInfo' ...
  >> result for command 'java org.matsim.run.ReleaseInfo': FAILED
== ... (took 3 secs)
== FAILED: Installation ended unsuccessfully: Sanity check failed: sanity check command java org.matsim.run.ReleaseInfo failed with exit code 1 (output: Error: Could not find or load main class org.matsim.run.ReleaseInfo
Caused by: java.lang.ClassNotFoundException: org.matsim.run.ReleaseInfo
) (took 9 secs)

This PR fixes this issue for good by generating a new fake module for each extension installed. This ensures that path/glob resolution can happen and new files from previous extensions will be accounted.

Moreover, I'm adding a new context manager to handle fake modules and a new unit test to test this particular case.

@lexming lexming added the bug fix label May 5, 2025
@lexming lexming added this to the 5.0.1 milestone May 5, 2025
@lexming lexming force-pushed the fake-module-per-extension branch from c055fbd to 6697207 Compare May 6, 2025 11:32
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants