Skip to content

Conversation

@wpoely86
Copy link
Member

Lmod 8.2 supports now extensions, this PR makes EB generate the needed lines in the module files for it.

wpoely86 added 14 commits June 21, 2019 14:36
* develop: (413 commits)
  bump version to 4.0.2dev
  tweak release notes and bump version for v4.0.1 release
  prepare release notes for eb401
  extend test_build_easyconfigs_in_parallel_gc3pie to also check for raised error when a job failed
  improve error message for jobs that failed when using GC3Pie job backend
  appease the Hound
  fix join for Tcl module syntax in test_toy_ghost_installdir
  make removal of ghost installation directories opt-in
  remove installation directory used by module file that is replaced under --force, to avoid leaving behind a 'ghost' installation
  add ModuleGenerator.det_installdir method to determine installation directory used by a given module file
  use remove_dir + move_file functions in EasyBlock.make_dir
  Fix wrong function invocation in last commit.
  Report what build jobs actually failed.
  fix broken test for get_paths_for
  appease the Hound
  only consider resolved path to 'eb' script if desired subdir is not found relative to 'eb' script location in get_paths_for
  enhance test for get_paths_for
  define $EB_SCRIPT_PATH in 'eb' wrapper script, and consider it before location of 'eb' determined via $PATH in get_paths_for function
  Error out if some GC3Pie job failed.
  better filtering + logging in test for move_logs
  ...
* origin/develop: (74 commits)
  always include mandatory easyconfig parameters in dumped easyconfig file, regardless of whether default value is used or not
  add support for is_mandatory_key method in EasyConfig class
  silence deprecation warning for Python 2.6 when testing with Python 2.6
  also test with Lmod 8.2.3 in GitHub CI
  ensure 'silence_deprecation_warnings' build option is correctly set in tests
  correctly silence deprecation warning for Lmod 6.x in Travis CI config
  add support for --silence-deprecation-warnings, and use it to avoid deprecation warnings breaking tests when testing with Lmod 6.x
  update help message for --external-modules-metadata
  fix trivial code style issue
  fix module version cache check
  keep testing single configuration in Travis CI with Lmod 6.5.1
  don't test non-Lmod module tools with Python 3.8
  also test with Python 3.8 in Travis CI
  take into account that platform.dist was removed in Python 3.8 in get_os_version, leverage distro package if available
  deprecate running EasyBuild with Python 2.6 via new check_python_version() function
  stop testing with Lmod 6.x in GitHub Actions
  stop testing with Lmod 6.x in Travis CI, update to Lmod 8.2.3, favor testing with Python 2.7 over 2.6 (+ cleanup)
  fix broken test_lmod_specific, use DEPR_VERSION rather than REQ_VERSION
  take into account that platform.linux_distribution was removed in Python 3.8 in get_os_name, leverage distro package if available
  deprecate support for using Lmod 6.x
  ...
@wpoely86 wpoely86 requested a review from boegel November 15, 2019 09:23
provide_list = self._generate_provides_list()
if self.modules_tool.supports_extensions and provide_list:
provide_list_str = ', '.join(['"%s"' % x for x in provide_list])
lines.extend(['', 'extensions(%s)' % provide_list_str])
Copy link
Member

@boegel boegel Nov 15, 2019

Choose a reason for hiding this comment

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

We have two options here:

  • either we include this if the Lmod version used at installation time is recent enough (which is what you're doing here)
  • or we add a conditional in the module file itself, using LmodVersion() (that's what @rtmclay recommended)

The benefit of the latter approach is that the module files will still work when loaded with an older Lmod version, and there's basically no overhead in using the conditional.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a design choice, I don't have a strong preference for either.

But adding support in a module file for something that doesn't work with the current Lmod seems a bit weird to me? Especially as it's pretty easy to regenerate a module.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, it might surprise people. You update Lmod and suddenly the output of av etc changes

Copy link
Member

Choose a reason for hiding this comment

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

It's a protection mechanism too though. You may be installing software on a system with a slightly more recent version of Lmod (8.2+), and then consume the modules elsewhere with a slightly older Lmod (<8.2).
That wouldn't be very nice either... It's certainly less serious than being a bit careful and only using extensions conditionally.

Output of ml av will change anyway as soon as you install something with extensions after updating to Lmod 8.2, and you're probably not going to notice straight away (I certainly rarely check the generated module files mannually ;) ).

@boegel boegel added this to the next release (4.1.0) milestone Nov 16, 2019
@easybuilders easybuilders deleted a comment from boegelbot Nov 16, 2019
@boegel
Copy link
Member

boegel commented Nov 17, 2019

@wpoely86 Tests are still broken when using Lmod 8.2+ (@boegelbot only reports broken tests from Travis CI).

======================================================================
FAIL: test_module_extensions (test.framework.module_generator.LuaModuleGeneratorTest)
test the extensions() for extensions
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/framework/module_generator.py", line 585, in test_module_extensions
    self.assertTrue(re.search(pattern, desc, re.M), "Pattern '%s' found in: %s" % (pattern, desc))
AssertionError: None is not true : Pattern '^extensions\("bar/0.0", "barbar/0.0", "l/s", "toy/0.0"\)' found in: help([==[

Description
===========
Toy C program, 100% toy.

More information
================
 - Homepage: https://easybuilders.github.io/easybuild


Included extensions
===================
bar-0.0, barbar-0.0, l-s, toy-0.0
]==])

whatis([==[Description: Toy C program, 100% toy.]==])
whatis([==[Homepage: https://easybuilders.github.io/easybuild]==])
whatis([==[URL: https://easybuilders.github.io/easybuild]==])
whatis([==[Extensions: bar-0.0, barbar-0.0, l-s, toy-0.0]==])

local root = "/tmp/eb-CRdgXt/eb-Qajkhe/eb-FkUxoO/tmp9r3vc4/software/toy/0.0-gompi-2018a-test"

conflict("toy")

if convertToCanonical(LmodVersion()) > convertToCanonical("8.2.0") then
    extensions("bar/0.0", "barbar/0.0", "l/s", "toy/0.0")
end

 ----------------------------------------------------------------------
Ran 653 tests in 874.006s

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

We need a Lmod version check and it's simple a complete pain in Tcl.
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.

@wpoely86 Some minor suggestions, it's pretty much good to go imho...

@easybuilders easybuilders deleted a comment from boegelbot Nov 29, 2019
wpoely86 and others added 2 commits November 29, 2019 17:39
…o modprovides

* 'modprovides' of github:wpoely86/easybuild-framework:
  drop forgotten print statement
@boegel boegel changed the title Adds support for extensions in Lmod modules adds support for including 'extensions' statement in Lua modules with Lmod 8.2+ Nov 29, 2019
minor style fixes in module_generator.py w.r.t. extensions support
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