Variant satisfaction for indirect dependencies

Fixes #4898

Constraints that were supposed to be conditionally activated for
specified values of a single-valued variant were being activated
unconditionally in the case that the variant was associated with
an implicit dependency. For example if X->Y->Z and Y places a
conditional constraint on Z for a given single-valued variant on
Y, then it would have been applied unconditionally when
concretizing X.

6 files changed, 59 insertions, 1 deletions

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index cbbd48d9fa..32e08d3896 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -1879,6 +1879,7 @@ class Spec(object):
         pkg = spack.repo.get(self.fullname)
         conditions = pkg.dependencies[name]
 
+        substitute_abstract_variants(self)
         # evaluate when specs to figure out constraints on the dependency.
         dep = None
         for when_spec, dep_spec in conditions.items():
diff --git a/lib/spack/spack/test/cmd/dependents.py b/lib/spack/spack/test/cmd/dependents.py
index 69f57d88a3..546d6d48c9 100644
--- a/lib/spack/spack/test/cmd/dependents.py
+++ b/lib/spack/spack/test/cmd/dependents.py
@@ -42,7 +42,8 @@ def test_transitive_dependents(builtin_mock):
     out, err = dependents('--transitive', 'libelf')
     actual = set(re.split(r'\s+', out.strip()))
     assert actual == set(
-        ['callpath', 'dyninst', 'libdwarf', 'mpileaks', 'multivalue_variant'])
+        ['callpath', 'dyninst', 'libdwarf', 'mpileaks', 'multivalue_variant',
+         'singlevalue-variant-dependent'])
 
 
 def test_immediate_installed_dependents(builtin_mock, database):
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 1623133ef3..4ed7c635e9 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -307,6 +307,20 @@ class TestSpecSematics(object):
         # Check that conditional dependencies are treated correctly
         assert '^b' in a
 
+    def test_unsatisfied_single_valued_variant(self):
+        a = Spec('a foobar=baz')
+        a.concretize()
+        assert '^b' not in a
+
+        mv = Spec('multivalue_variant')
+        mv.concretize()
+        assert 'a@1.0' not in mv
+
+    def test_indirect_unsatisfied_single_valued_variant(self):
+        spec = Spec('singlevalue-variant-dependent')
+        spec.concretize()
+        assert 'a@1.0' not in spec
+
     def test_unsatisfiable_multi_value_variant(self):
 
         # Semantics for a multi-valued variant is different
diff --git a/var/spack/repos/builtin.mock/packages/a/package.py b/var/spack/repos/builtin.mock/packages/a/package.py
index e2fa6c35b0..037b322f10 100644
--- a/var/spack/repos/builtin.mock/packages/a/package.py
+++ b/var/spack/repos/builtin.mock/packages/a/package.py
@@ -32,6 +32,7 @@ class A(AutotoolsPackage):
     url      = "http://www.example.com/a-1.0.tar.gz"
 
     version('1.0', '0123456789abcdef0123456789abcdef')
+    version('2.0', '2.0_a_hash')
 
     variant(
         'foo',
diff --git a/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py b/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py
index f20bf7c369..946b9893d8 100644
--- a/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py
+++ b/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py
@@ -52,6 +52,8 @@ class MultivalueVariant(Package):
 
     depends_on('mpi')
     depends_on('callpath')
+    depends_on('a')
+    depends_on('a@1.0', when='fee=barbaz')
 
     def install(self, spec, prefix):
         pass
diff --git a/var/spack/repos/builtin.mock/packages/singlevalue-variant-dependent/package.py b/var/spack/repos/builtin.mock/packages/singlevalue-variant-dependent/package.py
new file mode 100644
index 0000000000..c6630927c7
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/singlevalue-variant-dependent/package.py
@@ -0,0 +1,39 @@
+##############################################################################
+# Copyright (c) 2013-2016, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://github.com/llnl/spack
+# Please also see the NOTICE and LICENSE files for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+from spack import *
+
+
+class SinglevalueVariantDependent(Package):
+    """Simple package with one optional dependency"""
+
+    homepage = "http://www.example.com"
+    url      = "http://www.example.com/archive-1.0.tar.gz"
+
+    version('1.0', '0123456789abcdef0123456789abcdef')
+
+    depends_on('multivalue_variant fee=baz')
+
+    def install(self, spec, prefix):
+        pass