From 2421d903b034c92660fbe999b7d486e6f21a7417 Mon Sep 17 00:00:00 2001
From: Greg Becker <becker33@llnl.gov>
Date: Tue, 9 Jun 2020 08:52:46 -0700
Subject: SpecList: fix recursion for references (#16897)

* SpecList: fix and refactor variable expansion
---
 lib/spack/spack/spec_list.py      | 71 +++++++++++++++++++++++----------------
 lib/spack/spack/test/spec_list.py | 17 ++++++++++
 2 files changed, 59 insertions(+), 29 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/spec_list.py b/lib/spack/spack/spec_list.py
index 4468df7c35..bc473f530c 100644
--- a/lib/spack/spack/spec_list.py
+++ b/lib/spack/spack/spec_list.py
@@ -121,38 +121,42 @@ class SpecList(object):
         self._constraints = None
         self._specs = None
 
+    def _parse_reference(self, name):
+        sigil = ''
+        name = name[1:]
+
+        # Parse specs as constraints
+        if name.startswith('^') or name.startswith('%'):
+            sigil = name[0]
+            name = name[1:]
+
+        # Make sure the reference is valid
+        if name not in self._reference:
+            msg = 'SpecList %s refers to ' % self.name
+            msg += 'named list %s ' % name
+            msg += 'which does not appear in its reference dict'
+            raise UndefinedReferenceError(msg)
+
+        return (name, sigil)
+
     def _expand_references(self, yaml):
         if isinstance(yaml, list):
-            for idx, item in enumerate(yaml):
+            ret = []
+
+            for item in yaml:
+                # if it's a reference, expand it
                 if isinstance(item, string_types) and item.startswith('$'):
-                    # Reference type can add a constraint to items
-                    if item[1] in ('^', '%'):
-                        name = item[2:]
-                        sigil = item[1]
-                    else:
-                        name = item[1:]
-                        sigil = ''
-                    if name in self._reference:
-                        ret = [self._expand_references(i) for i in yaml[:idx]]
-                        ret += self._reference[name].specs_as_yaml_list
-                        ret += self._expand_references(yaml[idx + 1:])
-
-                        # Add the sigil if we're mapping a sigil to a ref
-                        def sigilify(arg):
-                            if isinstance(arg, dict):
-                                if sigil:
-                                    arg['sigil'] = sigil
-                                return arg
-                            else:
-                                return sigil + arg
-                        return list(map(sigilify, ret))
-                    else:
-                        msg = 'SpecList %s refers to ' % self.name
-                        msg += 'named list %s ' % name
-                        msg += 'which does not appear in its reference dict'
-                        raise UndefinedReferenceError(msg)
-            # No references in this
-            return [self._expand_references(item) for item in yaml]
+                    # replace the reference and apply the sigil if needed
+                    name, sigil = self._parse_reference(item)
+                    referent = [
+                        _sigilify(item, sigil)
+                        for item in self._reference[name].specs_as_yaml_list
+                    ]
+                    ret.extend(referent)
+                else:
+                    # else just recurse
+                    ret.append(self._expand_references(item))
+            return ret
         elif isinstance(yaml, dict):
             # There can't be expansions in dicts
             return dict((name, self._expand_references(val))
@@ -216,6 +220,15 @@ def _expand_matrix_constraints(object, specify=True):
     return results
 
 
+def _sigilify(item, sigil):
+    if isinstance(item, dict):
+        if sigil:
+            item['sigil'] = sigil
+        return item
+    else:
+        return sigil + item
+
+
 class SpecListError(SpackError):
     """Error class for all errors related to SpecList objects."""
 
diff --git a/lib/spack/spack/test/spec_list.py b/lib/spack/spack/test/spec_list.py
index ff45096c3e..27567b4080 100644
--- a/lib/spack/spack/test/spec_list.py
+++ b/lib/spack/spack/test/spec_list.py
@@ -2,6 +2,7 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import pytest
 import itertools
 from spack.spec_list import SpecList
 from spack.spec import Spec
@@ -157,6 +158,22 @@ class TestSpecList(object):
         expected = [Spec(' '.join(combo)) for combo in expected_components]
         assert set(speclist.specs) == set(expected)
 
+    @pytest.mark.regression('16897')
+    def test_spec_list_recursion_specs_as_constraints(self):
+        input = ['mpileaks', '$mpis',
+                 {'matrix': [['hypre'], ['$%gccs', '$%clangs']]},
+                 'libelf']
+
+        reference = {'gccs': SpecList('gccs', ['gcc@4.5.0']),
+                     'clangs': SpecList('clangs', ['clang@3.3']),
+                     'mpis': SpecList('mpis', ['zmpi@1.0', 'mpich@3.0'])}
+
+        speclist = SpecList('specs', input, reference)
+
+        assert speclist.specs_as_yaml_list == self.default_expansion
+        assert speclist.specs_as_constraints == self.default_constraints
+        assert speclist.specs == self.default_specs
+
     def test_spec_list_matrix_exclude(self, mock_packages):
         # Test on non-boolean variants for regression for #16841
         matrix = [{'matrix': [['multivalue-variant'], ['foo=bar', 'foo=baz']],
-- 
cgit v1.2.3-70-g09d2