Fix issues when a package provides the same vdep twice. (#2710)

* Fix issues when a package provides the same vdep twice.

- provides() now adds to a set of provided vdeps instead of a single one.

* flake8

lib/spack/spack/cmd/info.py

@@ -106,8 +106,15 @@ def print_text_info(pkg):
     print
     print "Virtual Packages: "
     if pkg.provided:
-        for spec, when in pkg.provided.items():
+        inverse_map = {}
-            print "    %s provides %s" % (when, spec)
+        for spec, whens in pkg.provided.items():
+            for when in whens:
+                if when not in inverse_map:
+                    inverse_map[when] = set()
+                inverse_map[when].add(spec)
+        for when, specs in reversed(sorted(inverse_map.items())):
+            print "    %s provides %s" % (
+                when, ', '.join(str(s) for s in specs))
     else:
         print "    None"
 

lib/spack/spack/directives.py

@@ -313,7 +313,9 @@ def _execute(pkg):
             for provided_spec in spack.spec.parse(string):
                 if pkg.name == provided_spec.name:
                     raise CircularReferenceError('depends_on', pkg.name)
-                pkg.provided[provided_spec] = provider_spec
+                if provided_spec not in pkg.provided:
+                    pkg.provided[provided_spec] = set()
+                pkg.provided[provided_spec].add(provider_spec)
     return _execute
 
 

lib/spack/spack/provider_index.py

@@ -97,35 +97,38 @@ def update(self, spec):
         assert(not spec.virtual)
 
         pkg = spec.package
-        for provided_spec, provider_spec in pkg.provided.iteritems():
+        for provided_spec, provider_specs in pkg.provided.iteritems():
-            # We want satisfaction other than flags
+            for provider_spec in provider_specs:
-            provider_spec.compiler_flags = spec.compiler_flags.copy()
+                # TODO: fix this comment.
+                # We want satisfaction other than flags
+                provider_spec.compiler_flags = spec.compiler_flags.copy()
 
-            if spec.satisfies(provider_spec, deps=False):
+                if spec.satisfies(provider_spec, deps=False):
-                provided_name = provided_spec.name
+                    provided_name = provided_spec.name
 
-                provider_map = self.providers.setdefault(provided_name, {})
+                    provider_map = self.providers.setdefault(provided_name, {})
-                if provided_spec not in provider_map:
+                    if provided_spec not in provider_map:
-                    provider_map[provided_spec] = set()
+                        provider_map[provided_spec] = set()
 
-                if self.restrict:
+                    if self.restrict:
-                    provider_set = provider_map[provided_spec]
+                        provider_set = provider_map[provided_spec]
 
-                    # If this package existed in the index before,
+                        # If this package existed in the index before,
-                    # need to take the old versions out, as they're
+                        # need to take the old versions out, as they're
-                    # now more constrained.
+                        # now more constrained.
-                    old = set([s for s in provider_set if s.name == spec.name])
+                        old = set(
-                    provider_set.difference_update(old)
+                            [s for s in provider_set if s.name == spec.name])
+                        provider_set.difference_update(old)
 
-                    # Now add the new version.
+                        # Now add the new version.
-                    provider_set.add(spec)
+                        provider_set.add(spec)
 
-                else:
+                    else:
-                    # Before putting the spec in the map, constrain it so that
+                        # Before putting the spec in the map, constrain
-                    # it provides what was asked for.
+                        # it so that it provides what was asked for.
-                    constrained = spec.copy()
+                        constrained = spec.copy()
-                    constrained.constrain(provider_spec)
+                        constrained.constrain(provider_spec)
-                    provider_map[provided_spec].add(constrained)
+                        provider_map[provided_spec].add(constrained)
 
     def providers_for(self, *vpkg_specs):
         """Gives specs of all packages that provide virtual packages

lib/spack/spack/spec.py

@@ -2020,8 +2020,9 @@ def satisfies(self, other, deps=True, strict=False):
         if not self.virtual and other.virtual:
             pkg = spack.repo.get(self.fullname)
             if pkg.provides(other.name):
-                for provided, when_spec in pkg.provided.items():
+                for provided, when_specs in pkg.provided.items():
-                    if self.satisfies(when_spec, deps=False, strict=strict):
+                    if any(self.satisfies(when_spec, deps=False, strict=strict)
+                           for when_spec in when_specs):
                         if provided.satisfies(other):
                             return True
             return False

lib/spack/spack/test/concretize.py

@@ -161,6 +161,25 @@ def test_concretize_with_provides_when(self):
             s.satisfies('mpich2') for s in repo.providers_for('mpi@3')
         )
 
+    def test_provides_handles_multiple_providers_of_same_vesrion(self):
+        """
+        """
+        providers = spack.repo.providers_for('mpi@3.0')
+
+        # Note that providers are repo-specific, so we don't misinterpret
+        # providers, but vdeps are not namespace-specific, so we can
+        # associate vdeps across repos.
+        assert Spec('builtin.mock.multi-provider-mpi@1.10.3') in providers
+        assert Spec('builtin.mock.multi-provider-mpi@1.10.2') in providers
+        assert Spec('builtin.mock.multi-provider-mpi@1.10.1') in providers
+        assert Spec('builtin.mock.multi-provider-mpi@1.10.0') in providers
+        assert Spec('builtin.mock.multi-provider-mpi@1.8.8') in providers
+
+    def concretize_multi_provider(self):
+        s = Spec('mpileaks ^multi-provider-mpi@3.0')
+        s.concretize()
+        assert s['mpi'].version == ver('1.10.3')
+
     def test_concretize_two_virtuals(self):
         """Test a package with multiple virtual dependencies."""
         Spec('hypre').concretize()

var/spack/repos/builtin.mock/packages/multi-provider-mpi/package.py

@@ -0,0 +1,51 @@
+##############################################################################
+# 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 LICENSE file 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 MultiProviderMpi(Package):
+    """This is a fake MPI package used to test packages providing multiple
+       virtuals at the same version."""
+    homepage = "http://www.spack-fake-mpi.org"
+    url      = "http://www.spack-fake-mpi.org/downloads/multi-mpi-1.0.tar.gz"
+
+    version('2.0.0', 'foobarbaz')
+    version('1.10.3', 'foobarbaz')
+    version('1.10.2', 'foobarbaz')
+    version('1.10.1', 'foobarbaz')
+    version('1.10.0', 'foobarbaz')
+    version('1.8.8', 'foobarbaz')
+    version('1.6.5', 'foobarbaz')
+
+    provides('mpi@3.1', when='@2.0.0')
+    provides('mpi@3.0', when='@1.10.3')
+    provides('mpi@3.0', when='@1.10.2')
+    provides('mpi@3.0', when='@1.10.1')
+    provides('mpi@3.0', when='@1.10.0')
+    provides('mpi@3.0', when='@1.8.8')
+    provides('mpi@2.2', when='@1.6.5')
+
+    def install(self, spec, prefix):
+        pass