easybuild-framework icon indicating copy to clipboard operation
easybuild-framework copied to clipboard

drop load storm safe guard for Environment Modules v4.2.4+

Open xdelaruelle opened this issue 1 year ago • 5 comments

Environment Modules v4+ safely handles automatic module load by not reloading already loaded module. Same goes when unloading module: already unloaded dependency will not be evaluated again.

As a result, no safe guard test is required and it should even be avoided to get the module dependency correctly tracked.

If module dependency declaration is skipped, no relation binds the loaded modules. Thus unloading the dependency will not lead to the auto unload of the dependent module. Without "is-loaded" safe guard, dependency is always declared and loaded environment is kept consistent by auto_handling mechanism [1]

A new ModuleTool property named "supports_safe_auto_load" is introduced. When enabled, module load safe guard code is not generated like if depends_on is enabled.

"supports_safe_auto_load" is enabled for EnviromentModules 4.2.4+. Prior v4 versions were not handling safely module load of module parent name if already loaded module were not corresponding to the default version.

[1] https://modules.readthedocs.io/en/latest/module.html#envvar-MODULES_AUTO_HANDLING

xdelaruelle avatar Nov 02 '23 06:11 xdelaruelle

@xdelaruelle Thanks a lot for the PR!

Correct me if I'm wrong, but this will effectively also change the behavior of module files that were created by EasyBuild with Modules v4.2.4+, since unloading a module file will now also trigger an unload of the modules that got loaded as dependency.

That may surprise people, so I wonder if we should make this opt-in, rather than auto-enabling this based on the version of Modules...

boegel avatar Nov 08 '23 07:11 boegel

@xdelaruelle Thanks a lot for the PR!

Correct me if I'm wrong, but this will effectively also change the behavior of module files that were created by EasyBuild with Modules v4.2.4+, since unloading a module file will now also trigger an unload of the modules that got loaded as dependency.

That may surprise people, so I wonder if we should make this opt-in, rather than auto-enabling this based on the version of Modules...

@boegel It will auto-unload dependency modules only if they are not needed by other loaded modules and if these dependency modules have been auto-loaded. So there is a change, but it should not disturb users as they have not explicitly asked for the dependency.

I would prefer to enable this by default rather making it an option to enable as with current load storm safe guard code users may end up with inconsistent environment:

With load storm safe guard (you can unload M4 and keep libtool, a dependent module, loaded):

$ ml M4 libtool
$ ml -M4
$ ml
Currently Loaded Modulefiles:
 1) libtool/2.4.7

Without load storm safe guard (dependent module is unloaded):

$ ml M4 libtool
$ ml -M4
Unloading M4/1.4.18
  Unloading dependent: libtool/2.4.7

xdelaruelle avatar Nov 08 '23 21:11 xdelaruelle

@xdelaruelle There's a window of opportunity for "breaking" changes since we're working towards EasyBuild v5.0, so I propose we apply this change then (either in the 5.0.x branch where we're currently preparing EasyBuild v5.0, or in develop once it's clear that there won't be any further EasyBuild 4.x releases after EasyBuild v4.9.0).

boegel avatar Dec 26 '23 15:12 boegel

@boegel I have just changed this pull request's target to 5.0.x and rebased it to this branch.

xdelaruelle avatar Jan 03 '24 05:01 xdelaruelle

@xdelaruelle: Tests failed in GitHub Actions, see https://github.com/easybuilders/easybuild-framework/actions/runs/8019810542 Output from first failing test suite run:

ERROR: test_toy_exts_parallel (test.framework.toy_build.ToyBuildTest)
Test parallel installation of extensions (--parallel-extensions-install)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/test/framework/toy_build.py", line 1920, in test_toy_exts_parallel
    stdout, stderr = self.run_test_toy_build_with_output(ec_file=test_ec, extra_args=args, raise_error=True)
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/test/framework/toy_build.py", line 239, in run_test_toy_build_with_output
    self._test_toy_build(*args, **kwargs)
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/test/framework/toy_build.py", line 195, in _test_toy_build
    raise myerr
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/test/framework/toy_build.py", line 190, in _test_toy_build
    outtxt = self.eb_main(args, logfile=self.dummylogfn, do_build=True, verbose=verbose,
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/test/framework/utilities.py", line 341, in eb_main
    raise myerr
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/test/framework/utilities.py", line 314, in eb_main
    main(args=main_args, logfile=logfile, do_build=do_build, testing=testing, modtool=modtool)
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/easybuild/main.py", line 727, in main
    do_cleanup = process_eb_args(orig_paths, eb_go, cfg_settings, modtool, testing, init_session_state,
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/easybuild/main.py", line 564, in process_eb_args
    ecs_with_res = build_and_install_software(ordered_ecs, init_session_state,
  File "/tmp/runner/1060f7c67d4a8f8a7ba53aa6f1a0ba2c2b445aa0/lib/python3.8/site-packages/easybuild/main.py", line 175, in build_and_install_software
    raise EasyBuildError(test_msg)
easybuild.tools.build_log.EasyBuildError: 'Installation of test.eb failed: "shell command \'gcc ...\' failed in extensions step for test.eb"'

----------------------------------------------------------------------
Ran 871 tests in 641.234s

FAILED (errors=1)
ERROR: Not all tests were successful.

bleep, bloop, I'm just a bot (boegelbot v20200716.01) Please talk to my owner @boegel if you notice me acting stupid), or submit a pull request to https://github.com/boegel/boegelbot fix the problem.

boegelbot avatar Feb 23 '24 14:02 boegelbot