From 0b57567824d23159f011e34b1a841b832308903c Mon Sep 17 00:00:00 2001 From: Matthias Wolf Date: Tue, 23 Jun 2020 22:38:04 +0200 Subject: Module index should not be unconditionally overwritten (#14837) * Module index should not be unconditionally overwritten Uncovered after we switched our CI to generate modules for packages one-by-one rather than in bulk. This overwrote a complete module index with an index with a single entry, and broke our downstream Spack instances that needed the upstream module index. --- lib/spack/spack/cmd/modules/__init__.py | 7 +++++-- lib/spack/spack/modules/common.py | 12 +++++++++--- lib/spack/spack/test/cmd/module.py | 9 +++++---- lib/spack/spack/test/modules/tcl.py | 17 +++++++++++++++++ 4 files changed, 36 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/modules/__init__.py b/lib/spack/spack/cmd/modules/__init__.py index 7d51224e14..7f3dd29ef1 100644 --- a/lib/spack/spack/cmd/modules/__init__.py +++ b/lib/spack/spack/cmd/modules/__init__.py @@ -289,15 +289,18 @@ def refresh(module_type, specs, args): msg = 'Nothing to be done for {0} module files.' tty.msg(msg.format(module_type)) return - # If we arrived here we have at least one writer module_type_root = writers[0].layout.dirname() - spack.modules.common.generate_module_index(module_type_root, writers) + # Proceed regenerating module files tty.msg('Regenerating {name} module files'.format(name=module_type)) if os.path.isdir(module_type_root) and args.delete_tree: shutil.rmtree(module_type_root, ignore_errors=False) filesystem.mkdirp(module_type_root) + + # Dump module index after potentially removing module tree + spack.modules.common.generate_module_index( + module_type_root, writers, overwrite=args.delete_tree) for x in writers: try: x.write(overwrite=True) diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index 95ef81415e..33e2c1a6d3 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -221,8 +221,15 @@ def root_path(name): return spack.util.path.canonicalize_path(path) -def generate_module_index(root, modules): - entries = syaml.syaml_dict() +def generate_module_index(root, modules, overwrite=False): + index_path = os.path.join(root, 'module-index.yaml') + if overwrite or not os.path.exists(index_path): + entries = syaml.syaml_dict() + else: + with open(index_path) as index_file: + yaml_content = syaml.load(index_file) + entries = yaml_content['module_index'] + for m in modules: entry = { 'path': m.layout.filename, @@ -230,7 +237,6 @@ def generate_module_index(root, modules): } entries[m.spec.dag_hash()] = entry index = {'module_index': entries} - index_path = os.path.join(root, 'module-index.yaml') llnl.util.filesystem.mkdirp(root) with open(index_path, 'w') as index_file: syaml.dump(index, default_flow_style=False, stream=index_file) diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py index f9aeb4af66..c3fe279306 100644 --- a/lib/spack/spack/test/cmd/module.py +++ b/lib/spack/spack/test/cmd/module.py @@ -14,6 +14,7 @@ from spack.test.conftest import use_store, use_configuration, use_repo module = spack.main.SpackCommand('module') + #: make sure module files are generated for all the tests here @pytest.fixture(scope='module', autouse=True) def ensure_module_files_are_there( @@ -168,10 +169,10 @@ def test_loads_recursive_blacklisted(database, module_configuration): output = module('lmod', 'loads', '-r', 'mpileaks ^mpich') lines = output.split('\n') - assert any(re.match(r'[^#]*module load.*mpileaks', l) for l in lines) - assert not any(re.match(r'[^#]module load.*callpath', l) for l in lines) - assert any(re.match(r'## blacklisted or missing.*callpath', l) - for l in lines) + assert any(re.match(r'[^#]*module load.*mpileaks', ln) for ln in lines) + assert not any(re.match(r'[^#]module load.*callpath', ln) for ln in lines) + assert any(re.match(r'## blacklisted or missing.*callpath', ln) + for ln in lines) # TODO: currently there is no way to separate stdout and stderr when # invoking a SpackCommand. Supporting this requires refactoring diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index 7672cdc676..41d8323456 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -236,6 +236,7 @@ class TestTcl(object): w1, s1 = factory('mpileaks') w2, s2 = factory('callpath') + w3, s3 = factory('openblas') test_root = str(tmpdir_factory.mktemp('module-root')) @@ -246,6 +247,22 @@ class TestTcl(object): assert index[s1.dag_hash()].use_name == w1.layout.use_name assert index[s2.dag_hash()].path == w2.layout.filename + spack.modules.common.generate_module_index(test_root, [w3]) + + index = spack.modules.common.read_module_index(test_root) + + assert len(index) == 3 + assert index[s1.dag_hash()].use_name == w1.layout.use_name + assert index[s2.dag_hash()].path == w2.layout.filename + + spack.modules.common.generate_module_index( + test_root, [w3], overwrite=True) + + index = spack.modules.common.read_module_index(test_root) + + assert len(index) == 1 + assert index[s3.dag_hash()].use_name == w3.layout.use_name + def test_suffixes(self, module_configuration, factory): """Tests adding suffixes to module file name.""" module_configuration('suffix') -- cgit v1.2.3-60-g2f50