Modules: Deduplicate suffixes but don't sort them. (#18351)

* Modules: Deduplicate suffixes but don't sort them.

The suffixes' order is defined by the order in which they appear in the configuration file.

* Modules: Modify tests to use spack_yaml.load_config.

spack_yaml.load_config ensures that the configuration is stored in an ordered manner. Without this change, the behavior of the tests did not match Spack's.

* Modules: Tweak the suffixes test to better catch ordering issues.

4 files changed, 11 insertions, 5 deletions

diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 33e2c1a6d3..42fc398e47 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -36,6 +36,7 @@ import os.path
 import re
 
 import llnl.util.filesystem
+from llnl.util.lang import dedupe
 import llnl.util.tty as tty
 import spack.build_environment as build_environment
 import spack.error
@@ -442,7 +443,7 @@ class BaseConfiguration(object):
         for constraint, suffix in self.conf.get('suffixes', {}).items():
             if constraint in self.spec:
                 suffixes.append(suffix)
-        suffixes = sorted(set(suffixes))
+        suffixes = list(dedupe(suffixes))
         if self.hash:
             suffixes.append(self.hash)
         return suffixes
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index eb52c32bd3..a8cdce90ee 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -16,7 +16,6 @@ import xml.etree.ElementTree
 
 import py
 import pytest
-import ruamel.yaml as yaml
 
 from llnl.util.filesystem import mkdirp, remove_linked_tree
 
@@ -35,6 +34,7 @@ import spack.repo
 import spack.stage
 import spack.util.executable
 import spack.util.gpg
+import spack.util.spack_yaml as syaml
 
 from spack.util.pattern import Bunch
 from spack.fetch_strategy import FetchStrategyComposite, URLFetchStrategy
@@ -748,7 +748,7 @@ def module_configuration(monkeypatch, request):
 
         file = os.path.join(root_for_conf, filename + '.yaml')
         with open(file) as f:
-            configuration = yaml.load(f)
+            configuration = syaml.load_config(f)
 
         def mock_config_function():
             return configuration
diff --git a/lib/spack/spack/test/data/modules/tcl/suffix.yaml b/lib/spack/spack/test/data/modules/tcl/suffix.yaml
index 57703e1733..db2e5f9d64 100644
--- a/lib/spack/spack/test/data/modules/tcl/suffix.yaml
+++ b/lib/spack/spack/test/data/modules/tcl/suffix.yaml
@@ -3,6 +3,7 @@ enable:
 tcl:
   mpileaks:
     suffixes:
+      '+opt': baz
       '+debug': foo
-      '~debug': bar
       '^mpich': foo
+      '~debug': bar
diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py
index 41d8323456..68f9af13c6 100644
--- a/lib/spack/spack/test/modules/tcl.py
+++ b/lib/spack/spack/test/modules/tcl.py
@@ -272,7 +272,11 @@ class TestTcl(object):
         assert 'foo-foo' not in writer.layout.use_name
 
         writer, spec = factory('mpileaks~debug arch=x86-linux')
-        assert 'bar-foo' in writer.layout.use_name
+        assert 'foo-bar' in writer.layout.use_name
+        assert 'baz' not in writer.layout.use_name
+
+        writer, spec = factory('mpileaks~debug+opt arch=x86-linux')
+        assert 'baz-foo-bar' in writer.layout.use_name
 
     def test_setup_environment(self, modulefile_content, module_configuration):
         """Tests the internal set-up of run-time environment."""