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.

4 files changed, 36 insertions, 9 deletions

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')