From 1d70b590fc4579c4e6b4bd592bc58eabf330fd9b Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 11 Mar 2016 13:20:57 +0100
Subject: build_environment : fixed minor spelling errors and a few style
 issues

---
 lib/spack/spack/build_environment.py | 37 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 87fc310b5a..392ba7ea4d 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -3,7 +3,7 @@ This module contains all routines related to setting up the package
 build environment.  All of this is set up by package.py just before
 install() is called.
 
-There are two parts to the bulid environment:
+There are two parts to the build environment:
 
 1. Python build environment (i.e. install() method)
 
@@ -13,7 +13,7 @@ There are two parts to the bulid environment:
    the package's module scope.  Ths allows package writers to call
    them all directly in Package.install() without writing 'self.'
    everywhere.  No, this isn't Pythonic.  Yes, it makes the code more
-   readable and more like the shell script from whcih someone is
+   readable and more like the shell script from which someone is
    likely porting.
 
 2. Build execution environment
@@ -27,17 +27,16 @@ There are two parts to the bulid environment:
 Skimming this module is a nice way to get acquainted with the types of
 calls you can make from within the install() function.
 """
-import os
-import sys
-import shutil
 import multiprocessing
+import os
 import platform
-from llnl.util.filesystem import *
+import shutil
+import sys
 
 import spack
-import spack.compilers as compilers
-from spack.util.executable import Executable, which
+from llnl.util.filesystem import *
 from spack.util.environment import *
+from spack.util.executable import Executable, which
 
 #
 # This can be set by the user to globally disable parallel builds.
@@ -107,18 +106,19 @@ def set_compiler_environment_variables(pkg):
     if compiler.fc:
         os.environ['SPACK_FC']  = compiler.fc
 
-    os.environ['SPACK_COMPILER_SPEC']  = str(pkg.spec.compiler)
+    os.environ['SPACK_COMPILER_SPEC'] = str(pkg.spec.compiler)
 
 
 def set_build_environment_variables(pkg):
-    """This ensures a clean install environment when we build packages.
+    """
+    This ensures a clean install environment when we build packages
     """
     # Add spack build environment path with compiler wrappers first in
     # the path. We add both spack.env_path, which includes default
     # wrappers (cc, c++, f77, f90), AND a subdirectory containing
     # compiler-specific symlinks.  The latter ensures that builds that
     # are sensitive to the *name* of the compiler see the right name
-    # when we're building wtih the wrappers.
+    # when we're building with the wrappers.
     #
     # Conflicts on case-insensitive systems (like "CC" and "cc") are
     # handled by putting one in the <build_env_path>/case-insensitive
@@ -296,23 +296,23 @@ def fork(pkg, function):
            # do stuff
        build_env.fork(pkg, child_fun)
 
-    Forked processes are run with the build environemnt set up by
+    Forked processes are run with the build environment set up by
     spack.build_environment.  This allows package authors to have
-    full control over the environment, etc. without offecting
+    full control over the environment, etc. without affecting
     other builds that might be executed in the same spack call.
 
-    If something goes wrong, the child process is expected toprint
+    If something goes wrong, the child process is expected to print
     the error and the parent process will exit with error as
     well. If things go well, the child exits and the parent
     carries on.
     """
     try:
         pid = os.fork()
-    except OSError, e:
+    except OSError as e:
         raise InstallError("Unable to fork build process: %s" % e)
 
     if pid == 0:
-        # Give the child process the package's build environemnt.
+        # Give the child process the package's build environment.
         setup_package(pkg)
 
         try:
@@ -323,7 +323,7 @@ def fork(pkg, function):
             # which interferes with unit tests.
             os._exit(0)
 
-        except spack.error.SpackError, e:
+        except spack.error.SpackError as e:
             e.die()
 
         except:
@@ -338,8 +338,7 @@ def fork(pkg, function):
         # message.  Just make the parent exit with an error code.
         pid, returncode = os.waitpid(pid, 0)
         if returncode != 0:
-            raise InstallError("Installation process had nonzero exit code."
-                .format(str(returncode)))
+            raise InstallError("Installation process had nonzero exit code.".format(str(returncode)))
 
 
 class InstallError(spack.error.SpackError):
-- 
cgit v1.2.3-70-g09d2


From 3383486adc86ba03456aae3703c0176620888e77 Mon Sep 17 00:00:00 2001
From: Elizabeth F <rpf2116@columbia.edu>
Date: Sun, 13 Mar 2016 19:38:05 -0400
Subject: Fixed typo bug.  Made error comment more explicit

---
 lib/spack/spack/cmd/diy.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/diy.py b/lib/spack/spack/cmd/diy.py
index 2c3a8761ab..199362d915 100644
--- a/lib/spack/spack/cmd/diy.py
+++ b/lib/spack/spack/cmd/diy.py
@@ -75,8 +75,8 @@ def diy(self, args):
                 edit_package(spec.name, spack.repo.first_repo(), None, True)
                 return
 
-        if not spec.version.concrete:
-            tty.die("spack diy spec must have a single, concrete version.")
+        if not spec.versions.concrete:
+            tty.die("spack spconfig spec must have a single, concrete version.  Did you forget a package version number?")
 
         spec.concretize()
         package = spack.repo.get(spec)
-- 
cgit v1.2.3-70-g09d2


From 5c865b9b702695797ab29263fb629b53e56983fb Mon Sep 17 00:00:00 2001
From: Elizabeth F <rpf2116@columbia.edu>
Date: Sun, 13 Mar 2016 19:42:15 -0400
Subject: Fixed typo in typo fix.

---
 lib/spack/spack/cmd/diy.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/diy.py b/lib/spack/spack/cmd/diy.py
index 199362d915..45f13e4463 100644
--- a/lib/spack/spack/cmd/diy.py
+++ b/lib/spack/spack/cmd/diy.py
@@ -76,7 +76,7 @@ def diy(self, args):
                 return
 
         if not spec.versions.concrete:
-            tty.die("spack spconfig spec must have a single, concrete version.  Did you forget a package version number?")
+            tty.die("spack diy spec must have a single, concrete version.  Did you forget a package version number?")
 
         spec.concretize()
         package = spack.repo.get(spec)
-- 
cgit v1.2.3-70-g09d2


From 003fd4d834d3e06ea89c7f3c2fa13241b0730f06 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 14 Mar 2016 04:55:30 -0700
Subject: Optimize __eq__ and __ne__ in key_ordering

- use `is` when possible before calling `_cmp_key()`
---
 lib/spack/llnl/util/lang.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py
index 1c4d1ed623..13d301f84e 100644
--- a/lib/spack/llnl/util/lang.py
+++ b/lib/spack/llnl/util/lang.py
@@ -235,11 +235,11 @@ def key_ordering(cls):
     if not has_method(cls, '_cmp_key'):
         raise TypeError("'%s' doesn't define _cmp_key()." % cls.__name__)
 
-    setter('__eq__', lambda s,o: o is not None and s._cmp_key() == o._cmp_key())
+    setter('__eq__', lambda s,o: (s is o) or (o is not None and s._cmp_key() == o._cmp_key()))
     setter('__lt__', lambda s,o: o is not None and s._cmp_key() <  o._cmp_key())
     setter('__le__', lambda s,o: o is not None and s._cmp_key() <= o._cmp_key())
 
-    setter('__ne__', lambda s,o: o is None or s._cmp_key() != o._cmp_key())
+    setter('__ne__', lambda s,o: (s is not o) and (o is None or s._cmp_key() != o._cmp_key()))
     setter('__gt__', lambda s,o: o is None or s._cmp_key() >  o._cmp_key())
     setter('__ge__', lambda s,o: o is None or s._cmp_key() >= o._cmp_key())
 
-- 
cgit v1.2.3-70-g09d2


From 05c761dee9c86faf9ce6d5b98ae57c8737694898 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 14 Mar 2016 04:59:29 -0700
Subject: Add `package_class` method to spec.

- Shouldn't call .package from within things like normalize() and
  concretize() beacuse spec may be inconsistent.

- Add `.package_class` property so that we can get at package metadata
  without constructing a Package with a Spec.

- should be faster than `.package` was, anyway. Use where possible.
---
 lib/spack/spack/concretize.py |  2 +-
 lib/spack/spack/repository.py |  9 +++++++--
 lib/spack/spack/spec.py       | 12 ++++++++++--
 3 files changed, 18 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index 8d29a03f93..445ecd8896 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -238,7 +238,7 @@ class DefaultConcretizer(object):
            the default variants from the package specification.
         """
         changed = False
-        for name, variant in spec.package.variants.items():
+        for name, variant in spec.package_class.variants.items():
             if name not in spec.variants:
                 spec.variants[name] = spack.spec.VariantSpec(name, variant.default)
                 changed = True
diff --git a/lib/spack/spack/repository.py b/lib/spack/spack/repository.py
index 3c3ba08bcc..d2fdc937f7 100644
--- a/lib/spack/spack/repository.py
+++ b/lib/spack/spack/repository.py
@@ -316,6 +316,11 @@ class RepoPath(object):
         return self.repo_for_pkg(spec).get(spec)
 
 
+    def get_pkg_class(self, pkg_name):
+        """Find a class for the spec's package and return the class object."""
+        return self.repo_for_pkg(pkg_name).get_pkg_class(pkg_name)
+
+
     @_autospec
     def dump_provenance(self, spec, path):
         """Dump provenance information for a spec to a particular path.
@@ -550,7 +555,7 @@ class Repo(object):
 
         key = hash(spec)
         if new or key not in self._instances:
-            package_class = self._get_pkg_class(spec.name)
+            package_class = self.get_pkg_class(spec.name)
             try:
                 copy = spec.copy() # defensive copy.  Package owns its spec.
                 self._instances[key] = package_class(copy)
@@ -715,7 +720,7 @@ class Repo(object):
         return self._modules[pkg_name]
 
 
-    def _get_pkg_class(self, pkg_name):
+    def get_pkg_class(self, pkg_name):
         """Get the class for the package out of its module.
 
         First loads (or fetches from cache) a module for the
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index c045e80365..573e288d17 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -353,7 +353,7 @@ class VariantMap(HashableMap):
     @property
     def concrete(self):
         return self.spec._concrete or all(
-            v in self for v in self.spec.package.variants)
+            v in self for v in self.spec.package_class.variants)
 
 
     def copy(self):
@@ -498,6 +498,14 @@ class Spec(object):
         return spack.repo.get(self)
 
 
+    @property
+    def package_class(self):
+        """Internal package call gets only the class object for a package.
+           Use this to just get package metadata.
+        """
+        return spack.repo.get_pkg_class(self.name)
+
+
     @property
     def virtual(self):
         """Right now, a spec is virtual if no package exists with its name.
@@ -1161,7 +1169,7 @@ class Spec(object):
 
             # Ensure that variants all exist.
             for vname, variant in spec.variants.items():
-                if vname not in spec.package.variants:
+                if vname not in spec.package_class.variants:
                     raise UnknownVariantError(spec.name, vname)
 
 
-- 
cgit v1.2.3-70-g09d2


From f45b8b1083e5f628dd31fa4b7b873b6df7119d0e Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 14 Mar 2016 05:02:50 -0700
Subject: Add some tests for packages with multiple virtual dependencies.

- Added mock `hypre` package, depends on `lapack` and `blas`.

- test cases where some packages provide both `lapack` and `blas`, but
  others do not.
---
 lib/spack/spack/test/concretize.py                 | 29 +++++++++++++++-
 .../repos/builtin.mock/packages/hypre/package.py   | 39 ++++++++++++++++++++++
 .../packages/openblas-with-lapack/package.py       | 38 +++++++++++++++++++++
 3 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 var/spack/repos/builtin.mock/packages/hypre/package.py
 create mode 100644 var/spack/repos/builtin.mock/packages/openblas-with-lapack/package.py

(limited to 'lib')

diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 07828d8ea6..f264faf17a 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -142,6 +142,34 @@ class ConcretizeTest(MockPackagesTest):
                                 for spec in spack.repo.providers_for('mpi@3')))
 
 
+    def test_concretize_two_virtuals(self):
+        """Test a package with multiple virtual dependencies."""
+        s = Spec('hypre').concretize()
+
+
+    def test_concretize_two_virtuals_with_one_bound(self):
+        """Test a package with multiple virtual dependencies and one preset."""
+        s = Spec('hypre ^openblas').concretize()
+
+
+    def test_concretize_two_virtuals_with_two_bound(self):
+        """Test a package with multiple virtual dependencies and two of them preset."""
+        s = Spec('hypre ^openblas ^netlib-lapack').concretize()
+
+
+    def test_concretize_two_virtuals_with_dual_provider(self):
+        """Test a package with multiple virtual dependencies and force a provider
+           that provides both."""
+        s = Spec('hypre ^openblas-with-lapack').concretize()
+
+
+    def test_concretize_two_virtuals_with_dual_provider_and_a_conflict(self):
+        """Test a package with multiple virtual dependencies and force a provider
+           that provides both, and another conflicting package that provides one."""
+        s = Spec('hypre ^openblas-with-lapack ^netlib-lapack')
+        self.assertRaises(spack.spec.MultipleProviderError, s.concretize)
+
+
     def test_virtual_is_fully_expanded_for_callpath(self):
         # force dependence on fake "zmpi" by asking for MPI 10.0
         spec = Spec('callpath ^mpi@10.0')
@@ -281,4 +309,3 @@ class ConcretizeTest(MockPackagesTest):
                       Spec('d')),
                  Spec('e'))
         self.assertEqual(None, find_spec(s['b'], lambda s: '+foo' in s))
-
diff --git a/var/spack/repos/builtin.mock/packages/hypre/package.py b/var/spack/repos/builtin.mock/packages/hypre/package.py
new file mode 100644
index 0000000000..f69f16d2cc
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/hypre/package.py
@@ -0,0 +1,39 @@
+##############################################################################
+# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Written 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 General Public License (as published by
+# the Free Software Foundation) version 2.1 dated 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 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 Hypre(Package):
+    """Hypre is included here as an example of a package that depends on
+       both LAPACK and BLAS."""
+    homepage = "http://www.openblas.net"
+    url      = "http://github.com/xianyi/OpenBLAS/archive/v0.2.15.tar.gz"
+
+    version('0.2.15', 'b1190f3d3471685f17cfd1ec1d252ac9')
+
+    depends_on('lapack')
+    depends_on('blas')
+
+    def install(self, spec, prefix):
+        pass
diff --git a/var/spack/repos/builtin.mock/packages/openblas-with-lapack/package.py b/var/spack/repos/builtin.mock/packages/openblas-with-lapack/package.py
new file mode 100644
index 0000000000..509bfb71e5
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/openblas-with-lapack/package.py
@@ -0,0 +1,38 @@
+##############################################################################
+# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Written 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 General Public License (as published by
+# the Free Software Foundation) version 2.1 dated 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 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 OpenblasWithLapack(Package):
+    """Dummy version of OpenBLAS that also provides LAPACK, for testing."""
+    homepage = "http://www.openblas.net"
+    url      = "http://github.com/xianyi/OpenBLAS/archive/v0.2.15.tar.gz"
+
+    version('0.2.15', 'b1190f3d3471685f17cfd1ec1d252ac9')
+
+    provides('lapack')
+    provides('blas')
+
+    def install(self, spec, prefix):
+        pass
-- 
cgit v1.2.3-70-g09d2


From f2761270f3c0506a689b484b0e12d7d6e9f4300d Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 14 Mar 2016 05:04:01 -0700
Subject: Make concretization less greedy: add backtracking for virtuals.

- `_expand_virtual_packages` now gets a candidate list and will try
  all the candidates.

  - Good news: If the first virtual in the list conflicts with something else in
               the spec, we'll keep trying until we find a good one.

  - Bad news: Only looks as far as the next normalize(); can't see
              conflicts further ahead than that if they're inevitable
              some other virtual expansion.

- Refactor `concretize.py` to keep all the nasty spec graph stitching in
  `spec.py`. This is more similar to before externals support.

  - `concretize.py` now just returns a list of candidates sorted by
    ABI compatibility to `_expand_virtual_packages`, and `spec.py`
    handles testing the candidates.

- Refactor the way external paths are handled in `config.py` and `concretize.py`:

  - previously, `spec_externals` returned spec/path pairs.  Now it
    returns specs with `external` set. Makes code in `concretize.py`
    more natural.
---
 lib/spack/spack/concretize.py | 140 +++++++++++++++++-------------------------
 lib/spack/spack/config.py     |  17 ++---
 lib/spack/spack/spec.py       | 129 +++++++++++++++++++++++++++++++-------
 3 files changed, 174 insertions(+), 112 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index 445ecd8896..8083f91982 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -51,10 +51,10 @@ class DefaultConcretizer(object):
     """
 
     def _valid_virtuals_and_externals(self, spec):
-        """Returns a list of spec/external-path pairs for both virtuals and externals
-           that can concretize this spec."""
-        # Get a list of candidate packages that could satisfy this spec
-        packages = []
+        """Returns a list of candidate virtual dep providers and external
+           packages that coiuld be used to concretize a spec."""
+        # First construct a list of concrete candidates to replace spec with.
+        candidates = [spec]
         if spec.virtual:
             providers = spack.repo.providers_for(spec)
             if not providers:
@@ -64,96 +64,72 @@ class DefaultConcretizer(object):
             if not spec_w_preferred_providers:
                 spec_w_preferred_providers = spec
             provider_cmp = partial(spack.pkgsort.provider_compare, spec_w_preferred_providers.name, spec.name)
-            packages = sorted(providers, cmp=provider_cmp)
-        else:
-            packages = [spec]
-
-        # For each candidate package, if it has externals add those to the candidates
-        # if it's not buildable, then only add the externals.
-        candidates = []
-        all_compilers = spack.compilers.all_compilers()
-        for pkg in packages:
-            externals = spec_externals(pkg)
-            buildable = is_spec_buildable(pkg)
-            if buildable:
-                candidates.append((pkg, None))
+            candidates = sorted(providers, cmp=provider_cmp)
+
+        # For each candidate package, if it has externals, add those to the usable list.
+        # if it's not buildable, then *only* add the externals.
+        usable = []
+        for cspec in candidates:
+            if is_spec_buildable(cspec):
+                usable.append(cspec)
+            externals = spec_externals(cspec)
             for ext in externals:
-                if ext[0].satisfies(spec):
-                    candidates.append(ext)
-        if not candidates:
+                if ext.satisfies(spec):
+                    usable.append(ext)
+
+        # If nothing is in the usable list now, it's because we aren't
+        # allowed to build anything.
+        if not usable:
             raise NoBuildError(spec)
 
         def cmp_externals(a, b):
-            if a[0].name != b[0].name:
-                #We're choosing between different providers. Maintain order from above sort
+            if a.name != b.name:
+                # We're choosing between different providers, so
+                # maintain order from provider sort
                 return candidates.index(a) - candidates.index(b)
-            result = cmp_specs(a[0], b[0])
+
+            result = cmp_specs(a, b)
             if result != 0:
                 return result
-            if not a[1] and b[1]:
-                return 1
-            if not b[1] and a[1]:
-                return -1
-            return cmp(a[1], b[1])
 
-        candidates = sorted(candidates, cmp=cmp_externals)
-        return candidates
+            # prefer external packages to internal packages.
+            if a.external is None or b.external is None:
+                return -cmp(a.external, b.external)
+            else:
+                return cmp(a.external, b.external)
+
+        usable.sort(cmp=cmp_externals)
+        return usable
 
 
-    def concretize_virtual_and_external(self, spec):
-        """From a list of candidate virtual and external packages, concretize to one that
-           is ABI compatible with the rest of the DAG."""
+    def choose_virtual_or_external(self, spec):
+        """Given a list of candidate virtual and external packages, try to
+           find one that is most ABI compatible.
+        """
         candidates = self._valid_virtuals_and_externals(spec)
         if not candidates:
-            return False
-
-        # Find the nearest spec in the dag that has a compiler.  We'll use that
-        # spec to test compiler compatibility.
-        other_spec = find_spec(spec, lambda(x): x.compiler)
-        if not other_spec:
-            other_spec = spec.root
-
-        # Choose an ABI-compatible candidate, or the first match otherwise.
-        candidate = None
-        if other_spec:
-            candidate = next((c for c in candidates if spack.abi.compatible(c[0], other_spec)), None)
-            if not candidate:
-                # Try a looser ABI matching
-                candidate = next((c for c in candidates if spack.abi.compatible(c[0], other_spec, loose=True)), None)
-        if not candidate:
-            # No ABI matches. Pick the top choice based on the orignal preferences.
-            candidate = candidates[0]
-        candidate_spec = candidate[0]
-        external = candidate[1]
-        changed = False
-
-        # If we're external then trim the dependencies
-        if external:
-            if (spec.dependencies):
-                changed = True
-            spec.dependencies = DependencyMap()
-            candidate_spec.dependencies = DependencyMap()
-
-        def fequal(candidate_field, spec_field):
-            return (not candidate_field) or (candidate_field == spec_field)
-        if (fequal(candidate_spec.name, spec.name) and
-            fequal(candidate_spec.versions, spec.versions) and
-            fequal(candidate_spec.compiler, spec.compiler) and
-            fequal(candidate_spec.architecture, spec.architecture) and
-            fequal(candidate_spec.dependencies, spec.dependencies) and
-            fequal(candidate_spec.variants, spec.variants) and
-            fequal(external, spec.external)):
-            return changed
-
-        # Refine this spec to the candidate.
-        if spec.virtual:
-            spec._replace_with(candidate_spec)
-            changed = True
-        if spec._dup(candidate_spec, deps=False, cleardeps=False):
-            changed = True
-        spec.external = external
-
-        return changed
+            return candidates
+
+        # Find the nearest spec in the dag that has a compiler.  We'll
+        # use that spec to calibrate compiler compatibility.
+        abi_exemplar = find_spec(spec, lambda(x): x.compiler)
+        if not abi_exemplar:
+            abi_exemplar = spec.root
+
+        # Make a list including ABI compatibility of specs with the exemplar.
+        strict = [spack.abi.compatible(c, abi_exemplar) for c in candidates]
+        loose  = [spack.abi.compatible(c, abi_exemplar, loose=True) for c in candidates]
+        keys = zip(strict, loose, candidates)
+
+        # Sort candidates from most to least compatibility.
+        # Note:
+        #   1. We reverse because True > False.
+        #   2. Sort is stable, so c's keep their order.
+        keys.sort(key=lambda k:k[:2], reverse=True)
+
+        # Pull the candidates back out and return them in order
+        candidates = [c for s,l,c in keys]
+        return candidates
 
 
     def concretize_version(self, spec):
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index a21dd6dbe1..6afd69b3ac 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -539,22 +539,25 @@ def print_section(section):
 
 
 def spec_externals(spec):
-    """Return a list of spec, directory pairs for each external location for spec"""
+    """Return a list of external specs (with external directory path filled in),
+       one for each known external installation."""
     allpkgs = get_config('packages')
     name = spec.name
-    spec_locations = []
 
+    external_specs = []
     pkg_paths = allpkgs.get(name, {}).get('paths', None)
     if not pkg_paths:
         return []
 
-    for pkg,path in pkg_paths.iteritems():
-        if not spec.satisfies(pkg):
-            continue
+    for external_spec, path in pkg_paths.iteritems():
         if not path:
+            # skip entries without paths (avoid creating extra Specs)
             continue
-        spec_locations.append( (spack.spec.Spec(pkg), path) )
-    return spec_locations
+
+        external_spec = spack.spec.Spec(external_spec, external=path)
+        if external_spec.satisfies(spec):
+            external_specs.append(external_spec)
+    return external_specs
 
 
 def is_spec_buildable(spec):
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 573e288d17..d04135860e 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -418,9 +418,11 @@ class Spec(object):
         # cases we've read them from a file want to assume normal.
         # This allows us to manipulate specs that Spack doesn't have
         # package.py files for.
-        self._normal = kwargs.get('normal', False)
+        self._normal   = kwargs.get('normal', False)
         self._concrete = kwargs.get('concrete', False)
-        self.external = None
+
+        # Allow a spec to be constructed with an external path.
+        self.external  = kwargs.get('external', None)
 
         # This allows users to construct a spec DAG with literals.
         # Note that given two specs a and b, Spec(a) copies a, but
@@ -794,8 +796,30 @@ class Spec(object):
         """Replace this virtual spec with a concrete spec."""
         assert(self.virtual)
         for name, dependent in self.dependents.items():
+            # remove self from all dependents.
             del dependent.dependencies[self.name]
-            dependent._add_dependency(concrete)
+
+            # add the replacement, unless it is already a dep of dependent.
+            if concrete.name not in dependent.dependencies:
+                dependent._add_dependency(concrete)
+
+
+    def _replace_node(self, replacement):
+        """Replace this spec with another.
+
+        Connects all dependents of this spec to its replacement, and
+        disconnects this spec from any dependencies it has. New spec
+        will have any dependencies the replacement had, and may need
+        to be normalized.
+
+        """
+        for name, dependent in self.dependents.items():
+            del dependent.dependencies[self.name]
+            dependent._add_dependency(replacement)
+
+        for name, dep in self.dependencies.items():
+            del dep.dependents[self.name]
+            del self.dependencies[dep.name]
 
 
     def _expand_virtual_packages(self):
@@ -815,18 +839,80 @@ class Spec(object):
               this are infrequent, but should implement this before it is
               a problem.
         """
+        # Make an index of stuff this spec already provides
+        self_index = ProviderIndex(self.traverse(), restrict=True)
+
         changed = False
         done = False
         while not done:
             done = True
             for spec in list(self.traverse()):
-                if spack.concretizer.concretize_virtual_and_external(spec):
-                    done = False
+                replacement = None
+                if spec.virtual:
+                    replacement = self._find_provider(spec, self_index)
+                    if replacement:
+                        # TODO: may break if in-place on self but
+                        # shouldn't happen if root is traversed first.
+                        spec._replace_with(replacement)
+                        done=False
+                        break
+
+                if not replacement:
+                    # Get a list of possible replacements in order of preference.
+                    candidates = spack.concretizer.choose_virtual_or_external(spec)
+
+                    # Try the replacements in order, skipping any that cause
+                    # satisfiability problems.
+                    for replacement in candidates:
+                        if replacement is spec:
+                            break
+
+                        # Replace spec with the candidate and normalize
+                        copy = self.copy()
+                        copy[spec.name]._dup(replacement.copy(deps=False))
+
+                        try:
+                            # If there are duplicate providers or duplicate provider
+                            # deps, consolidate them and merge constraints.
+                            copy.normalize(force=True)
+                            break
+                        except SpecError as e:
+                            # On error, we'll try the next replacement.
+                            continue
+
+                # If replacement is external then trim the dependencies
+                if replacement.external:
+                    if (spec.dependencies):
+                        changed = True
+                        spec.dependencies = DependencyMap()
+                    replacement.dependencies = DependencyMap()
+
+                # TODO: could this and the stuff in _dup be cleaned up?
+                def feq(cfield, sfield):
+                    return (not cfield) or (cfield == sfield)
+
+                if replacement is spec or (feq(replacement.name, spec.name) and
+                    feq(replacement.versions, spec.versions) and
+                    feq(replacement.compiler, spec.compiler) and
+                    feq(replacement.architecture, spec.architecture) and
+                    feq(replacement.dependencies, spec.dependencies) and
+                    feq(replacement.variants, spec.variants) and
+                    feq(replacement.external, spec.external)):
+                    continue
+
+                # Refine this spec to the candidate. This uses
+                # replace_with AND dup so that it can work in
+                # place. TODO: make this more efficient.
+                if spec.virtual:
+                    spec._replace_with(replacement)
                     changed = True
+                if spec._dup(replacement, deps=False, cleardeps=False):
+                    changed = True
+
+                self_index.update(spec)
+                done=False
+                break
 
-        # If there are duplicate providers or duplicate provider deps, this
-        # consolidates them and merge constraints.
-        changed |= self.normalize(force=True)
         return changed
 
 
@@ -850,7 +936,7 @@ class Spec(object):
         force = False
 
         while changed:
-            changes = (self.normalize(force=force),
+            changes = (self.normalize(force),
                        self._expand_virtual_packages(),
                        self._concretize_helper())
             changed = any(changes)
@@ -976,8 +1062,8 @@ class Spec(object):
 
     def _find_provider(self, vdep, provider_index):
         """Find provider for a virtual spec in the provider index.
-        Raise an exception if there is a conflicting virtual
-        dependency already in this spec.
+           Raise an exception if there is a conflicting virtual
+           dependency already in this spec.
         """
         assert(vdep.virtual)
         providers = provider_index.providers_for(vdep)
@@ -1018,17 +1104,14 @@ class Spec(object):
         """
         changed = False
 
-        # If it's a virtual dependency, try to find a provider and
-        # merge that.
+        # If it's a virtual dependency, try to find an existing
+        # provider in the spec, and merge that.
         if dep.virtual:
             visited.add(dep.name)
             provider = self._find_provider(dep, provider_index)
             if provider:
                 dep = provider
-
         else:
-            # if it's a real dependency, check whether it provides
-            # something already required in the spec.
             index = ProviderIndex([dep], restrict=True)
             for vspec in (v for v in spec_deps.values() if v.virtual):
                 if index.providers_for(vspec):
@@ -1125,13 +1208,14 @@ class Spec(object):
         # Get all the dependencies into one DependencyMap
         spec_deps = self.flat_dependencies(copy=False)
 
-        # Initialize index of virtual dependency providers
-        index = ProviderIndex(spec_deps.values(), restrict=True)
+        # Initialize index of virtual dependency providers if
+        # concretize didn't pass us one already
+        provider_index = ProviderIndex(spec_deps.values(), restrict=True)
 
         # traverse the package DAG and fill out dependencies according
         # to package files & their 'when' specs
         visited = set()
-        any_change = self._normalize_helper(visited, spec_deps, index)
+        any_change = self._normalize_helper(visited, spec_deps, provider_index)
 
         # If there are deps specified but not visited, they're not
         # actually deps of this package.  Raise an error.
@@ -1410,13 +1494,12 @@ class Spec(object):
                Whether deps should be copied too.  Set to false to copy a
                spec but not its dependencies.
         """
-
         # We don't count dependencies as changes here
         changed = True
         if hasattr(self, 'name'):
-            changed = (self.name != other.name and self.versions != other.versions and \
-                       self.architecture != other.architecture and self.compiler != other.compiler and \
-                       self.variants != other.variants and self._normal != other._normal and \
+            changed = (self.name != other.name and self.versions != other.versions and
+                       self.architecture != other.architecture and self.compiler != other.compiler and
+                       self.variants != other.variants and self._normal != other._normal and
                        self.concrete != other.concrete and self.external != other.external)
 
         # Local node attributes get copied first.
-- 
cgit v1.2.3-70-g09d2


From f9923452b3365e1472a4f1bd63712c57fea0ee0d Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Mon, 14 Mar 2016 14:35:48 +0100
Subject: environment : added machinery to collect modifications to the
 environment and apply them later

---
 lib/spack/spack/environment.py      | 157 ++++++++++++++++++++++++++++++++++++
 lib/spack/spack/test/__init__.py    |   3 +-
 lib/spack/spack/test/environment.py |  50 ++++++++++++
 3 files changed, 209 insertions(+), 1 deletion(-)
 create mode 100644 lib/spack/spack/environment.py
 create mode 100644 lib/spack/spack/test/environment.py

(limited to 'lib')

diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
new file mode 100644
index 0000000000..7d3d7af0de
--- /dev/null
+++ b/lib/spack/spack/environment.py
@@ -0,0 +1,157 @@
+import os
+import os.path
+import collections
+
+
+class SetEnv(object):
+    def __init__(self, name, value, **kwargs):
+        self.name = name
+        self.value = value
+        for key, value in kwargs.items():
+            setattr(self, key, value)
+
+    def execute(self):
+        os.environ[self.name] = str(self.value)
+
+
+class UnsetEnv(object):
+    def __init__(self, name, **kwargs):
+        self.name = name
+        for key, value in kwargs.items():
+            setattr(self, key, value)
+
+    def execute(self):
+        os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
+
+
+class AppendPath(object):
+    def __init__(self, name, path, **kwargs):
+        self.name = name
+        self.path = path
+        for key, value in kwargs.items():
+            setattr(self, key, value)
+
+    def execute(self):
+        environment_value = os.environ.get(self.name, '')
+        directories = environment_value.split(':') if environment_value else []
+        # TODO : Check if this is a valid directory name
+        directories.append(os.path.normpath(self.path))
+        os.environ[self.name] = ':'.join(directories)
+
+
+class PrependPath(object):
+    def __init__(self, name, path, **kwargs):
+        self.name = name
+        self.path = path
+        for key, value in kwargs.items():
+            setattr(self, key, value)
+
+    def execute(self):
+        environment_value = os.environ.get(self.name, '')
+        directories = environment_value.split(':') if environment_value else []
+        # TODO : Check if this is a valid directory name
+        directories = [os.path.normpath(self.path)] + directories
+        os.environ[self.name] = ':'.join(directories)
+
+
+class RemovePath(object):
+    def __init__(self, name, path, **kwargs):
+        self.name = name
+        self.path = path
+        for key, value in kwargs.items():
+            setattr(self, key, value)
+
+    def execute(self):
+        environment_value = os.environ.get(self.name, '')
+        directories = environment_value.split(':') if environment_value else []
+        directories = [os.path.normpath(x) for x in directories if x != os.path.normpath(self.path)]
+        os.environ[self.name] = ':'.join(directories)
+
+
+class EnvironmentModifications(object):
+    """
+    Keeps track of requests to modify the current environment
+    """
+
+    def __init__(self):
+        self.env_modifications = []
+
+    def __iter__(self):
+        return iter(self.env_modifications)
+
+    def set_env(self, name, value, **kwargs):
+        """
+        Stores in the current object a request to set an environment variable
+
+        Args:
+            name: name of the environment variable to be set
+            value: value of the environment variable
+        """
+        item = SetEnv(name, value, **kwargs)
+        self.env_modifications.append(item)
+
+    def unset_env(self, name, **kwargs):
+        """
+        Stores in the current object a request to unset an environment variable
+
+        Args:
+            name: name of the environment variable to be set
+        """
+        item = UnsetEnv(name, **kwargs)
+        self.env_modifications.append(item)
+
+    def append_path(self, name, path, **kwargs):
+        """
+        Stores in the current object a request to append a path to a path list
+
+        Args:
+            name: name of the path list in the environment
+            path: path to be appended
+        """
+        item = AppendPath(name, path, **kwargs)
+        self.env_modifications.append(item)
+
+    def prepend_path(self, name, path, **kwargs):
+        """
+        Same as `append_path`, but the path is pre-pended
+
+        Args:
+            name: name of the path list in the environment
+            path: path to be pre-pended
+        """
+        item = PrependPath(name, path, **kwargs)
+        self.env_modifications.append(item)
+
+    def remove_path(self, name, path, **kwargs):
+        """
+        Stores in the current object a request to remove a path from a path list
+
+        Args:
+            name: name of the path list in the environment
+            path: path to be removed
+        """
+        item = RemovePath(name, path, **kwargs)
+        self.env_modifications.append(item)
+
+
+def validate_environment_modifications(env):
+    modifications = collections.defaultdict(list)
+    for item in env:
+        modifications[item.name].append(item)
+    return modifications
+
+
+def apply_environment_modifications(env):
+    """
+    Modifies the current environment according to the request in env
+
+    Args:
+        env: object storing modifications to the environment
+    """
+    modifications = validate_environment_modifications(env)
+
+    # Cycle over the environment variables that will be modified
+    for variable, actions in modifications.items():
+        # Execute all the actions in the order they were issued
+        for x in actions:
+            x.execute()
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index d5d8b64765..cd842561e6 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -66,7 +66,8 @@ test_names = ['versions',
               'database',
               'namespace_trie',
               'yaml',
-              'sbang']
+              'sbang',
+              'environment']
 
 
 def list_tests():
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
new file mode 100644
index 0000000000..dff3863d32
--- /dev/null
+++ b/lib/spack/spack/test/environment.py
@@ -0,0 +1,50 @@
+import unittest
+import os
+from spack.environment import EnvironmentModifications, apply_environment_modifications
+
+
+class EnvironmentTest(unittest.TestCase):
+    def setUp(self):
+        os.environ.clear()
+        os.environ['UNSET_ME'] = 'foo'
+        os.environ['EMPTY_PATH_LIST'] = ''
+        os.environ['PATH_LIST'] = '/path/second:/path/third'
+        os.environ['REMOVE_PATH_LIST'] = '/a/b:/duplicate:/a/c:/remove/this:/a/d:/duplicate/:/f/g'
+
+    def test_set_env(self):
+        env = EnvironmentModifications()
+        env.set_env('A', 'dummy value')
+        env.set_env('B', 3)
+        apply_environment_modifications(env)
+        self.assertEqual('dummy value', os.environ['A'])
+        self.assertEqual(str(3), os.environ['B'])
+
+    def test_unset_env(self):
+        env = EnvironmentModifications()
+        self.assertEqual('foo', os.environ['UNSET_ME'])
+        env.unset_env('UNSET_ME')
+        apply_environment_modifications(env)
+        self.assertRaises(KeyError, os.environ.__getitem__, 'UNSET_ME')
+
+    def test_path_manipulation(self):
+        env = EnvironmentModifications()
+
+        env.append_path('PATH_LIST', '/path/last')
+        env.prepend_path('PATH_LIST', '/path/first')
+
+        env.append_path('EMPTY_PATH_LIST', '/path/middle')
+        env.append_path('EMPTY_PATH_LIST', '/path/last')
+        env.prepend_path('EMPTY_PATH_LIST', '/path/first')
+
+        env.append_path('NEWLY_CREATED_PATH_LIST', '/path/middle')
+        env.append_path('NEWLY_CREATED_PATH_LIST', '/path/last')
+        env.prepend_path('NEWLY_CREATED_PATH_LIST', '/path/first')
+
+        env.remove_path('REMOVE_PATH_LIST', '/remove/this')
+        env.remove_path('REMOVE_PATH_LIST', '/duplicate/')
+
+        apply_environment_modifications(env)
+        self.assertEqual('/path/first:/path/second:/path/third:/path/last', os.environ['PATH_LIST'])
+        self.assertEqual('/path/first:/path/middle:/path/last', os.environ['EMPTY_PATH_LIST'])
+        self.assertEqual('/path/first:/path/middle:/path/last', os.environ['NEWLY_CREATED_PATH_LIST'])
+        self.assertEqual('/a/b:/a/c:/a/d:/f/g', os.environ['REMOVE_PATH_LIST'])
-- 
cgit v1.2.3-70-g09d2


From f20247ae55362424d1d5543840a21142017661af Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 15 Mar 2016 10:08:54 +0100
Subject: environment : refactoreded set_compiler_environment_variables

---
 lib/spack/spack/build_environment.py | 38 ++++++++++++++++++++----------------
 lib/spack/spack/environment.py       | 17 +++++++++++++++-
 lib/spack/spack/test/database.py     |  1 +
 lib/spack/spack/test/environment.py  |  9 +++++++++
 4 files changed, 47 insertions(+), 18 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 392ba7ea4d..e91a8a1997 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -35,6 +35,7 @@ import sys
 
 import spack
 from llnl.util.filesystem import *
+from spack.environment import EnvironmentModifications, apply_environment_modifications
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -83,30 +84,32 @@ class MakeExecutable(Executable):
 
 
 def set_compiler_environment_variables(pkg):
-    assert(pkg.spec.concrete)
-    compiler = pkg.compiler
-
+    assert pkg.spec.concrete
     # Set compiler variables used by CMake and autotools
-    assert all(key in pkg.compiler.link_paths
-               for key in ('cc', 'cxx', 'f77', 'fc'))
+    assert all(key in pkg.compiler.link_paths for key in ('cc', 'cxx', 'f77', 'fc'))
 
+    # Populate an object with the list of environment modifications
+    # and return it
+    # TODO : add additional kwargs for better diagnostics, like requestor, ttyout, ttyerr, etc.
+    env = EnvironmentModifications()
     link_dir = spack.build_env_path
-    os.environ['CC']  = join_path(link_dir, pkg.compiler.link_paths['cc'])
-    os.environ['CXX'] = join_path(link_dir, pkg.compiler.link_paths['cxx'])
-    os.environ['F77'] = join_path(link_dir, pkg.compiler.link_paths['f77'])
-    os.environ['FC']  = join_path(link_dir, pkg.compiler.link_paths['fc'])
-
+    env.set_env('CC', join_path(link_dir, pkg.compiler.link_paths['cc']))
+    env.set_env('CXX', join_path(link_dir, pkg.compiler.link_paths['cxx']))
+    env.set_env('F77', join_path(link_dir, pkg.compiler.link_paths['f77']))
+    env.set_env('FC', join_path(link_dir, pkg.compiler.link_paths['fc']))
     # Set SPACK compiler variables so that our wrapper knows what to call
+    compiler = pkg.compiler
     if compiler.cc:
-        os.environ['SPACK_CC']  = compiler.cc
+        env.set_env('SPACK_CC', compiler.cc)
     if compiler.cxx:
-        os.environ['SPACK_CXX'] = compiler.cxx
+        env.set_env('SPACK_CXX', compiler.cxx)
     if compiler.f77:
-        os.environ['SPACK_F77'] = compiler.f77
+        env.set_env('SPACK_F77', compiler.f77)
     if compiler.fc:
-        os.environ['SPACK_FC']  = compiler.fc
+        env.set_env('SPACK_FC', compiler.fc)
 
-    os.environ['SPACK_COMPILER_SPEC'] = str(pkg.spec.compiler)
+    env.set_env('SPACK_COMPILER_SPEC', str(pkg.spec.compiler))
+    return env
 
 
 def set_build_environment_variables(pkg):
@@ -264,9 +267,10 @@ def parent_class_modules(cls):
 
 def setup_package(pkg):
     """Execute all environment setup routines."""
-    set_compiler_environment_variables(pkg)
+    env = EnvironmentModifications()
+    env.extend(set_compiler_environment_variables(pkg))
+    apply_environment_modifications(env)
     set_build_environment_variables(pkg)
-
     # If a user makes their own package repo, e.g.
     # spack.repos.mystuff.libelf.Libelf, and they inherit from
     # an existing class like spack.repos.original.libelf.Libelf,
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 7d3d7af0de..3b73f1c7a2 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -73,12 +73,24 @@ class EnvironmentModifications(object):
     Keeps track of requests to modify the current environment
     """
 
-    def __init__(self):
+    def __init__(self, other=None):
         self.env_modifications = []
+        if other is not None:
+            self._check_other(other)
+            self.env_modifications.extend(other.env_modifications)
 
     def __iter__(self):
         return iter(self.env_modifications)
 
+    def extend(self, other):
+        self._check_other(other)
+        self.env_modifications.extend(other.env_modifications)
+
+    @staticmethod
+    def _check_other(other):
+        if not isinstance(other, EnvironmentModifications):
+            raise TypeError('other must be an instance of EnvironmentModifications')
+
     def set_env(self, name, value, **kwargs):
         """
         Stores in the current object a request to set an environment variable
@@ -138,6 +150,9 @@ def validate_environment_modifications(env):
     modifications = collections.defaultdict(list)
     for item in env:
         modifications[item.name].append(item)
+    # TODO : once we organized the modifications into a dictionary that maps an environment variable
+    # TODO : to a list of action to be done on it, we may easily spot inconsistencies and warn the user if
+    # TODO : something suspicious is happening
     return modifications
 
 
diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py
index 9a57e1f03e..ce6e8a0552 100644
--- a/lib/spack/spack/test/database.py
+++ b/lib/spack/spack/test/database.py
@@ -26,6 +26,7 @@
 These tests check the database is functioning properly,
 both in memory and in its file
 """
+import os.path
 import multiprocessing
 import shutil
 import tempfile
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index dff3863d32..17061c8fd0 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -48,3 +48,12 @@ class EnvironmentTest(unittest.TestCase):
         self.assertEqual('/path/first:/path/middle:/path/last', os.environ['EMPTY_PATH_LIST'])
         self.assertEqual('/path/first:/path/middle:/path/last', os.environ['NEWLY_CREATED_PATH_LIST'])
         self.assertEqual('/a/b:/a/c:/a/d:/f/g', os.environ['REMOVE_PATH_LIST'])
+
+    def test_extra_arguments(self):
+        env = EnvironmentModifications()
+        env.set_env('A', 'dummy value', who='Pkg1')
+        apply_environment_modifications(env)
+        self.assertEqual('dummy value', os.environ['A'])
+
+    def test_copy(self):
+        pass
-- 
cgit v1.2.3-70-g09d2


From bcea1df01ce145aea5e49eaaceb7a063b44ddf80 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 15 Mar 2016 10:49:33 +0100
Subject: environment : refactoreded set_build_environment_variables

---
 lib/spack/spack/build_environment.py               | 56 +++++++++++-----------
 lib/spack/spack/environment.py                     |  3 ++
 .../repos/builtin/packages/mvapich2/package.py     |  2 +-
 3 files changed, 33 insertions(+), 28 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index e91a8a1997..770e191ac9 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -35,7 +35,7 @@ import sys
 
 import spack
 from llnl.util.filesystem import *
-from spack.environment import EnvironmentModifications, apply_environment_modifications
+from spack.environment import EnvironmentModifications, apply_environment_modifications, concatenate_paths
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -127,44 +127,45 @@ def set_build_environment_variables(pkg):
     # handled by putting one in the <build_env_path>/case-insensitive
     # directory.  Add that to the path too.
     env_paths = []
-    def add_env_path(path):
-        env_paths.append(path)
-        ci = join_path(path, 'case-insensitive')
-        if os.path.isdir(ci): env_paths.append(ci)
-    add_env_path(spack.build_env_path)
-    add_env_path(join_path(spack.build_env_path, pkg.compiler.name))
-
-    path_put_first("PATH", env_paths)
-    path_set(SPACK_ENV_PATH, env_paths)
-
-    # Prefixes of all of the package's dependencies go in
-    # SPACK_DEPENDENCIES
+    for item in [spack.build_env_path, join_path(spack.build_env_path, pkg.compiler.name)]:
+        env_paths.append(item)
+        ci = join_path(item, 'case-insensitive')
+        if os.path.isdir(ci):
+            env_paths.append(ci)
+
+    env = EnvironmentModifications()
+    for item in reversed(env_paths):
+        env.prepend_path('PATH', item)
+    env.set_env(SPACK_ENV_PATH, concatenate_paths(env_paths))
+
+    # Prefixes of all of the package's dependencies go in SPACK_DEPENDENCIES
     dep_prefixes = [d.prefix for d in pkg.spec.traverse(root=False)]
-    path_set(SPACK_DEPENDENCIES, dep_prefixes)
+    env.set_env(SPACK_DEPENDENCIES, concatenate_paths(dep_prefixes))
+    env.set_env('CMAKE_PREFIX_PATH', concatenate_paths(dep_prefixes))  # Add dependencies to CMAKE_PREFIX_PATH
 
     # Install prefix
-    os.environ[SPACK_PREFIX] = pkg.prefix
+    env.set_env(SPACK_PREFIX, pkg.prefix)
 
     # Install root prefix
-    os.environ[SPACK_INSTALL] = spack.install_path
+    env.set_env(SPACK_INSTALL, spack.install_path)
 
     # Remove these vars from the environment during build because they
     # can affect how some packages find libraries.  We want to make
     # sure that builds never pull in unintended external dependencies.
-    pop_keys(os.environ, "LD_LIBRARY_PATH", "LD_RUN_PATH", "DYLD_LIBRARY_PATH")
+    env.unset_env('LD_LIBRARY_PATH')
+    env.unset_env('LD_RUN_PATH')
+    env.unset_env('DYLD_LIBRARY_PATH')
 
     # Add bin directories from dependencies to the PATH for the build.
-    bin_dirs = ['%s/bin' % prefix for prefix in dep_prefixes]
-    path_put_first('PATH', [bin for bin in bin_dirs if os.path.isdir(bin)])
+    bin_dirs = reversed(filter(os.path.isdir, ['%s/bin' % prefix for prefix in dep_prefixes]))
+    for item in bin_dirs:
+        env.prepend_path('PATH', item)
 
     # Working directory for the spack command itself, for debug logs.
     if spack.debug:
-        os.environ[SPACK_DEBUG] = "TRUE"
-    os.environ[SPACK_SHORT_SPEC] = pkg.spec.short_spec
-    os.environ[SPACK_DEBUG_LOG_DIR] = spack.spack_working_dir
-
-    # Add dependencies to CMAKE_PREFIX_PATH
-    path_set("CMAKE_PREFIX_PATH", dep_prefixes)
+        env.set_env(SPACK_DEBUG, 'TRUE')
+    env.set_env(SPACK_SHORT_SPEC, pkg.spec.short_spec)
+    env.set_env(SPACK_DEBUG_LOG_DIR, spack.spack_working_dir)
 
     # Add any pkgconfig directories to PKG_CONFIG_PATH
     pkg_config_dirs = []
@@ -173,8 +174,9 @@ def set_build_environment_variables(pkg):
             pcdir = join_path(p, libdir, 'pkgconfig')
             if os.path.isdir(pcdir):
                 pkg_config_dirs.append(pcdir)
-    path_set("PKG_CONFIG_PATH", pkg_config_dirs)
+    env.set_env('PKG_CONFIG_PATH', concatenate_paths(pkg_config_dirs))
 
+    return env
 
 def set_module_variables_for_package(pkg, m):
     """Populate the module scope of install() with some useful functions.
@@ -269,8 +271,8 @@ def setup_package(pkg):
     """Execute all environment setup routines."""
     env = EnvironmentModifications()
     env.extend(set_compiler_environment_variables(pkg))
+    env.extend(set_build_environment_variables(pkg))
     apply_environment_modifications(env)
-    set_build_environment_variables(pkg)
     # If a user makes their own package repo, e.g.
     # spack.repos.mystuff.libelf.Libelf, and they inherit from
     # an existing class like spack.repos.original.libelf.Libelf,
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 3b73f1c7a2..18fef1ef12 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -146,6 +146,9 @@ class EnvironmentModifications(object):
         self.env_modifications.append(item)
 
 
+def concatenate_paths(paths):
+    return ':'.join(str(item) for item in paths)
+
 def validate_environment_modifications(env):
     modifications = collections.defaultdict(list)
     for item in env:
diff --git a/var/spack/repos/builtin/packages/mvapich2/package.py b/var/spack/repos/builtin/packages/mvapich2/package.py
index af5ed1b088..e4e95f92af 100644
--- a/var/spack/repos/builtin/packages/mvapich2/package.py
+++ b/var/spack/repos/builtin/packages/mvapich2/package.py
@@ -123,7 +123,7 @@ class Mvapich2(Package):
                 count += 1
         if count > 1:
             raise RuntimeError('network variants are mutually exclusive (only one can be selected at a time)')
-
+        network_options = []
         # From here on I can suppose that only one variant has been selected
         if self.enabled(Mvapich2.PSM) in spec:
             network_options = ["--with-device=ch3:psm"]
-- 
cgit v1.2.3-70-g09d2


From c85888eb5763d453af7b7255b6d8c3461082362f Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 15 Mar 2016 13:36:41 +0100
Subject: package : added `environment_modifications`

---
 lib/spack/spack/build_environment.py               |  7 ++++---
 lib/spack/spack/package.py                         |  4 ++++
 var/spack/repos/builtin/packages/mpich/package.py  | 16 ++++++++++------
 .../builtin/packages/netlib-scalapack/package.py   |  3 +--
 .../repos/builtin/packages/openmpi/package.py      | 14 ++++++++------
 var/spack/repos/builtin/packages/python/package.py | 20 +++++++++++---------
 var/spack/repos/builtin/packages/qt/package.py     | 12 +++++++-----
 var/spack/repos/builtin/packages/ruby/package.py   | 22 +++++++++++++---------
 8 files changed, 58 insertions(+), 40 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 770e191ac9..d321a0e495 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -178,6 +178,7 @@ def set_build_environment_variables(pkg):
 
     return env
 
+
 def set_module_variables_for_package(pkg, m):
     """Populate the module scope of install() with some useful functions.
        This makes things easier for package writers.
@@ -272,7 +273,6 @@ def setup_package(pkg):
     env = EnvironmentModifications()
     env.extend(set_compiler_environment_variables(pkg))
     env.extend(set_build_environment_variables(pkg))
-    apply_environment_modifications(env)
     # If a user makes their own package repo, e.g.
     # spack.repos.mystuff.libelf.Libelf, and they inherit from
     # an existing class like spack.repos.original.libelf.Libelf,
@@ -284,8 +284,9 @@ def setup_package(pkg):
 
     # Allow dependencies to set up environment as well.
     for dep_spec in pkg.spec.traverse(root=False):
-        dep_spec.package.setup_dependent_environment(
-            pkg.module, dep_spec, pkg.spec)
+        env.extend(dep_spec.package.environment_modifications(pkg.module, dep_spec, pkg.spec))
+        dep_spec.package.setup_dependent_environment(pkg.module, dep_spec, pkg.spec)
+    apply_environment_modifications(env)
 
 
 def fork(pkg, function):
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 696adaf896..224d7f8f4b 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -63,6 +63,7 @@ import spack.build_environment
 import spack.url
 import spack.util.web
 import spack.fetch_strategy as fs
+from spack.environment import EnvironmentModifications
 from spack.version import *
 from spack.stage import Stage, ResourceStage, StageComposite
 from spack.util.compression import allowed_archive, extension
@@ -983,6 +984,9 @@ class Package(object):
                           fromlist=[self.__class__.__name__])
 
 
+    def environment_modifications(self, module, spec, dependent_spec):
+        return EnvironmentModifications()
+
     def setup_dependent_environment(self, module, spec, dependent_spec):
         """Called before the install() method of dependents.
 
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index e2b3654c19..c85e8febf3 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -46,14 +46,18 @@ class Mpich(Package):
     provides('mpi@:3.0', when='@3:')
     provides('mpi@:1.3', when='@1:')
 
+    def environment_modifications(self, module, spec, dependent_spec):
+        env = super(Mpich, self).environment_modifications(module, spec, dependent_spec)
+        env.set_env('MPICH_CC', os.environ['CC'])
+        env.set_env('MPICH_CXX', os.environ['CXX'])
+        env.set_env('MPICH_F77', os.environ['F77'])
+        env.set_env('MPICH_F90', os.environ['FC'])
+        env.set_env('MPICH_FC', os.environ['FC'])
+        return env
+
     def setup_dependent_environment(self, module, spec, dep_spec):
         """For dependencies, make mpicc's use spack wrapper."""
-        os.environ['MPICH_CC']  = os.environ['CC']
-        os.environ['MPICH_CXX'] = os.environ['CXX']
-        os.environ['MPICH_F77'] = os.environ['F77']
-        os.environ['MPICH_F90'] = os.environ['FC']
-        os.environ['MPICH_FC'] = os.environ['FC']
-
+        # FIXME : is this necessary ? Shouldn't this be part of a contract with MPI providers?
         module.mpicc = join_path(self.prefix.bin, 'mpicc')
 
     def install(self, spec, prefix):
diff --git a/var/spack/repos/builtin/packages/netlib-scalapack/package.py b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
index 22d538560e..6dbf367475 100644
--- a/var/spack/repos/builtin/packages/netlib-scalapack/package.py
+++ b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
@@ -46,5 +46,4 @@ class NetlibScalapack(Package):
 
         spec['scalapack'].fc_link = '-L%s -lscalapack' % spec['scalapack'].prefix.lib
         spec['scalapack'].cc_link = spec['scalapack'].fc_link
-        spec['scalapack'].libraries = [join_path(spec['scalapack'].prefix.lib,
-                                                 'libscalapack%s' % lib_suffix)]
+        spec['scalapack'].libraries = [join_path(spec['scalapack'].prefix.lib, 'libscalapack%s' % lib_suffix)]
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index e4484af8c5..83a3fe7a4f 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -41,12 +41,14 @@ class Openmpi(Package):
     def url_for_version(self, version):
         return "http://www.open-mpi.org/software/ompi/v%s/downloads/openmpi-%s.tar.bz2" % (version.up_to(2), version)
 
-    def setup_dependent_environment(self, module, spec, dep_spec):
-        """For dependencies, make mpicc's use spack wrapper."""
-        os.environ['OMPI_CC'] = 'cc'
-        os.environ['OMPI_CXX'] = 'c++'
-        os.environ['OMPI_FC'] = 'f90'
-        os.environ['OMPI_F77'] = 'f77'
+    def environment_modifications(self, module, spec, dependent_spec):
+        env = super(Openmpi, self).environment_modifications(module, spec, dependent_spec)
+        # FIXME : the compilers should point to the current wrappers, not to generic cc etc.
+        env.set_env('OMPI_CC', 'cc')
+        env.set_env('OMPI_CXX', 'c++')
+        env.set_env('OMPI_FC', 'f90')
+        env.set_env('OMPI_F77', 'f77')
+        return env
 
     def install(self, spec, prefix):
         config_args = ["--prefix=%s" % prefix,
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index dd240d1ea0..39ced0a120 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -90,6 +90,17 @@ class Python(Package):
         return os.path.join(self.python_lib_dir, 'site-packages')
 
 
+    def environment_modifications(self, module, spec, dependent_spec):
+        env = super(Python, self).environment_modifications(module, spec, dependent_spec)
+        # Set PYTHONPATH to include site-packages dir for the
+        # extension and any other python extensions it depends on.
+        python_paths = []
+        for d in ext_spec.traverse():
+            if d.package.extends(self.spec):
+                python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
+        env.set_env['PYTHONPATH'] = ':'.join(python_paths)
+
+
     def setup_dependent_environment(self, module, spec, ext_spec):
         """Called before python modules' install() methods.
 
@@ -111,15 +122,6 @@ class Python(Package):
         # Make the site packages directory if it does not exist already.
         mkdirp(module.site_packages_dir)
 
-        # Set PYTHONPATH to include site-packages dir for the
-        # extension and any other python extensions it depends on.
-        python_paths = []
-        for d in ext_spec.traverse():
-            if d.package.extends(self.spec):
-                python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
-        os.environ['PYTHONPATH'] = ':'.join(python_paths)
-
-
     # ========================================================================
     # Handle specifics of activating and deactivating python modules.
     # ========================================================================
diff --git a/var/spack/repos/builtin/packages/qt/package.py b/var/spack/repos/builtin/packages/qt/package.py
index 91afa420c1..373623cd95 100644
--- a/var/spack/repos/builtin/packages/qt/package.py
+++ b/var/spack/repos/builtin/packages/qt/package.py
@@ -52,11 +52,13 @@ class Qt(Package):
     depends_on("mesa", when='@4:+mesa')
     depends_on("libxcb")
 
-
-    def setup_dependent_environment(self, module, spec, dep_spec):
-        """Dependencies of Qt find it using the QTDIR environment variable."""
-        os.environ['QTDIR'] = self.prefix
-
+    def environment_modifications(self, module, spec, dep_spec):
+        """
+        Dependencies of Qt find it using the QTDIR environment variable
+        """
+        env = super(Qt, self).environment_modifications(module, spec, dep_spec)
+        env.set_env['QTDIR'] = self.prefix
+        return env
 
     def patch(self):
         if self.spec.satisfies('@4'):
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 6b6242362c..9ec0afa268 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -15,10 +15,21 @@ class Ruby(Package):
 
     def install(self, spec, prefix):
         configure("--prefix=%s" % prefix)
-
         make()
         make("install")
 
+    def environment_modifications(self, module, spec, ext_spec):
+        env = super(Ruby, self).environment_modifications(module, spec, ext_spec)
+        # Set GEM_PATH to include dependent gem directories
+        ruby_paths = []
+        for d in ext_spec.traverse():
+            if d.package.extends(self.spec):
+                ruby_paths.append(d.prefix)
+        env.set_env('GEM_PATH', concatenate_paths(ruby_paths))
+        # The actual installation path for this gem
+        env.set_env('GEM_HOME', ext_spec.prefix)
+        return env
+
     def setup_dependent_environment(self, module, spec, ext_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME
         and GEM_PATH to values appropriate for the package being built.
@@ -31,11 +42,4 @@ class Ruby(Package):
         module.ruby = Executable(join_path(spec.prefix.bin, 'ruby'))
         module.gem = Executable(join_path(spec.prefix.bin, 'gem'))
 
-        # Set GEM_PATH to include dependent gem directories
-        ruby_paths = []
-        for d in ext_spec.traverse():
-            if d.package.extends(self.spec):
-                ruby_paths.append(d.prefix)
-        os.environ['GEM_PATH'] = ':'.join(ruby_paths)
-        # The actual installation path for this gem
-        os.environ['GEM_HOME'] = ext_spec.prefix
+
-- 
cgit v1.2.3-70-g09d2


From 572cb93bf8131d222d2d08bca13fd9de6fded1f4 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 15 Mar 2016 14:05:30 +0100
Subject: package : renamed `setup_dependent_environment` to
 `module_modifications`

---
 lib/spack/spack/build_environment.py                        |  4 ++--
 lib/spack/spack/package.py                                  |  4 ++--
 var/spack/repos/builtin/packages/mpich/package.py           |  6 +++---
 .../repos/builtin/packages/netlib-scalapack/package.py      |  2 +-
 var/spack/repos/builtin/packages/openmpi/package.py         |  4 ++--
 var/spack/repos/builtin/packages/python/package.py          | 10 ++++------
 var/spack/repos/builtin/packages/qt/package.py              |  7 ++-----
 var/spack/repos/builtin/packages/ruby/package.py            | 13 ++++++-------
 8 files changed, 22 insertions(+), 28 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index d321a0e495..e86a7c413a 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -284,8 +284,8 @@ def setup_package(pkg):
 
     # Allow dependencies to set up environment as well.
     for dep_spec in pkg.spec.traverse(root=False):
-        env.extend(dep_spec.package.environment_modifications(pkg.module, dep_spec, pkg.spec))
-        dep_spec.package.setup_dependent_environment(pkg.module, dep_spec, pkg.spec)
+        dep_spec.package.module_modifications(pkg.module, dep_spec, pkg.spec)
+        env.extend(dep_spec.package.environment_modifications(pkg.spec))
     apply_environment_modifications(env)
 
 
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 224d7f8f4b..c1a5c912be 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -984,10 +984,10 @@ class Package(object):
                           fromlist=[self.__class__.__name__])
 
 
-    def environment_modifications(self, module, spec, dependent_spec):
+    def environment_modifications(self, dependent_spec):
         return EnvironmentModifications()
 
-    def setup_dependent_environment(self, module, spec, dependent_spec):
+    def module_modifications(self, module, spec, dependent_spec):
         """Called before the install() method of dependents.
 
         Default implementation does nothing, but this can be
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index c85e8febf3..d298981c92 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -46,8 +46,8 @@ class Mpich(Package):
     provides('mpi@:3.0', when='@3:')
     provides('mpi@:1.3', when='@1:')
 
-    def environment_modifications(self, module, spec, dependent_spec):
-        env = super(Mpich, self).environment_modifications(module, spec, dependent_spec)
+    def environment_modifications(self, dependent_spec):
+        env = super(Mpich, self).environment_modifications(dependent_spec)
         env.set_env('MPICH_CC', os.environ['CC'])
         env.set_env('MPICH_CXX', os.environ['CXX'])
         env.set_env('MPICH_F77', os.environ['F77'])
@@ -55,7 +55,7 @@ class Mpich(Package):
         env.set_env('MPICH_FC', os.environ['FC'])
         return env
 
-    def setup_dependent_environment(self, module, spec, dep_spec):
+    def module_modifications(self, module, spec, dep_spec):
         """For dependencies, make mpicc's use spack wrapper."""
         # FIXME : is this necessary ? Shouldn't this be part of a contract with MPI providers?
         module.mpicc = join_path(self.prefix.bin, 'mpicc')
diff --git a/var/spack/repos/builtin/packages/netlib-scalapack/package.py b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
index 6dbf367475..ecdea46442 100644
--- a/var/spack/repos/builtin/packages/netlib-scalapack/package.py
+++ b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
@@ -40,7 +40,7 @@ class NetlibScalapack(Package):
             make()
             make("install")
 
-    def setup_dependent_environment(self, module, spec, dependent_spec):
+    def module_modifications(self, module, spec, dependent_spec):
         # TODO treat OS that are not Linux...
         lib_suffix = '.so' if '+shared' in spec['scalapack'] else '.a'
 
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index 83a3fe7a4f..3a14170457 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -41,8 +41,8 @@ class Openmpi(Package):
     def url_for_version(self, version):
         return "http://www.open-mpi.org/software/ompi/v%s/downloads/openmpi-%s.tar.bz2" % (version.up_to(2), version)
 
-    def environment_modifications(self, module, spec, dependent_spec):
-        env = super(Openmpi, self).environment_modifications(module, spec, dependent_spec)
+    def environment_modifications(self, dependent_spec):
+        env = super(Openmpi, self).environment_modifications(dependent_spec)
         # FIXME : the compilers should point to the current wrappers, not to generic cc etc.
         env.set_env('OMPI_CC', 'cc')
         env.set_env('OMPI_CXX', 'c++')
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 39ced0a120..307cec726b 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -89,19 +89,17 @@ class Python(Package):
     def site_packages_dir(self):
         return os.path.join(self.python_lib_dir, 'site-packages')
 
-
-    def environment_modifications(self, module, spec, dependent_spec):
-        env = super(Python, self).environment_modifications(module, spec, dependent_spec)
+    def environment_modifications(self, extension_spec):
+        env = super(Python, self).environment_modifications(extension_spec)
         # Set PYTHONPATH to include site-packages dir for the
         # extension and any other python extensions it depends on.
         python_paths = []
-        for d in ext_spec.traverse():
+        for d in extension_spec.traverse():
             if d.package.extends(self.spec):
                 python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
         env.set_env['PYTHONPATH'] = ':'.join(python_paths)
 
-
-    def setup_dependent_environment(self, module, spec, ext_spec):
+    def module_modifications(self, module, spec, ext_spec):
         """Called before python modules' install() methods.
 
         In most cases, extensions will only need to have one line::
diff --git a/var/spack/repos/builtin/packages/qt/package.py b/var/spack/repos/builtin/packages/qt/package.py
index 373623cd95..8391ded3e0 100644
--- a/var/spack/repos/builtin/packages/qt/package.py
+++ b/var/spack/repos/builtin/packages/qt/package.py
@@ -52,11 +52,8 @@ class Qt(Package):
     depends_on("mesa", when='@4:+mesa')
     depends_on("libxcb")
 
-    def environment_modifications(self, module, spec, dep_spec):
-        """
-        Dependencies of Qt find it using the QTDIR environment variable
-        """
-        env = super(Qt, self).environment_modifications(module, spec, dep_spec)
+    def environment_modifications(self, dependent_spec):
+        env = super(Qt, self).environment_modifications(dependent_spec)
         env.set_env['QTDIR'] = self.prefix
         return env
 
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 9ec0afa268..9caea30ef4 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -1,6 +1,5 @@
 from spack import *
-import spack
-import os
+
 
 class Ruby(Package):
     """A dynamic, open source programming language with a focus on 
@@ -18,19 +17,19 @@ class Ruby(Package):
         make()
         make("install")
 
-    def environment_modifications(self, module, spec, ext_spec):
-        env = super(Ruby, self).environment_modifications(module, spec, ext_spec)
+    def environment_modifications(self, extension_spec):
+        env = super(Ruby, self).environment_modifications(extension_spec)
         # Set GEM_PATH to include dependent gem directories
         ruby_paths = []
-        for d in ext_spec.traverse():
+        for d in extension_spec.traverse():
             if d.package.extends(self.spec):
                 ruby_paths.append(d.prefix)
         env.set_env('GEM_PATH', concatenate_paths(ruby_paths))
         # The actual installation path for this gem
-        env.set_env('GEM_HOME', ext_spec.prefix)
+        env.set_env('GEM_HOME', extension_spec.prefix)
         return env
 
-    def setup_dependent_environment(self, module, spec, ext_spec):
+    def module_modifications(self, module, spec, ext_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME
         and GEM_PATH to values appropriate for the package being built.
 
-- 
cgit v1.2.3-70-g09d2


From cc3d9f4eb751fe028db5a074fc7bc52fbe0a962a Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 15 Mar 2016 15:09:35 +0100
Subject: environment : added test, modified docs

---
 lib/spack/spack/environment.py      | 59 ++++++++++++++++++++++++++-----------
 lib/spack/spack/package.py          | 45 +++++++++++++++++-----------
 lib/spack/spack/test/environment.py | 10 +++++--
 lib/spack/spack/util/environment.py |  8 ++---
 4 files changed, 79 insertions(+), 43 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 18fef1ef12..5a68e10c99 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -3,33 +3,40 @@ import os.path
 import collections
 
 
-class SetEnv(object):
+class AttributeHolder(object):
+    """
+    Policy that permits to store any kind of attribute on self. The attributes must be passed as key/value pairs
+    during the initialization of the instance.
+    """
+    def __init__(self, **kwargs):
+        for key, value in kwargs.items():
+            setattr(self, key, value)
+
+
+class SetEnv(AttributeHolder):
     def __init__(self, name, value, **kwargs):
+        super(SetEnv, self).__init__(**kwargs)
         self.name = name
         self.value = value
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         os.environ[self.name] = str(self.value)
 
 
-class UnsetEnv(object):
+class UnsetEnv(AttributeHolder):
     def __init__(self, name, **kwargs):
+        super(UnsetEnv, self).__init__(**kwargs)
         self.name = name
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
 
 
-class AppendPath(object):
+class AppendPath(AttributeHolder):
     def __init__(self, name, path, **kwargs):
+        super(AppendPath, self).__init__(**kwargs)
         self.name = name
         self.path = path
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         environment_value = os.environ.get(self.name, '')
@@ -39,12 +46,11 @@ class AppendPath(object):
         os.environ[self.name] = ':'.join(directories)
 
 
-class PrependPath(object):
+class PrependPath(AttributeHolder):
     def __init__(self, name, path, **kwargs):
+        super(PrependPath, self).__init__(**kwargs)
         self.name = name
         self.path = path
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         environment_value = os.environ.get(self.name, '')
@@ -54,12 +60,11 @@ class PrependPath(object):
         os.environ[self.name] = ':'.join(directories)
 
 
-class RemovePath(object):
+class RemovePath(AttributeHolder):
     def __init__(self, name, path, **kwargs):
+        super(RemovePath, self).__init__(**kwargs)
         self.name = name
         self.path = path
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         environment_value = os.environ.get(self.name, '')
@@ -70,18 +75,26 @@ class RemovePath(object):
 
 class EnvironmentModifications(object):
     """
-    Keeps track of requests to modify the current environment
+    Keeps track of requests to modify the current environment.
     """
 
     def __init__(self, other=None):
+        """
+        Initializes a new instance, copying commands from other if it is not None
+
+        Args:
+            other: another instance of EnvironmentModifications from which (optional)
+        """
         self.env_modifications = []
         if other is not None:
-            self._check_other(other)
-            self.env_modifications.extend(other.env_modifications)
+            self.extend(other)
 
     def __iter__(self):
         return iter(self.env_modifications)
 
+    def __len__(self):
+        return len(self.env_modifications)
+
     def extend(self, other):
         self._check_other(other)
         self.env_modifications.extend(other.env_modifications)
@@ -147,8 +160,18 @@ class EnvironmentModifications(object):
 
 
 def concatenate_paths(paths):
+    """
+    Concatenates an iterable of paths into a column separated string
+
+    Args:
+        paths: iterable of paths
+
+    Returns:
+        column separated string
+    """
     return ':'.join(str(item) for item in paths)
 
+
 def validate_environment_modifications(env):
     modifications = collections.defaultdict(list)
     for item in env:
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index c1a5c912be..5d8258d4cf 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -985,30 +985,41 @@ class Package(object):
 
 
     def environment_modifications(self, dependent_spec):
-        return EnvironmentModifications()
+        """
+        Called before the install() method of dependents.
 
-    def module_modifications(self, module, spec, dependent_spec):
-        """Called before the install() method of dependents.
+        Return the list of environment modifications needed by dependents (or extensions). Default implementation does
+        nothing, but this can be overridden by an extendable package to set up the install environment for its
+        extensions. This is useful if there are some common steps to installing all extensions for a certain package.
 
-        Default implementation does nothing, but this can be
-        overridden by an extendable package to set up the install
-        environment for its extensions.  This is useful if there are
-        some common steps to installing all extensions for a
-        certain package.
+        Example :
 
-        Some examples:
+        1. Installing python modules generally requires `PYTHONPATH` to point to the lib/pythonX.Y/site-packages
+        directory in the module's install prefix.  This could set that variable.
 
-        1. Installing python modules generally requires PYTHONPATH to
-           point to the lib/pythonX.Y/site-packages directory in the
-           module's install prefix.  This could set that variable.
+        2. A lot of Qt extensions need `QTDIR` set.  This can be used to do that.
 
-        2. Extensions often need to invoke the 'python' interpreter
-           from the Python installation being extended.  This routine can
-           put a 'python' Execuable object in the module scope for the
-           extension package to simplify extension installs.
+        Args:
+            dependent_spec: dependent (or extension) of this spec
+
+        Returns:
+            instance of environment modifications
+        """
+        return EnvironmentModifications()
+
+    def module_modifications(self, module, spec, dependent_spec):
+        """
+        Called before the install() method of dependents.
+
+        Default implementation does nothing, but this can be overridden by an extendable package to set up the module of
+        its extensions. This is useful if there are some common steps to installing all extensions for a
+        certain package.
 
-        3. A lot of Qt extensions need QTDIR set.  This can be used to do that.
+        Example :
 
+        1. Extensions often need to invoke the 'python' interpreter from the Python installation being extended.
+        This routine can put a 'python' Executable object in the module scope for the extension package to simplify
+        extension installs.
         """
         pass
 
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index 17061c8fd0..97581ecb76 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -55,5 +55,11 @@ class EnvironmentTest(unittest.TestCase):
         apply_environment_modifications(env)
         self.assertEqual('dummy value', os.environ['A'])
 
-    def test_copy(self):
-        pass
+    def test_extend(self):
+        env = EnvironmentModifications()
+        env.set_env('A', 'dummy value')
+        env.set_env('B', 3)
+        copy_construct = EnvironmentModifications(env)
+        self.assertEqual(len(copy_construct), 2)
+        for x, y in zip(env, copy_construct):
+            self.assertIs(x, y)
diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py
index ae8e5708be..00cda8d508 100644
--- a/lib/spack/spack/util/environment.py
+++ b/lib/spack/spack/util/environment.py
@@ -40,11 +40,13 @@ def env_flag(name):
     return False
 
 
+# FIXME : remove this function ?
 def path_set(var_name, directories):
     path_str = ":".join(str(dir) for dir in directories)
     os.environ[var_name] = path_str
 
 
+# FIXME : remove this function ?
 def path_put_first(var_name, directories):
     """Puts the provided directories first in the path, adding them
        if they're not already there.
@@ -59,12 +61,6 @@ def path_put_first(var_name, directories):
     path_set(var_name, new_path)
 
 
-def pop_keys(dictionary, *keys):
-    for key in keys:
-        if key in dictionary:
-            dictionary.pop(key)
-
-
 def dump_environment(path):
     """Dump the current environment out to a file."""
     with open(path, 'w') as env_file:
-- 
cgit v1.2.3-70-g09d2


From ccced9f290bc213561209db2479bccfb5605366d Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 15 Mar 2016 15:12:39 +0100
Subject: package : optimized imports

---
 lib/spack/spack/package.py | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 5d8258d4cf..042833964a 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -34,41 +34,34 @@ rundown on spack and how it differs from homebrew, look at the
 README.
 """
 import os
-import errno
 import re
-import shutil
-import time
-import itertools
-import subprocess
-import platform as py_platform
-import multiprocessing
-from urlparse import urlparse, urljoin
 import textwrap
-from StringIO import StringIO
+import time
 
 import llnl.util.tty as tty
-from llnl.util.tty.log import log_output
-from llnl.util.link_tree import LinkTree
-from llnl.util.filesystem import *
-from llnl.util.lang import *
-
 import spack
-import spack.error
+import spack.build_environment
 import spack.compilers
-import spack.mirror
-import spack.hooks
 import spack.directives
+import spack.error
+import spack.fetch_strategy as fs
+import spack.hooks
+import spack.mirror
 import spack.repository
-import spack.build_environment
 import spack.url
 import spack.util.web
-import spack.fetch_strategy as fs
+from StringIO import StringIO
+from llnl.util.filesystem import *
+from llnl.util.lang import *
+from llnl.util.link_tree import LinkTree
+from llnl.util.tty.log import log_output
 from spack.environment import EnvironmentModifications
-from spack.version import *
 from spack.stage import Stage, ResourceStage, StageComposite
-from spack.util.compression import allowed_archive, extension
-from spack.util.executable import ProcessError
+from spack.util.compression import allowed_archive
 from spack.util.environment import dump_environment
+from spack.util.executable import ProcessError
+from spack.version import *
+from urlparse import urlparse
 
 """Allowed URL schemes for spack packages."""
 _ALLOWED_URL_SCHEMES = ["http", "https", "ftp", "file", "git"]
-- 
cgit v1.2.3-70-g09d2


From c8cc6f4fc111d5dd2d55295e569a10cd5739ceee Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 15 Mar 2016 16:59:29 +0100
Subject: test : fix for python 2.6

---
 lib/spack/spack/test/environment.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index 97581ecb76..f44282d6f3 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -62,4 +62,4 @@ class EnvironmentTest(unittest.TestCase):
         copy_construct = EnvironmentModifications(env)
         self.assertEqual(len(copy_construct), 2)
         for x, y in zip(env, copy_construct):
-            self.assertIs(x, y)
+            assert x is y
-- 
cgit v1.2.3-70-g09d2


From 15bbd088e6007a9a6df8c4427340a371e5871506 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Tue, 15 Mar 2016 14:38:06 -0700
Subject: Fix #551: version bug in `spack create`

- `spack create` now sets a proper version in generated file, based on
  the filename, even if it can't find any tarballs for the package.
---
 lib/spack/spack/cmd/create.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py
index 4564143f83..f0cd50b8df 100644
--- a/lib/spack/spack/cmd/create.py
+++ b/lib/spack/spack/cmd/create.py
@@ -208,7 +208,7 @@ def find_repository(spec, args):
     return repo
 
 
-def fetch_tarballs(url, name, args):
+def fetch_tarballs(url, name, version):
     """Try to find versions of the supplied archive by scraping the web.
 
     Prompts the user to select how many to download if many are found.
@@ -222,7 +222,7 @@ def fetch_tarballs(url, name, args):
     archives_to_fetch = 1
     if not versions:
         # If the fetch failed for some reason, revert to what the user provided
-        versions = { "version" : url }
+        versions = { version : url }
     elif len(versions) > 1:
         tty.msg("Found %s versions of %s:" % (len(versions), name),
                 *spack.cmd.elide_list(
@@ -256,7 +256,7 @@ def create(parser, args):
     tty.msg("Creating template for package %s" % name)
 
     # Fetch tarballs (prompting user if necessary)
-    versions, urls = fetch_tarballs(url, name, args)
+    versions, urls = fetch_tarballs(url, name, version)
 
     # Try to guess what configure system is used.
     guesser = ConfigureGuesser()
-- 
cgit v1.2.3-70-g09d2


From 7eca383b10f017666c320ffd7deb6cae83b5c26b Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Thu, 10 Mar 2016 01:12:25 -0800
Subject: Add sanity check paths to packages; fix #505

---
 lib/spack/spack/package.py | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 696adaf896..02fb3e5834 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -318,6 +318,17 @@ class Package(object):
     """Most packages are NOT extendable.  Set to True if you want extensions."""
     extendable = False
 
+    """List of prefix-relative file paths. If these do not exist after
+       install, or if they exist but are not files, sanity checks fail.
+    """
+    sanity_check_files = []
+
+    """List of prefix-relative directory paths. If these do not exist
+       after install, or if they exist but are not directories, sanity
+       checks will fail.
+    """
+    sanity_check_dirs = []
+
 
     def __init__(self, spec):
         # this determines how the package should be built.
@@ -909,7 +920,7 @@ class Package(object):
                          raise e
 
                      # Ensure that something was actually installed.
-                     self._sanity_check_install()
+                     self.sanity_check_prefix()
 
                      # Copy provenance into the install directory on success
                      log_install_path = spack.install_layout.build_log_path(self.spec)
@@ -952,7 +963,18 @@ class Package(object):
         spack.hooks.post_install(self)
 
 
-    def _sanity_check_install(self):
+    def sanity_check_prefix(self):
+        """This function checks whether install succeeded."""
+        def check_paths(path_list, filetype, predicate):
+            for path in path_list:
+                abs_path = os.path.join(self.prefix, path)
+                if not predicate(abs_path):
+                    raise InstallError("Install failed for %s. No such %s in prefix: %s"
+                                       % (self.name, filetype, path))
+
+        check_paths(self.sanity_check_files, 'file', os.path.isfile)
+        check_paths(self.sanity_check_dirs, 'directory', os.path.isdir)
+
         installed = set(os.listdir(self.prefix))
         installed.difference_update(spack.install_layout.hidden_file_paths)
         if not installed:
-- 
cgit v1.2.3-70-g09d2


From b45ec3f04e3627dbe3633239560873ae01bf3beb Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 16 Mar 2016 10:55:28 +0100
Subject: environment : simplified modification of the environment

---
 lib/spack/spack/build_environment.py               | 12 +++--
 lib/spack/spack/cmd/module.py                      |  2 +-
 lib/spack/spack/environment.py                     | 58 +++++++++++-----------
 lib/spack/spack/test/environment.py                | 12 +++--
 lib/spack/spack/util/environment.py                |  2 +-
 var/spack/repos/builtin/packages/python/package.py |  1 +
 6 files changed, 47 insertions(+), 40 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index e86a7c413a..e5d256a2e0 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -35,7 +35,7 @@ import sys
 
 import spack
 from llnl.util.filesystem import *
-from spack.environment import EnvironmentModifications, apply_environment_modifications, concatenate_paths
+from spack.environment import EnvironmentModifications, concatenate_paths
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -283,10 +283,12 @@ def setup_package(pkg):
         set_module_variables_for_package(pkg, mod)
 
     # Allow dependencies to set up environment as well.
-    for dep_spec in pkg.spec.traverse(root=False):
-        dep_spec.package.module_modifications(pkg.module, dep_spec, pkg.spec)
-        env.extend(dep_spec.package.environment_modifications(pkg.spec))
-    apply_environment_modifications(env)
+    for dependency_spec in pkg.spec.traverse(root=False):
+        dependency_spec.package.module_modifications(pkg.module, dependency_spec, pkg.spec)
+        env.extend(dependency_spec.package.environment_modifications(pkg.spec))
+    # TODO : implement validation
+    #validate(env)
+    env.apply_modifications()
 
 
 def fork(pkg, function):
diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py
index 1d6867c1d9..315d9fc926 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/module.py
@@ -80,7 +80,7 @@ def module_find(mtype, spec_array):
     if not os.path.isfile(mod.file_name):
         tty.die("No %s module is installed for %s" % (mtype, spec))
 
-    print mod.use_name
+    print(mod.use_name)
 
 
 def module_refresh():
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 5a68e10c99..b557bf0bbc 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -158,41 +158,43 @@ class EnvironmentModifications(object):
         item = RemovePath(name, path, **kwargs)
         self.env_modifications.append(item)
 
+    def group_by_name(self):
+        """
+        Returns a dict of the modifications grouped by variable name
+
+        Returns:
+            dict mapping the environment variable name to the modifications to be done on it
+        """
+        modifications = collections.defaultdict(list)
+        for item in self:
+            modifications[item.name].append(item)
+        return modifications
+
+    def clear(self):
+        """
+        Clears the current list of modifications
+        """
+        self.env_modifications.clear()
+
+    def apply_modifications(self):
+        """
+        Applies the modifications and clears the list
+        """
+        modifications = self.group_by_name()
+        # Apply the modifications to the environment variables one variable at a time
+        for name, actions in sorted(modifications.items()):
+            for x in actions:
+                x.execute()
+
 
 def concatenate_paths(paths):
     """
-    Concatenates an iterable of paths into a column separated string
+    Concatenates an iterable of paths into a  string of column separated paths
 
     Args:
         paths: iterable of paths
 
     Returns:
-        column separated string
+        string
     """
     return ':'.join(str(item) for item in paths)
-
-
-def validate_environment_modifications(env):
-    modifications = collections.defaultdict(list)
-    for item in env:
-        modifications[item.name].append(item)
-    # TODO : once we organized the modifications into a dictionary that maps an environment variable
-    # TODO : to a list of action to be done on it, we may easily spot inconsistencies and warn the user if
-    # TODO : something suspicious is happening
-    return modifications
-
-
-def apply_environment_modifications(env):
-    """
-    Modifies the current environment according to the request in env
-
-    Args:
-        env: object storing modifications to the environment
-    """
-    modifications = validate_environment_modifications(env)
-
-    # Cycle over the environment variables that will be modified
-    for variable, actions in modifications.items():
-        # Execute all the actions in the order they were issued
-        for x in actions:
-            x.execute()
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index f44282d6f3..d0b093b054 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -1,6 +1,6 @@
 import unittest
 import os
-from spack.environment import EnvironmentModifications, apply_environment_modifications
+from spack.environment import EnvironmentModifications
 
 
 class EnvironmentTest(unittest.TestCase):
@@ -15,7 +15,7 @@ class EnvironmentTest(unittest.TestCase):
         env = EnvironmentModifications()
         env.set_env('A', 'dummy value')
         env.set_env('B', 3)
-        apply_environment_modifications(env)
+        env.apply_modifications()
         self.assertEqual('dummy value', os.environ['A'])
         self.assertEqual(str(3), os.environ['B'])
 
@@ -23,7 +23,7 @@ class EnvironmentTest(unittest.TestCase):
         env = EnvironmentModifications()
         self.assertEqual('foo', os.environ['UNSET_ME'])
         env.unset_env('UNSET_ME')
-        apply_environment_modifications(env)
+        env.apply_modifications()
         self.assertRaises(KeyError, os.environ.__getitem__, 'UNSET_ME')
 
     def test_path_manipulation(self):
@@ -43,7 +43,7 @@ class EnvironmentTest(unittest.TestCase):
         env.remove_path('REMOVE_PATH_LIST', '/remove/this')
         env.remove_path('REMOVE_PATH_LIST', '/duplicate/')
 
-        apply_environment_modifications(env)
+        env.apply_modifications()
         self.assertEqual('/path/first:/path/second:/path/third:/path/last', os.environ['PATH_LIST'])
         self.assertEqual('/path/first:/path/middle:/path/last', os.environ['EMPTY_PATH_LIST'])
         self.assertEqual('/path/first:/path/middle:/path/last', os.environ['NEWLY_CREATED_PATH_LIST'])
@@ -52,7 +52,9 @@ class EnvironmentTest(unittest.TestCase):
     def test_extra_arguments(self):
         env = EnvironmentModifications()
         env.set_env('A', 'dummy value', who='Pkg1')
-        apply_environment_modifications(env)
+        for x in env:
+            assert hasattr(x, 'who')
+        env.apply_modifications()
         self.assertEqual('dummy value', os.environ['A'])
 
     def test_extend(self):
diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py
index 00cda8d508..1485992b0f 100644
--- a/lib/spack/spack/util/environment.py
+++ b/lib/spack/spack/util/environment.py
@@ -64,5 +64,5 @@ def path_put_first(var_name, directories):
 def dump_environment(path):
     """Dump the current environment out to a file."""
     with open(path, 'w') as env_file:
-        for key,val in sorted(os.environ.items()):
+        for key, val in sorted(os.environ.items()):
             env_file.write("%s=%s\n" % (key, val))
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 307cec726b..2f9948d451 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -98,6 +98,7 @@ class Python(Package):
             if d.package.extends(self.spec):
                 python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
         env.set_env['PYTHONPATH'] = ':'.join(python_paths)
+        return env
 
     def module_modifications(self, module, spec, ext_spec):
         """Called before python modules' install() methods.
-- 
cgit v1.2.3-70-g09d2


From 597727f8bedc894330dfd26eab1a82859980f2f1 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 16 Mar 2016 15:19:13 +0100
Subject: tclmodules : added hooks to process EnvironmentModifications objects

---
 lib/spack/spack/modules.py                         | 151 +++++++++++++++------
 var/spack/repos/builtin/packages/mpich/package.py  |  25 +++-
 var/spack/repos/builtin/packages/python/package.py |  15 +-
 3 files changed, 137 insertions(+), 54 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index c27043db8c..1a0a0fd4d6 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -47,18 +47,18 @@ of module file.
 __all__ = ['EnvModule', 'Dotkit', 'TclModule']
 
 import os
+import os.path
 import re
-import textwrap
 import shutil
+import textwrap
 from glob import glob
 
 import llnl.util.tty as tty
-from llnl.util.filesystem import join_path, mkdirp
-
 import spack
+from spack.environment import *
+from llnl.util.filesystem import join_path, mkdirp
 
-"""Registry of all types of modules.  Entries created by EnvModule's
-   metaclass."""
+# Registry of all types of modules.  Entries created by EnvModule's metaclass
 module_types = {}
 
 
@@ -79,6 +79,32 @@ def print_help():
             "")
 
 
+class PathInspector(object):
+    dirname2varname = {
+        'bin': ('PATH',),
+        'man': ('MANPATH',),
+        'lib': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
+        'lib64': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
+        'include': ('CPATH',),
+        'pkgconfig': ('PKG_CONFIG_PATH',)
+    }
+
+    def __call__(self, env, directory, names):
+        for name in names:
+            variables = PathInspector.dirname2varname.get(name, None)
+            if variables is None:
+                continue
+            absolute_path = join_path(os.path.abspath(directory), name)
+            for variable in variables:
+                env.prepend_path(variable, absolute_path)
+
+
+def inspect_path(path):
+    env, inspector = EnvironmentModifications(), PathInspector()
+    os.path.walk(path, inspector, env)
+    return env
+
+
 class EnvModule(object):
     name = 'env_module'
 
@@ -88,21 +114,27 @@ class EnvModule(object):
             if cls.name != 'env_module':
                 module_types[cls.name] = cls
 
-
     def __init__(self, spec=None):
         # category in the modules system
         # TODO: come up with smarter category names.
         self.category = "spack"
 
-        # Descriptions for the module system's UI
-        self.short_description = ""
-        self.long_description = ""
-
         # dict pathname -> list of directories to be prepended to in
         # the module file.
         self._paths = None
         self.spec = spec
+        self.pkg = spec.package  # Just stored for convenience
+
+        # short description default is just the package + version
+        # packages can provide this optional attribute
+        self.short_description = spec.format("$_ $@")
+        if hasattr(self.pkg, 'short_description'):
+            self.short_description = self.pkg.short_description
 
+        # long description is the docstring with reduced whitespace.
+        self.long_description = None
+        if self.spec.package.__doc__:
+            self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
 
     @property
     def paths(self):
@@ -130,26 +162,19 @@ class EnvModule(object):
                     add_path(var, directory)
 
             # Add python path unless it's an actual python installation
-            # TODO: is there a better way to do this?
+            # TODO : is there a better way to do this?
+            # FIXME : add PYTHONPATH to every python package
             if self.spec.name != 'python':
                 site_packages = glob(join_path(self.spec.prefix.lib, "python*/site-packages"))
                 if site_packages:
                     add_path('PYTHONPATH', site_packages[0])
 
+            # FIXME : Same for GEM_PATH
             if self.spec.package.extends(spack.spec.Spec('ruby')):
-              add_path('GEM_PATH', self.spec.prefix)
-
-            # short description is just the package + version
-            # TODO: maybe packages can optionally provide it.
-            self.short_description = self.spec.format("$_ $@")
-
-            # long description is the docstring with reduced whitespace.
-            if self.spec.package.__doc__:
-                self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
+                add_path('GEM_PATH', self.spec.prefix)
 
         return self._paths
 
-
     def write(self):
         """Write out a module file for this object."""
         module_dir = os.path.dirname(self.file_name)
@@ -160,9 +185,18 @@ class EnvModule(object):
         if not self.paths:
             return
 
-        with open(self.file_name, 'w') as f:
-            self._write(f)
+        # Construct the changes that needs to be done on the environment for
+        env = inspect_path(self.spec.prefix)
+        # FIXME : move the logic to inspection
+        env.prepend_path('CMAKE_PREFIX_PATH', self.spec.prefix)
+        # FIXME : decide how to distinguish between calls done in the installation and elsewhere
+        env.extend(self.spec.package.environment_modifications(None))
+        # site_specific = ...`
+        if not env:
+            return
 
+        with open(self.file_name, 'w') as f:
+            self._write(f, env)
 
     def _write(self, stream):
         """To be implemented by subclasses."""
@@ -175,14 +209,12 @@ class EnvModule(object):
            where this module lives."""
         raise NotImplementedError()
 
-
     @property
     def use_name(self):
         """Subclasses should implement this to return the name the
            module command uses to refer to the package."""
         raise NotImplementedError()
 
-
     def remove(self):
         mod_file = self.file_name
         if os.path.exists(mod_file):
@@ -205,7 +237,7 @@ class Dotkit(EnvModule):
                                  self.spec.compiler.version, 
                                  self.spec.dag_hash())
 
-    def _write(self, dk_file):
+    def _write(self, dk_file, env):
         # Category
         if self.category:
             dk_file.write('#c %s\n' % self.category)
@@ -231,6 +263,10 @@ class Dotkit(EnvModule):
 class TclModule(EnvModule):
     name = 'tcl'
     path = join_path(spack.share_path, "modules")
+    formats = {
+        PrependPath: 'prepend-path {0.name} \"{0.path}\"\n',
+        SetEnv: 'setenv {0.name} \"{0.value}\"\n'
+    }
 
     @property
     def file_name(self):
@@ -244,25 +280,56 @@ class TclModule(EnvModule):
                                  self.spec.compiler.version, 
                                  self.spec.dag_hash())
 
-
-    def _write(self, m_file):
-        # TODO: cateogry?
-        m_file.write('#%Module1.0\n')
-
+    def process_environment_command(self, env):
+        for command in env:
+            # FIXME : how should we handle errors here?
+            yield self.formats[type(command)].format(command)
+
+    def _write(self, module_file, env):
+        """
+        Writes a TCL module file for this package
+
+        Args:
+            module_file: module file stream
+            env: list of environment modifications to be written in the module file
+        """
+        # TCL Modulefile header
+        module_file.write('#%Module1.0\n')
+        # TODO : category ?
         # Short description
         if self.short_description:
-            m_file.write('module-whatis \"%s\"\n\n' % self.short_description)
+            module_file.write('module-whatis \"%s\"\n\n' % self.short_description)
 
         # Long description
         if self.long_description:
-            m_file.write('proc ModulesHelp { } {\n')
+            module_file.write('proc ModulesHelp { } {\n')
             doc = re.sub(r'"', '\"', self.long_description)
-            m_file.write("puts stderr \"%s\"\n" % doc)
-            m_file.write('}\n\n')
-
-        # Path alterations
-        for var, dirs in self.paths.items():
-            for directory in dirs:
-                m_file.write("prepend-path %s \"%s\"\n" % (var, directory))
-
-        m_file.write("prepend-path CMAKE_PREFIX_PATH \"%s\"\n" % self.spec.prefix)
+            module_file.write("puts stderr \"%s\"\n" % doc)
+            module_file.write('}\n\n')
+
+        # Environment modifications
+        for line in self.process_environment_command(env):
+            module_file.write(line)
+
+    # FIXME : REMOVE
+    # def _write(self, m_file):
+    #     # TODO: cateogry?
+    #     m_file.write('#%Module1.0\n')
+    #
+    #     # Short description
+    #     if self.short_description:
+    #         m_file.write('module-whatis \"%s\"\n\n' % self.short_description)
+    #
+    #     # Long description
+    #     if self.long_description:
+    #         m_file.write('proc ModulesHelp { } {\n')
+    #         doc = re.sub(r'"', '\"', self.long_description)
+    #         m_file.write("puts stderr \"%s\"\n" % doc)
+    #         m_file.write('}\n\n')
+    #
+    #     # Path alterations
+    #     for var, dirs in self.paths.items():
+    #         for directory in dirs:
+    #             m_file.write("prepend-path %s \"%s\"\n" % (var, directory))
+    #
+    #     m_file.write("prepend-path CMAKE_PREFIX_PATH \"%s\"\n" % self.spec.prefix)
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index d298981c92..4c34d0308f 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -25,6 +25,7 @@
 from spack import *
 import os
 
+
 class Mpich(Package):
     """MPICH is a high performance and widely portable implementation of
        the Message Passing Interface (MPI) standard."""
@@ -48,11 +49,25 @@ class Mpich(Package):
 
     def environment_modifications(self, dependent_spec):
         env = super(Mpich, self).environment_modifications(dependent_spec)
-        env.set_env('MPICH_CC', os.environ['CC'])
-        env.set_env('MPICH_CXX', os.environ['CXX'])
-        env.set_env('MPICH_F77', os.environ['F77'])
-        env.set_env('MPICH_F90', os.environ['FC'])
-        env.set_env('MPICH_FC', os.environ['FC'])
+
+        if dependent_spec is None:
+            # We are not using compiler wrappers
+            cc = self.compiler.cc
+            cxx = self.compiler.cxx
+            f77 = self.compiler.f77
+            f90 = fc = self.compiler.fc
+        else:
+            # Spack compiler wrappers
+            cc = os.environ['CC']
+            cxx = os.environ['CXX']
+            f77 = os.environ['F77']
+            f90 = fc = os.environ['FC']
+
+        env.set_env('MPICH_CC', cc)
+        env.set_env('MPICH_CXX', cxx)
+        env.set_env('MPICH_F77', f77)
+        env.set_env('MPICH_F90', f90)
+        env.set_env('MPICH_FC', fc)
         return env
 
     def module_modifications(self, module, spec, dep_spec):
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 2f9948d451..acb3651726 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -91,13 +91,14 @@ class Python(Package):
 
     def environment_modifications(self, extension_spec):
         env = super(Python, self).environment_modifications(extension_spec)
-        # Set PYTHONPATH to include site-packages dir for the
-        # extension and any other python extensions it depends on.
-        python_paths = []
-        for d in extension_spec.traverse():
-            if d.package.extends(self.spec):
-                python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
-        env.set_env['PYTHONPATH'] = ':'.join(python_paths)
+        if extension_spec is not None:
+            # Set PYTHONPATH to include site-packages dir for the
+            # extension and any other python extensions it depends on.
+            python_paths = []
+            for d in extension_spec.traverse():
+                if d.package.extends(self.spec):
+                    python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
+            env.set_env['PYTHONPATH'] = ':'.join(python_paths)
         return env
 
     def module_modifications(self, module, spec, ext_spec):
-- 
cgit v1.2.3-70-g09d2


From ac762e95a6213e1514f29d3d6501e4a95dd3e1d4 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 16 Mar 2016 16:23:02 +0100
Subject: modules : removed dead code

---
 lib/spack/spack/modules.py | 205 +++++++++++++++------------------------------
 1 file changed, 66 insertions(+), 139 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 1a0a0fd4d6..b64a8b3226 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -22,14 +22,12 @@
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-"""This module contains code for creating environment modules, which
-can include dotkits, tcl modules, lmod, and others.
+"""
+This module contains code for creating environment modules, which can include dotkits, tcl modules, lmod, and others.
 
-The various types of modules are installed by post-install hooks and
-removed after an uninstall by post-uninstall hooks.  This class
-consolidates the logic for creating an abstract description of the
-information that module systems need.  Currently that includes a
-number of directories to be appended to paths in the user's environment:
+The various types of modules are installed by post-install hooks and removed after an uninstall by post-uninstall hooks.
+This class consolidates the logic for creating an abstract description of the information that module systems need.
+Currently that includes a number of directories to be appended to paths in the user's environment:
 
   * /bin directories to be appended to PATH
   * /lib* directories for LD_LIBRARY_PATH
@@ -37,26 +35,23 @@ number of directories to be appended to paths in the user's environment:
   * /man* and /share/man* directories for MANPATH
   * the package prefix for CMAKE_PREFIX_PATH
 
-This module also includes logic for coming up with unique names for
-the module files so that they can be found by the various
-shell-support files in $SPACK/share/spack/setup-env.*.
+This module also includes logic for coming up with unique names for the module files so that they can be found by the
+various shell-support files in $SPACK/share/spack/setup-env.*.
 
-Each hook in hooks/ implements the logic for writing its specific type
-of module file.
+Each hook in hooks/ implements the logic for writing its specific type of module file.
 """
-__all__ = ['EnvModule', 'Dotkit', 'TclModule']
-
 import os
 import os.path
 import re
 import shutil
 import textwrap
-from glob import glob
 
 import llnl.util.tty as tty
 import spack
-from spack.environment import *
 from llnl.util.filesystem import join_path, mkdirp
+from spack.environment import *
+
+__all__ = ['EnvModule', 'Dotkit', 'TclModule']
 
 # Registry of all types of modules.  Entries created by EnvModule's metaclass
 module_types = {}
@@ -79,34 +74,35 @@ def print_help():
             "")
 
 
-class PathInspector(object):
-    dirname2varname = {
-        'bin': ('PATH',),
-        'man': ('MANPATH',),
-        'lib': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
-        'lib64': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
-        'include': ('CPATH',),
-        'pkgconfig': ('PKG_CONFIG_PATH',)
-    }
-
-    def __call__(self, env, directory, names):
-        for name in names:
-            variables = PathInspector.dirname2varname.get(name, None)
-            if variables is None:
-                continue
-            absolute_path = join_path(os.path.abspath(directory), name)
-            for variable in variables:
-                env.prepend_path(variable, absolute_path)
-
-
 def inspect_path(path):
+    class PathInspector(object):
+        dirname2varname = {
+            'bin': ('PATH',),
+            'man': ('MANPATH',),
+            'lib': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
+            'lib64': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
+            'include': ('CPATH',),
+            'pkgconfig': ('PKG_CONFIG_PATH',)
+        }
+
+        def __call__(self, env, directory, names):
+            for name in names:
+                variables = PathInspector.dirname2varname.get(name, None)
+                if variables is None:
+                    continue
+                absolute_path = join_path(os.path.abspath(directory), name)
+                for variable in variables:
+                    env.prepend_path(variable, absolute_path)
+
     env, inspector = EnvironmentModifications(), PathInspector()
     os.path.walk(path, inspector, env)
+    env.prepend_path('CMAKE_PREFIX_PATH', path)
     return env
 
 
 class EnvModule(object):
     name = 'env_module'
+    formats = {}
 
     class __metaclass__(type):
         def __init__(cls, name, bases, dict):
@@ -119,9 +115,6 @@ class EnvModule(object):
         # TODO: come up with smarter category names.
         self.category = "spack"
 
-        # dict pathname -> list of directories to be prepended to in
-        # the module file.
-        self._paths = None
         self.spec = spec
         self.pkg = spec.package  # Just stored for convenience
 
@@ -136,44 +129,21 @@ class EnvModule(object):
         if self.spec.package.__doc__:
             self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
 
-    @property
-    def paths(self):
-        if self._paths is None:
-            self._paths = {}
-
-            def add_path(path_name, directory):
-                path = self._paths.setdefault(path_name, [])
-                path.append(directory)
-
-            # Add paths if they exist.
-            for var, directory in [
-                    ('PATH', self.spec.prefix.bin),
-                    ('MANPATH', self.spec.prefix.man),
-                    ('MANPATH', self.spec.prefix.share_man),
-                    ('LIBRARY_PATH', self.spec.prefix.lib),
-                    ('LIBRARY_PATH', self.spec.prefix.lib64),
-                    ('LD_LIBRARY_PATH', self.spec.prefix.lib),
-                    ('LD_LIBRARY_PATH', self.spec.prefix.lib64),
-                    ('CPATH', self.spec.prefix.include),
-                    ('PKG_CONFIG_PATH', join_path(self.spec.prefix.lib, 'pkgconfig')),
-                    ('PKG_CONFIG_PATH', join_path(self.spec.prefix.lib64, 'pkgconfig'))]:
-
-                if os.path.isdir(directory):
-                    add_path(var, directory)
-
-            # Add python path unless it's an actual python installation
-            # TODO : is there a better way to do this?
-            # FIXME : add PYTHONPATH to every python package
-            if self.spec.name != 'python':
-                site_packages = glob(join_path(self.spec.prefix.lib, "python*/site-packages"))
-                if site_packages:
-                    add_path('PYTHONPATH', site_packages[0])
-
-            # FIXME : Same for GEM_PATH
-            if self.spec.package.extends(spack.spec.Spec('ruby')):
-                add_path('GEM_PATH', self.spec.prefix)
-
-        return self._paths
+    # @property
+    # def paths(self):
+    #         # Add python path unless it's an actual python installation
+    #         # TODO : is there a better way to do this?
+    #         # FIXME : add PYTHONPATH to every python package
+    #         if self.spec.name != 'python':
+    #             site_packages = glob(join_path(self.spec.prefix.lib, "python*/site-packages"))
+    #             if site_packages:
+    #                 add_path('PYTHONPATH', site_packages[0])
+    #
+    #         # FIXME : Same for GEM_PATH
+    #         if self.spec.package.extends(spack.spec.Spec('ruby')):
+    #             add_path('GEM_PATH', self.spec.prefix)
+    #
+    #     return self._paths
 
     def write(self):
         """Write out a module file for this object."""
@@ -181,14 +151,9 @@ class EnvModule(object):
         if not os.path.exists(module_dir):
             mkdirp(module_dir)
 
-        # If there are no paths, no need for a dotkit.
-        if not self.paths:
-            return
-
-        # Construct the changes that needs to be done on the environment for
+        # Environment modifications guessed by inspecting the installation prefix
         env = inspect_path(self.spec.prefix)
-        # FIXME : move the logic to inspection
-        env.prepend_path('CMAKE_PREFIX_PATH', self.spec.prefix)
+        # Package-specific environment modifications
         # FIXME : decide how to distinguish between calls done in the installation and elsewhere
         env.extend(self.spec.package.environment_modifications(None))
         # site_specific = ...`
@@ -196,12 +161,17 @@ class EnvModule(object):
             return
 
         with open(self.file_name, 'w') as f:
-            self._write(f, env)
+            self.write_header(f)
+            for line in self.process_environment_command(env):
+                f.write(line)
 
-    def _write(self, stream):
-        """To be implemented by subclasses."""
+    def write_header(self, stream):
         raise NotImplementedError()
 
+    def process_environment_command(self, env):
+        for command in env:
+            # FIXME : how should we handle errors here?
+            yield self.formats[type(command)].format(command)
 
     @property
     def file_name(self):
@@ -225,10 +195,14 @@ class Dotkit(EnvModule):
     name = 'dotkit'
     path = join_path(spack.share_path, "dotkit")
 
+    formats = {
+        PrependPath: 'dk_alter {0.name} {0.path}\n',
+        SetEnv: 'dk_setenv {0.name} {0.value}\n'
+    }
+
     @property
     def file_name(self):
-        return join_path(Dotkit.path, self.spec.architecture,
-                         '%s.dk' % self.use_name)
+        return join_path(Dotkit.path, self.spec.architecture, '%s.dk' % self.use_name)
 
     @property
     def use_name(self):
@@ -237,7 +211,7 @@ class Dotkit(EnvModule):
                                  self.spec.compiler.version, 
                                  self.spec.dag_hash())
 
-    def _write(self, dk_file, env):
+    def write_header(self, dk_file):
         # Category
         if self.category:
             dk_file.write('#c %s\n' % self.category)
@@ -251,18 +225,11 @@ class Dotkit(EnvModule):
             for line in textwrap.wrap(self.long_description, 72):
                 dk_file.write("#h %s\n" % line)
 
-        # Path alterations
-        for var, dirs in self.paths.items():
-            for directory in dirs:
-                dk_file.write("dk_alter %s %s\n" % (var, directory))
-
-        # Let CMake find this package.
-        dk_file.write("dk_alter CMAKE_PREFIX_PATH %s\n" % self.spec.prefix)
-
 
 class TclModule(EnvModule):
     name = 'tcl'
     path = join_path(spack.share_path, "modules")
+
     formats = {
         PrependPath: 'prepend-path {0.name} \"{0.path}\"\n',
         SetEnv: 'setenv {0.name} \"{0.value}\"\n'
@@ -272,7 +239,6 @@ class TclModule(EnvModule):
     def file_name(self):
         return join_path(TclModule.path, self.spec.architecture, self.use_name)
 
-
     @property
     def use_name(self):
       return "%s-%s-%s-%s-%s" % (self.spec.name, self.spec.version,
@@ -280,19 +246,7 @@ class TclModule(EnvModule):
                                  self.spec.compiler.version, 
                                  self.spec.dag_hash())
 
-    def process_environment_command(self, env):
-        for command in env:
-            # FIXME : how should we handle errors here?
-            yield self.formats[type(command)].format(command)
-
-    def _write(self, module_file, env):
-        """
-        Writes a TCL module file for this package
-
-        Args:
-            module_file: module file stream
-            env: list of environment modifications to be written in the module file
-        """
+    def write_header(self, module_file):
         # TCL Modulefile header
         module_file.write('#%Module1.0\n')
         # TODO : category ?
@@ -306,30 +260,3 @@ class TclModule(EnvModule):
             doc = re.sub(r'"', '\"', self.long_description)
             module_file.write("puts stderr \"%s\"\n" % doc)
             module_file.write('}\n\n')
-
-        # Environment modifications
-        for line in self.process_environment_command(env):
-            module_file.write(line)
-
-    # FIXME : REMOVE
-    # def _write(self, m_file):
-    #     # TODO: cateogry?
-    #     m_file.write('#%Module1.0\n')
-    #
-    #     # Short description
-    #     if self.short_description:
-    #         m_file.write('module-whatis \"%s\"\n\n' % self.short_description)
-    #
-    #     # Long description
-    #     if self.long_description:
-    #         m_file.write('proc ModulesHelp { } {\n')
-    #         doc = re.sub(r'"', '\"', self.long_description)
-    #         m_file.write("puts stderr \"%s\"\n" % doc)
-    #         m_file.write('}\n\n')
-    #
-    #     # Path alterations
-    #     for var, dirs in self.paths.items():
-    #         for directory in dirs:
-    #             m_file.write("prepend-path %s \"%s\"\n" % (var, directory))
-    #
-    #     m_file.write("prepend-path CMAKE_PREFIX_PATH \"%s\"\n" % self.spec.prefix)
-- 
cgit v1.2.3-70-g09d2


From bb04d5cc63182b6129f9abf78db260a2dab9f506 Mon Sep 17 00:00:00 2001
From: "Adam J. Stewart" <ajstewart426@gmail.com>
Date: Wed, 16 Mar 2016 14:27:22 -0500
Subject: Fix grammar in mirrors documentation

---
 lib/spack/docs/mirrors.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/docs/mirrors.rst b/lib/spack/docs/mirrors.rst
index b20fedb55f..dad04d053b 100644
--- a/lib/spack/docs/mirrors.rst
+++ b/lib/spack/docs/mirrors.rst
@@ -38,7 +38,7 @@ contains tarballs for each package, named after each package.
 
 .. note::
 
-   Archives are **not** named exactly they were in the package's fetch
+   Archives are **not** named exactly the way they were in the package's fetch
    URL.  They have the form ``<name>-<version>.<extension>``, where
    ``<name>`` is Spack's name for the package, ``<version>`` is the
    version of the tarball, and ``<extension>`` is whatever format the
-- 
cgit v1.2.3-70-g09d2


From 9cdd79e33f8463699fcea0b668eafac1c9fae1d4 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 17 Mar 2016 14:14:18 +0100
Subject: modules : restored previous logic for path inspection

---
 lib/spack/spack/modules.py | 54 ++++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 23 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index b64a8b3226..7e395736e4 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -74,29 +74,37 @@ def print_help():
             "")
 
 
-def inspect_path(path):
-    class PathInspector(object):
-        dirname2varname = {
-            'bin': ('PATH',),
-            'man': ('MANPATH',),
-            'lib': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
-            'lib64': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
-            'include': ('CPATH',),
-            'pkgconfig': ('PKG_CONFIG_PATH',)
-        }
-
-        def __call__(self, env, directory, names):
-            for name in names:
-                variables = PathInspector.dirname2varname.get(name, None)
-                if variables is None:
-                    continue
-                absolute_path = join_path(os.path.abspath(directory), name)
-                for variable in variables:
-                    env.prepend_path(variable, absolute_path)
-
-    env, inspector = EnvironmentModifications(), PathInspector()
-    os.path.walk(path, inspector, env)
-    env.prepend_path('CMAKE_PREFIX_PATH', path)
+def inspect_path(prefix):
+    """
+    Inspects the prefix of an installation to search for common layouts. Issues a request to modify the environment
+    accordingly when an item is found.
+
+    Args:
+        prefix: prefix of the installation
+
+    Returns:
+        instance of EnvironmentModifications containing the requested modifications
+    """
+    env = EnvironmentModifications()
+    # Inspect the prefix to check for the existence of common directories
+    prefix_inspections = {
+        'bin': ('PATH',),
+        'man': ('MANPATH',),
+        'lib': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
+        'lib64': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
+        'include': ('CPATH',)
+    }
+    for attribute, variables in prefix_inspections.items():
+        expected = getattr(prefix, attribute)
+        if os.path.isdir(expected):
+            for variable in variables:
+                env.prepend_path(variable, expected)
+    # PKGCONFIG
+    for expected in (join_path(prefix.lib, 'pkgconfig'), join_path(prefix.lib64, 'pkgconfig')):
+        if os.path.isdir(expected):
+            env.prepend_path('PKG_CONFIG_PATH', expected)
+    # CMake related variables
+    env.prepend_path('CMAKE_PREFIX_PATH', prefix)
     return env
 
 
-- 
cgit v1.2.3-70-g09d2


From f0f0663d1b9ece1e2b0b0a8f720b1325eec443bb Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 17 Mar 2016 15:11:39 +0100
Subject: package : split `environment_modifications` into `setup_environment`
 and `setup_dependent_environment`. package : renamed `module_modifications`
 to `modify_module` for consistency

---
 lib/spack/spack/build_environment.py               |  4 +--
 lib/spack/spack/modules.py                         |  5 ++-
 lib/spack/spack/package.py                         |  9 +++--
 var/spack/repos/builtin/packages/mpich/package.py  | 39 +++++++++-------------
 .../builtin/packages/netlib-scalapack/package.py   |  2 +-
 .../repos/builtin/packages/openmpi/package.py      | 19 ++++++-----
 var/spack/repos/builtin/packages/python/package.py | 27 +++++++--------
 var/spack/repos/builtin/packages/qt/package.py     |  4 +--
 var/spack/repos/builtin/packages/ruby/package.py   |  6 ++--
 9 files changed, 52 insertions(+), 63 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index e5d256a2e0..68477145fe 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -284,8 +284,8 @@ def setup_package(pkg):
 
     # Allow dependencies to set up environment as well.
     for dependency_spec in pkg.spec.traverse(root=False):
-        dependency_spec.package.module_modifications(pkg.module, dependency_spec, pkg.spec)
-        env.extend(dependency_spec.package.environment_modifications(pkg.spec))
+        dependency_spec.package.modify_module(pkg.module, dependency_spec, pkg.spec)
+        dependency_spec.package.setup_dependent_environment(env, pkg.spec)
     # TODO : implement validation
     #validate(env)
     env.apply_modifications()
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 7e395736e4..09895d8c44 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -162,9 +162,8 @@ class EnvModule(object):
         # Environment modifications guessed by inspecting the installation prefix
         env = inspect_path(self.spec.prefix)
         # Package-specific environment modifications
-        # FIXME : decide how to distinguish between calls done in the installation and elsewhere
-        env.extend(self.spec.package.environment_modifications(None))
-        # site_specific = ...`
+        self.spec.package.setup_environment(env)
+        # TODO : implement site-specific modifications and filters
         if not env:
             return
 
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 042833964a..8e56dd1f97 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -977,7 +977,7 @@ class Package(object):
                           fromlist=[self.__class__.__name__])
 
 
-    def environment_modifications(self, dependent_spec):
+    def setup_environment(self, env):
         """
         Called before the install() method of dependents.
 
@@ -998,9 +998,12 @@ class Package(object):
         Returns:
             instance of environment modifications
         """
-        return EnvironmentModifications()
+        pass
+
+    def setup_dependent_environment(self, env, dependent_spec):
+        self.setup_environment(env)
 
-    def module_modifications(self, module, spec, dependent_spec):
+    def modify_module(self, module, spec, dependent_spec):
         """
         Called before the install() method of dependents.
 
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index 4c34d0308f..5af9b585ea 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -47,30 +47,21 @@ class Mpich(Package):
     provides('mpi@:3.0', when='@3:')
     provides('mpi@:1.3', when='@1:')
 
-    def environment_modifications(self, dependent_spec):
-        env = super(Mpich, self).environment_modifications(dependent_spec)
-
-        if dependent_spec is None:
-            # We are not using compiler wrappers
-            cc = self.compiler.cc
-            cxx = self.compiler.cxx
-            f77 = self.compiler.f77
-            f90 = fc = self.compiler.fc
-        else:
-            # Spack compiler wrappers
-            cc = os.environ['CC']
-            cxx = os.environ['CXX']
-            f77 = os.environ['F77']
-            f90 = fc = os.environ['FC']
-
-        env.set_env('MPICH_CC', cc)
-        env.set_env('MPICH_CXX', cxx)
-        env.set_env('MPICH_F77', f77)
-        env.set_env('MPICH_F90', f90)
-        env.set_env('MPICH_FC', fc)
-        return env
-
-    def module_modifications(self, module, spec, dep_spec):
+    def setup_environment(self, env):
+        env.set_env('MPICH_CC', self.compiler.cc)
+        env.set_env('MPICH_CXX', self.compiler.cxx)
+        env.set_env('MPICH_F77', self.compiler.f77)
+        env.set_env('MPICH_F90', self.compiler.fc)
+        env.set_env('MPICH_FC', self.compiler.fc)
+
+    def setup_dependent_environment(self, env, dependent_spec):
+        env.set_env('MPICH_CC', spack_cc)
+        env.set_env('MPICH_CXX', spack_cxx)
+        env.set_env('MPICH_F77', spack_f77)
+        env.set_env('MPICH_F90', spack_f90)
+        env.set_env('MPICH_FC', spack_fc)
+
+    def modify_module(self, module, spec, dep_spec):
         """For dependencies, make mpicc's use spack wrapper."""
         # FIXME : is this necessary ? Shouldn't this be part of a contract with MPI providers?
         module.mpicc = join_path(self.prefix.bin, 'mpicc')
diff --git a/var/spack/repos/builtin/packages/netlib-scalapack/package.py b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
index ecdea46442..7e7e5b2e2e 100644
--- a/var/spack/repos/builtin/packages/netlib-scalapack/package.py
+++ b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
@@ -40,7 +40,7 @@ class NetlibScalapack(Package):
             make()
             make("install")
 
-    def module_modifications(self, module, spec, dependent_spec):
+    def modify_module(self, module, spec, dependent_spec):
         # TODO treat OS that are not Linux...
         lib_suffix = '.so' if '+shared' in spec['scalapack'] else '.a'
 
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index 3a14170457..7783ca8766 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -41,14 +41,17 @@ class Openmpi(Package):
     def url_for_version(self, version):
         return "http://www.open-mpi.org/software/ompi/v%s/downloads/openmpi-%s.tar.bz2" % (version.up_to(2), version)
 
-    def environment_modifications(self, dependent_spec):
-        env = super(Openmpi, self).environment_modifications(dependent_spec)
-        # FIXME : the compilers should point to the current wrappers, not to generic cc etc.
-        env.set_env('OMPI_CC', 'cc')
-        env.set_env('OMPI_CXX', 'c++')
-        env.set_env('OMPI_FC', 'f90')
-        env.set_env('OMPI_F77', 'f77')
-        return env
+    def setup_environment(self, env):
+        env.set_env('OMPI_CC', self.compiler.cc)
+        env.set_env('OMPI_CXX', self.compiler.cxx)
+        env.set_env('OMPI_FC', self.compiler.fc)
+        env.set_env('OMPI_F77', self.compiler.f77)
+
+    def setup_dependent_environment(self, env, dependent_spec):
+        env.set_env('OMPI_CC', spack_cc)
+        env.set_env('OMPI_CXX', spack_cxx)
+        env.set_env('OMPI_FC', spack_fc)
+        env.set_env('OMPI_F77', spack_f77)
 
     def install(self, spec, prefix):
         config_args = ["--prefix=%s" % prefix,
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index acb3651726..d47c1d1b2f 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -89,24 +89,21 @@ class Python(Package):
     def site_packages_dir(self):
         return os.path.join(self.python_lib_dir, 'site-packages')
 
-    def environment_modifications(self, extension_spec):
-        env = super(Python, self).environment_modifications(extension_spec)
-        if extension_spec is not None:
-            # Set PYTHONPATH to include site-packages dir for the
-            # extension and any other python extensions it depends on.
-            python_paths = []
-            for d in extension_spec.traverse():
-                if d.package.extends(self.spec):
-                    python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
-            env.set_env['PYTHONPATH'] = ':'.join(python_paths)
-        return env
-
-    def module_modifications(self, module, spec, ext_spec):
-        """Called before python modules' install() methods.
+    def setup_dependent_environment(self, env, extension_spec):
+        # Set PYTHONPATH to include site-packages dir for the extension and any other python extensions it depends on.
+        python_paths = []
+        for d in extension_spec.traverse():
+            if d.package.extends(self.spec):
+                python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
+        env.set_env['PYTHONPATH'] = ':'.join(python_paths)
+
+    def modify_module(self, module, spec, ext_spec):
+        """
+        Called before python modules' install() methods.
 
         In most cases, extensions will only need to have one line::
 
-            python('setup.py', 'install', '--prefix=%s' % prefix)
+        python('setup.py', 'install', '--prefix=%s' % prefix)
         """
         # Python extension builds can have a global python executable function
         if self.version >= Version("3.0.0") and self.version < Version("4.0.0"):
diff --git a/var/spack/repos/builtin/packages/qt/package.py b/var/spack/repos/builtin/packages/qt/package.py
index 0adf352be2..35b9d68462 100644
--- a/var/spack/repos/builtin/packages/qt/package.py
+++ b/var/spack/repos/builtin/packages/qt/package.py
@@ -55,10 +55,8 @@ class Qt(Package):
     depends_on("mesa", when='@4:+mesa')
     depends_on("libxcb")
 
-    def environment_modifications(self, dependent_spec):
-        env = super(Qt, self).environment_modifications(dependent_spec)
+    def setup_environment(self, env):
         env.set_env['QTDIR'] = self.prefix
-        return env
 
     def patch(self):
         if self.spec.satisfies('@4'):
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 9caea30ef4..2d1da8c9af 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -17,8 +17,7 @@ class Ruby(Package):
         make()
         make("install")
 
-    def environment_modifications(self, extension_spec):
-        env = super(Ruby, self).environment_modifications(extension_spec)
+    def setup_dependent_environment(self, env, extension_spec):
         # Set GEM_PATH to include dependent gem directories
         ruby_paths = []
         for d in extension_spec.traverse():
@@ -27,9 +26,8 @@ class Ruby(Package):
         env.set_env('GEM_PATH', concatenate_paths(ruby_paths))
         # The actual installation path for this gem
         env.set_env('GEM_HOME', extension_spec.prefix)
-        return env
 
-    def module_modifications(self, module, spec, ext_spec):
+    def modify_module(self, module, spec, ext_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME
         and GEM_PATH to values appropriate for the package being built.
 
-- 
cgit v1.2.3-70-g09d2


From 3da4d6664bbee38fde6faeffb39f889698ea320c Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 17 Mar 2016 15:38:08 +0100
Subject: environment : simplified hierarchy according to comments in review

---
 lib/spack/spack/environment.py      | 53 ++++++++++++-------------------------
 lib/spack/spack/modules.py          | 10 +++----
 lib/spack/spack/package.py          |  1 +
 lib/spack/spack/test/environment.py |  2 +-
 lib/spack/spack/util/environment.py |  2 --
 5 files changed, 24 insertions(+), 44 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index b557bf0bbc..3ee917fcec 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -3,73 +3,54 @@ import os.path
 import collections
 
 
-class AttributeHolder(object):
-    """
-    Policy that permits to store any kind of attribute on self. The attributes must be passed as key/value pairs
-    during the initialization of the instance.
-    """
-    def __init__(self, **kwargs):
-        for key, value in kwargs.items():
-            setattr(self, key, value)
+class NameModifier(object):
+    def __init__(self, name, **kwargs):
+        self.name = name
+        self.args = {'name': name}
+        self.args.update(kwargs)
 
 
-class SetEnv(AttributeHolder):
+class NameValueModifier(object):
     def __init__(self, name, value, **kwargs):
-        super(SetEnv, self).__init__(**kwargs)
         self.name = name
         self.value = value
+        self.args = {'name': name, 'value': value}
+        self.args.update(kwargs)
 
+
+class SetEnv(NameValueModifier):
     def execute(self):
         os.environ[self.name] = str(self.value)
 
 
-class UnsetEnv(AttributeHolder):
-    def __init__(self, name, **kwargs):
-        super(UnsetEnv, self).__init__(**kwargs)
-        self.name = name
-
+class UnsetEnv(NameModifier):
     def execute(self):
         os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
 
 
-class AppendPath(AttributeHolder):
-    def __init__(self, name, path, **kwargs):
-        super(AppendPath, self).__init__(**kwargs)
-        self.name = name
-        self.path = path
-
+class AppendPath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
         directories = environment_value.split(':') if environment_value else []
         # TODO : Check if this is a valid directory name
-        directories.append(os.path.normpath(self.path))
+        directories.append(os.path.normpath(self.value))
         os.environ[self.name] = ':'.join(directories)
 
 
-class PrependPath(AttributeHolder):
-    def __init__(self, name, path, **kwargs):
-        super(PrependPath, self).__init__(**kwargs)
-        self.name = name
-        self.path = path
-
+class PrependPath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
         directories = environment_value.split(':') if environment_value else []
         # TODO : Check if this is a valid directory name
-        directories = [os.path.normpath(self.path)] + directories
+        directories = [os.path.normpath(self.value)] + directories
         os.environ[self.name] = ':'.join(directories)
 
 
-class RemovePath(AttributeHolder):
-    def __init__(self, name, path, **kwargs):
-        super(RemovePath, self).__init__(**kwargs)
-        self.name = name
-        self.path = path
-
+class RemovePath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
         directories = environment_value.split(':') if environment_value else []
-        directories = [os.path.normpath(x) for x in directories if x != os.path.normpath(self.path)]
+        directories = [os.path.normpath(x) for x in directories if x != os.path.normpath(self.value)]
         os.environ[self.name] = ':'.join(directories)
 
 
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 09895d8c44..0f826c7363 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -178,7 +178,7 @@ class EnvModule(object):
     def process_environment_command(self, env):
         for command in env:
             # FIXME : how should we handle errors here?
-            yield self.formats[type(command)].format(command)
+            yield self.formats[type(command)].format(**command.args)
 
     @property
     def file_name(self):
@@ -203,8 +203,8 @@ class Dotkit(EnvModule):
     path = join_path(spack.share_path, "dotkit")
 
     formats = {
-        PrependPath: 'dk_alter {0.name} {0.path}\n',
-        SetEnv: 'dk_setenv {0.name} {0.value}\n'
+        PrependPath: 'dk_alter {name} {value}\n',
+        SetEnv: 'dk_setenv {name} {value}\n'
     }
 
     @property
@@ -238,8 +238,8 @@ class TclModule(EnvModule):
     path = join_path(spack.share_path, "modules")
 
     formats = {
-        PrependPath: 'prepend-path {0.name} \"{0.path}\"\n',
-        SetEnv: 'setenv {0.name} \"{0.value}\"\n'
+        PrependPath: 'prepend-path {name} \"{value}\"\n',
+        SetEnv: 'setenv {name} \"{value}\"\n'
     }
 
     @property
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 8e56dd1f97..680b26d69a 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -979,6 +979,7 @@ class Package(object):
 
     def setup_environment(self, env):
         """
+
         Called before the install() method of dependents.
 
         Return the list of environment modifications needed by dependents (or extensions). Default implementation does
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index d0b093b054..3e03760c01 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -53,7 +53,7 @@ class EnvironmentTest(unittest.TestCase):
         env = EnvironmentModifications()
         env.set_env('A', 'dummy value', who='Pkg1')
         for x in env:
-            assert hasattr(x, 'who')
+            assert 'who' in x.args
         env.apply_modifications()
         self.assertEqual('dummy value', os.environ['A'])
 
diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py
index 1485992b0f..55e653fd2f 100644
--- a/lib/spack/spack/util/environment.py
+++ b/lib/spack/spack/util/environment.py
@@ -40,13 +40,11 @@ def env_flag(name):
     return False
 
 
-# FIXME : remove this function ?
 def path_set(var_name, directories):
     path_str = ":".join(str(dir) for dir in directories)
     os.environ[var_name] = path_str
 
 
-# FIXME : remove this function ?
 def path_put_first(var_name, directories):
     """Puts the provided directories first in the path, adding them
        if they're not already there.
-- 
cgit v1.2.3-70-g09d2


From 38c3c84969c25aaee8f643ff9770759c7e6d9c35 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 17 Mar 2016 17:37:33 +0100
Subject: environment : added caller information

---
 lib/spack/spack/environment.py | 27 +++++++++++++++++++++++++--
 lib/spack/spack/modules.py     |  8 ++++++--
 2 files changed, 31 insertions(+), 4 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 3ee917fcec..6d214595a3 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -1,6 +1,7 @@
 import os
 import os.path
 import collections
+import inspect
 
 
 class NameModifier(object):
@@ -32,7 +33,6 @@ class AppendPath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
         directories = environment_value.split(':') if environment_value else []
-        # TODO : Check if this is a valid directory name
         directories.append(os.path.normpath(self.value))
         os.environ[self.name] = ':'.join(directories)
 
@@ -41,7 +41,6 @@ class PrependPath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
         directories = environment_value.split(':') if environment_value else []
-        # TODO : Check if this is a valid directory name
         directories = [os.path.normpath(self.value)] + directories
         os.environ[self.name] = ':'.join(directories)
 
@@ -57,6 +56,11 @@ class RemovePath(NameValueModifier):
 class EnvironmentModifications(object):
     """
     Keeps track of requests to modify the current environment.
+
+    Each call to a method to modify the environment stores the extra information on the caller in the request:
+    - 'filename' : filename of the module where the caller is defined
+    - 'lineno': line number where the request occurred
+    - 'context' : line of code that issued the request that failed
     """
 
     def __init__(self, other=None):
@@ -85,6 +89,20 @@ class EnvironmentModifications(object):
         if not isinstance(other, EnvironmentModifications):
             raise TypeError('other must be an instance of EnvironmentModifications')
 
+    def _get_outside_caller_attributes(self):
+        stack = inspect.stack()
+        try:
+            _, filename, lineno, _, context, index = stack[2]
+            context = context[index].strip()
+        except Exception:
+            filename, lineno, context = 'unknown file', 'unknown line', 'unknown context'
+        args = {
+            'filename': filename,
+            'lineno': lineno,
+            'context': context
+        }
+        return args
+
     def set_env(self, name, value, **kwargs):
         """
         Stores in the current object a request to set an environment variable
@@ -93,6 +111,7 @@ class EnvironmentModifications(object):
             name: name of the environment variable to be set
             value: value of the environment variable
         """
+        kwargs.update(self._get_outside_caller_attributes())
         item = SetEnv(name, value, **kwargs)
         self.env_modifications.append(item)
 
@@ -103,6 +122,7 @@ class EnvironmentModifications(object):
         Args:
             name: name of the environment variable to be set
         """
+        kwargs.update(self._get_outside_caller_attributes())
         item = UnsetEnv(name, **kwargs)
         self.env_modifications.append(item)
 
@@ -114,6 +134,7 @@ class EnvironmentModifications(object):
             name: name of the path list in the environment
             path: path to be appended
         """
+        kwargs.update(self._get_outside_caller_attributes())
         item = AppendPath(name, path, **kwargs)
         self.env_modifications.append(item)
 
@@ -125,6 +146,7 @@ class EnvironmentModifications(object):
             name: name of the path list in the environment
             path: path to be pre-pended
         """
+        kwargs.update(self._get_outside_caller_attributes())
         item = PrependPath(name, path, **kwargs)
         self.env_modifications.append(item)
 
@@ -136,6 +158,7 @@ class EnvironmentModifications(object):
             name: name of the path list in the environment
             path: path to be removed
         """
+        kwargs.update(self._get_outside_caller_attributes())
         item = RemovePath(name, path, **kwargs)
         self.env_modifications.append(item)
 
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 0f826c7363..d192bbe004 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -177,8 +177,12 @@ class EnvModule(object):
 
     def process_environment_command(self, env):
         for command in env:
-            # FIXME : how should we handle errors here?
-            yield self.formats[type(command)].format(**command.args)
+            try:
+                yield self.formats[type(command)].format(**command.args)
+            except KeyError:
+                tty.warn('Cannot handle command of type {command} : skipping request'.format(command=type(command)))
+                tty.warn('{context} at {filename}:{lineno}'.format(**command.args))
+
 
     @property
     def file_name(self):
-- 
cgit v1.2.3-70-g09d2


From e673127263a03e63d6a64840d1071a788f0c099e Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 17 Mar 2016 18:11:23 +0100
Subject: package : added documentation

---
 lib/spack/spack/package.py | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 680b26d69a..d0b94dbbeb 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -976,32 +976,41 @@ class Package(object):
         return __import__(self.__class__.__module__,
                           fromlist=[self.__class__.__name__])
 
-
     def setup_environment(self, env):
         """
+        Appends in `env` the list of environment modifications needed to use this package outside of spack.
+
+        Default implementation does nothing, but this can be overridden if the package needs a particular environment.
+
+        Example :
+
+        1. A lot of Qt extensions need `QTDIR` set.  This can be used to do that.
+
+        Args:
+            env: list of environment modifications to be updated
+        """
+        pass
 
+    def setup_dependent_environment(self, env, dependent_spec):
+        """
         Called before the install() method of dependents.
 
-        Return the list of environment modifications needed by dependents (or extensions). Default implementation does
-        nothing, but this can be overridden by an extendable package to set up the install environment for its
-        extensions. This is useful if there are some common steps to installing all extensions for a certain package.
+        Appends in `env` the list of environment modifications needed by dependents (or extensions) during the
+        installation of a package. The default implementation delegates to `setup_environment`, but can be overridden
+        if the modifications to the environment happen to be different from the one needed to use the package outside
+        of spack.
+
+        This is useful if there are some common steps to installing all extensions for a certain package.
 
         Example :
 
         1. Installing python modules generally requires `PYTHONPATH` to point to the lib/pythonX.Y/site-packages
         directory in the module's install prefix.  This could set that variable.
 
-        2. A lot of Qt extensions need `QTDIR` set.  This can be used to do that.
-
         Args:
+            env: list of environment modifications to be updated
             dependent_spec: dependent (or extension) of this spec
-
-        Returns:
-            instance of environment modifications
         """
-        pass
-
-    def setup_dependent_environment(self, env, dependent_spec):
         self.setup_environment(env)
 
     def modify_module(self, module, spec, dependent_spec):
@@ -1020,12 +1029,10 @@ class Package(object):
         """
         pass
 
-
     def install(self, spec, prefix):
         """Package implementations override this with their own build configuration."""
         raise InstallError("Package %s provides no install method!" % self.name)
 
-
     def do_uninstall(self, force=False):
         if not self.installed:
             raise InstallError(str(self.spec) + " is not installed.")
-- 
cgit v1.2.3-70-g09d2


From ac394718ec0aa67c468a8529b930eaade0bcbed1 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 17 Mar 2016 18:22:07 +0100
Subject: python : implemented possible solution

---
 lib/spack/spack/package.py                          | 11 ++++++++++-
 var/spack/repos/builtin/packages/py-nose/package.py |  3 ++-
 2 files changed, 12 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index d0b94dbbeb..a7ab20137e 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -37,6 +37,7 @@ import os
 import re
 import textwrap
 import time
+import glob
 
 import llnl.util.tty as tty
 import spack
@@ -55,7 +56,6 @@ from llnl.util.filesystem import *
 from llnl.util.lang import *
 from llnl.util.link_tree import LinkTree
 from llnl.util.tty.log import log_output
-from spack.environment import EnvironmentModifications
 from spack.stage import Stage, ResourceStage, StageComposite
 from spack.util.compression import allowed_archive
 from spack.util.environment import dump_environment
@@ -1236,6 +1236,15 @@ class Package(object):
         return " ".join("-Wl,-rpath,%s" % p for p in self.rpath)
 
 
+class PythonExtension(Package):
+    def setup_dependent_environment(self, env, dependent_spec):
+        pass
+
+    def setup_environment(self, env):
+        site_packages = glob.glob(join_path(self.spec.prefix.lib, "python*/site-packages"))
+        if site_packages:
+            env.prepend_path('PYTHONPATH', site_packages[0])
+
 def validate_package_url(url_string):
     """Determine whether spack can handle a particular URL or not."""
     url = urlparse(url_string)
diff --git a/var/spack/repos/builtin/packages/py-nose/package.py b/var/spack/repos/builtin/packages/py-nose/package.py
index e7c6cf0264..e817b8eb51 100644
--- a/var/spack/repos/builtin/packages/py-nose/package.py
+++ b/var/spack/repos/builtin/packages/py-nose/package.py
@@ -1,6 +1,7 @@
 from spack import *
+from spack.package import PythonExtension
 
-class PyNose(Package):
+class PyNose(PythonExtension):
     """nose extends the test loading and running features of unittest,
     making it easier to write, find and run tests."""
 
-- 
cgit v1.2.3-70-g09d2


From 90268876f7d740e5b7dcfd079168f248a337791a Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Thu, 17 Mar 2016 18:49:58 -0700
Subject: Change sanity_check_[file|dir] to sanity_check_is_[file|dir], per
 #554

- Add documentation as well.
---
 lib/spack/docs/packaging_guide.rst                 | 84 ++++++++++++++++++----
 lib/spack/spack/package.py                         | 22 +++---
 var/spack/repos/builtin/packages/libelf/package.py |  3 +-
 3 files changed, 84 insertions(+), 25 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index 169899212d..c1077e4497 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -1559,11 +1559,11 @@ you ask for a particular spec.
 A user may have certain preferences for how packages should
 be concretized on their system.  For example, one user may prefer packages
 built with OpenMPI and the Intel compiler.  Another user may prefer
-packages be built with MVAPICH and GCC.  
+packages be built with MVAPICH and GCC.
 
 Spack can be configured to prefer certain compilers, package
 versions, depends_on, and variants during concretization.
-The preferred configuration can be controlled via the 
+The preferred configuration can be controlled via the
 ``~/.spack/packages.yaml`` file for user configuations, or the
 ``etc/spack/packages.yaml`` site configuration.
 
@@ -1582,32 +1582,32 @@ Here's an example packages.yaml file that sets preferred packages:
         compiler: [gcc@4.4.7, gcc@4.6:, intel, clang, pgi]
         providers:
           mpi: [mvapich, mpich, openmpi]
-          
+
 
 At a high level, this example is specifying how packages should be
-concretized.  The dyninst package should prefer using gcc 4.9 and 
+concretized.  The dyninst package should prefer using gcc 4.9 and
 be built with debug options.  The gperftools package should prefer version
 2.2 over 2.4.  Every package on the system should prefer mvapich for
-its MPI and gcc 4.4.7 (except for Dyninst, which overrides this by preferring gcc 4.9).  
-These options are used to fill in implicit defaults.  Any of them can be overwritten 
+its MPI and gcc 4.4.7 (except for Dyninst, which overrides this by preferring gcc 4.9).
+These options are used to fill in implicit defaults.  Any of them can be overwritten
 on the command line if explicitly requested.
 
-Each packages.yaml file begins with the string ``packages:`` and 
+Each packages.yaml file begins with the string ``packages:`` and
 package names are specified on the next level. The special string ``all``
-applies settings to each package. Underneath each package name is 
-one or more components: ``compiler``, ``variants``, ``version``, 
-or ``providers``.  Each component has an ordered list of spec 
+applies settings to each package. Underneath each package name is
+one or more components: ``compiler``, ``variants``, ``version``,
+or ``providers``.  Each component has an ordered list of spec
 ``constraints``, with earlier entries in the list being preferred over
 later entries.
 
-Sometimes a package installation may have constraints that forbid 
+Sometimes a package installation may have constraints that forbid
 the first concretization rule, in which case Spack will use the first
 legal concretization rule.  Going back to the example, if a user
-requests gperftools 2.3 or later, then Spack will install version 2.4 
+requests gperftools 2.3 or later, then Spack will install version 2.4
 as the 2.4 version of gperftools is preferred over 2.3.
 
-An explicit concretization rule in the preferred section will always 
-take preference over unlisted concretizations.  In the above example, 
+An explicit concretization rule in the preferred section will always
+take preference over unlisted concretizations.  In the above example,
 xlc isn't listed in the compiler list.  Every listed compiler from
 gcc to pgi will thus be preferred over the xlc compiler.
 
@@ -2160,6 +2160,62 @@ package, this allows us to avoid race conditions in the library's
 build system.
 
 
+.. _sanity-checks:
+
+Sanity checking an intallation
+--------------------------------
+
+By default, Spack assumes that a build has failed if nothing is
+written to the install prefix, and that it has succeeded if anything
+(a file, a directory, etc.)  is written to the install prefix after
+``install()`` completes.
+
+Consider a simple autotools build like this:
+
+.. code-block:: python
+
+   def install(self, spec, prefix):
+       configure("--prefix=" + prefix)
+       make()
+       make("install")
+
+If you are using using standard autotools or CMake, ``configure`` and
+``make`` will not write anything to the install prefix.  Only ``make
+install`` writes the files, and only once the build is already
+complete.  Not all builds are like this.  Many builds of scientific
+software modify the install prefix *before* ``make install``. Builds
+like this can falsely report that they were successfully installed if
+an error occurs before the install is complete but after files have
+been written to the ``prefix``.
+
+
+``sanity_check_is_file`` and ``sanity_check_is_dir``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+You can optionally specify *sanity checks* to deal with this problem.
+Add properties like this to your package:
+
+.. code-block:: python
+
+   class MyPackage(Package):
+       ...
+
+       sanity_check_is_file = ['include/libelf.h']
+       sanity_check_is_dir  = [lib]
+
+       def install(self, spec, prefix):
+           configure("--prefix=" + prefix)
+           make()
+           make("install")
+
+Now, after ``install()`` runs, Spack will check whether
+``$prefix/include/libelf.h`` exists and is a file, and whether
+``$prefix/lib`` exists and is a directory.  If the checks fail, then
+the build will fail and the install prefix will be removed.  If they
+succeed, Spack considers the build succeeful and keeps the prefix in
+place.
+
+
 .. _file-manipulation:
 
 File manipulation functions
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 02fb3e5834..291056a7b9 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -318,16 +318,17 @@ class Package(object):
     """Most packages are NOT extendable.  Set to True if you want extensions."""
     extendable = False
 
-    """List of prefix-relative file paths. If these do not exist after
-       install, or if they exist but are not files, sanity checks fail.
+    """List of prefix-relative file paths (or a single path). If these do
+       not exist after install, or if they exist but are not files,
+       sanity checks fail.
     """
-    sanity_check_files = []
+    sanity_check_is_file = []
 
-    """List of prefix-relative directory paths. If these do not exist
-       after install, or if they exist but are not directories, sanity
-       checks will fail.
+    """List of prefix-relative directory paths (or a single path). If
+       these do not exist after install, or if they exist but are not
+       directories, sanity checks will fail.
     """
-    sanity_check_dirs = []
+    sanity_check_is_dir = []
 
 
     def __init__(self, spec):
@@ -966,14 +967,17 @@ class Package(object):
     def sanity_check_prefix(self):
         """This function checks whether install succeeded."""
         def check_paths(path_list, filetype, predicate):
+            if isinstance(path_list, basestring):
+                path_list = [path_list]
+
             for path in path_list:
                 abs_path = os.path.join(self.prefix, path)
                 if not predicate(abs_path):
                     raise InstallError("Install failed for %s. No such %s in prefix: %s"
                                        % (self.name, filetype, path))
 
-        check_paths(self.sanity_check_files, 'file', os.path.isfile)
-        check_paths(self.sanity_check_dirs, 'directory', os.path.isdir)
+        check_paths(self.sanity_check_is_file, 'file', os.path.isfile)
+        check_paths(self.sanity_check_is_dir, 'directory', os.path.isdir)
 
         installed = set(os.listdir(self.prefix))
         installed.difference_update(spack.install_layout.hidden_file_paths)
diff --git a/var/spack/repos/builtin/packages/libelf/package.py b/var/spack/repos/builtin/packages/libelf/package.py
index 0fcb56c164..9f16708af5 100644
--- a/var/spack/repos/builtin/packages/libelf/package.py
+++ b/var/spack/repos/builtin/packages/libelf/package.py
@@ -38,8 +38,7 @@ class Libelf(Package):
 
     provides('elf')
 
-    sanity_check_files = ['include/libelf.h']
-    sanity_check_dirs  = ['lib']
+    sanity_check_is_file = 'include/libelf.h'
 
     def install(self, spec, prefix):
         configure("--prefix=" + prefix,
-- 
cgit v1.2.3-70-g09d2


From 076cc764a78c387d0e0cc38047a9df8497416f90 Mon Sep 17 00:00:00 2001
From: Ben Morgan <Ben.Morgan@warwick.ac.uk>
Date: Fri, 18 Mar 2016 11:26:22 +0000
Subject: Add additional suffixes for GCC compiler

---
 lib/spack/spack/compilers/gcc.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/compilers/gcc.py b/lib/spack/spack/compilers/gcc.py
index 495b638a3a..64214db32d 100644
--- a/lib/spack/spack/compilers/gcc.py
+++ b/lib/spack/spack/compilers/gcc.py
@@ -40,7 +40,8 @@ class Gcc(Compiler):
     fc_names = ['gfortran']
 
     # MacPorts builds gcc versions with prefixes and -mp-X.Y suffixes.
-    suffixes = [r'-mp-\d\.\d']
+    # Homebrew and Linuxes may build gcc with -X, -X.Y suffixes
+    suffixes = [r'-mp-\d\.\d', r'-\d\.\d', r'-\d']
 
     # Named wrapper links within spack.build_env_path
     link_paths = {'cc'  : 'gcc/gcc',
-- 
cgit v1.2.3-70-g09d2


From ec8cc2b52839007f0825815c8cfdee8f9a8629a6 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 18 Mar 2016 14:40:53 +0100
Subject: PYTHONPATH : python patches methods for its extensions

---
 lib/spack/spack/build_environment.py               |  4 ++-
 lib/spack/spack/cmd/uninstall.py                   | 12 ++++----
 lib/spack/spack/directory_layout.py                |  2 +-
 lib/spack/spack/modules.py                         | 12 ++++++--
 lib/spack/spack/package.py                         | 10 -------
 .../repos/builtin/packages/py-nose/package.py      |  6 ++--
 var/spack/repos/builtin/packages/python/package.py | 34 ++++++++++++++++++++--
 7 files changed, 53 insertions(+), 27 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 68477145fe..c51fa58477 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -282,9 +282,11 @@ def setup_package(pkg):
     for mod in modules:
         set_module_variables_for_package(pkg, mod)
 
-    # Allow dependencies to set up environment as well.
+    # Allow dependencies to modify the module
     for dependency_spec in pkg.spec.traverse(root=False):
         dependency_spec.package.modify_module(pkg.module, dependency_spec, pkg.spec)
+    # Allow dependencies to set up environment as well
+    for dependency_spec in pkg.spec.traverse(root=False):
         dependency_spec.package.setup_dependent_environment(env, pkg.spec)
     # TODO : implement validation
     #validate(env)
diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index d01aa2136b..2fab8f6f2a 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/lib/spack/spack/cmd/uninstall.py
@@ -79,7 +79,7 @@ def uninstall(parser, args):
                 try:
                     # should work if package is known to spack
                     pkgs.append(s.package)
-                except spack.repository.UnknownPackageError, e:
+                except spack.repository.UnknownPackageError as e:
                     # The package.py file has gone away -- but still
                     # want to uninstall.
                     spack.Package(s).do_uninstall(force=True)
@@ -94,11 +94,11 @@ def uninstall(parser, args):
         for pkg in pkgs:
             try:
                 pkg.do_uninstall(force=args.force)
-            except PackageStillNeededError, e:
+            except PackageStillNeededError as e:
                 tty.error("Will not uninstall %s" % e.spec.format("$_$@$%@$#", color=True))
-                print
-                print "The following packages depend on it:"
+                print()
+                print("The following packages depend on it:")
                 display_specs(e.dependents, long=True)
-                print
-                print "You can use spack uninstall -f to force this action."
+                print()
+                print("You can use spack uninstall -f to force this action.")
                 sys.exit(1)
diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py
index 39ee4e203d..da8f1aa1bc 100644
--- a/lib/spack/spack/directory_layout.py
+++ b/lib/spack/spack/directory_layout.py
@@ -150,7 +150,7 @@ class DirectoryLayout(object):
         if os.path.exists(path):
             try:
                 shutil.rmtree(path)
-            except exceptions.OSError, e:
+            except exceptions.OSError as e:
                 raise RemoveFailedError(spec, path, e)
 
         path = os.path.dirname(path)
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index d192bbe004..7303844229 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -119,13 +119,13 @@ class EnvModule(object):
                 module_types[cls.name] = cls
 
     def __init__(self, spec=None):
+        self.spec = spec
+        self.pkg = spec.package  # Just stored for convenience
+
         # category in the modules system
         # TODO: come up with smarter category names.
         self.category = "spack"
 
-        self.spec = spec
-        self.pkg = spec.package  # Just stored for convenience
-
         # short description default is just the package + version
         # packages can provide this optional attribute
         self.short_description = spec.format("$_ $@")
@@ -161,6 +161,12 @@ class EnvModule(object):
 
         # Environment modifications guessed by inspecting the installation prefix
         env = inspect_path(self.spec.prefix)
+
+        # Let the extendee modify their extensions before asking for package-specific modifications
+        for extendee in self.pkg.extendees:
+            extendee_spec = self.spec[extendee]
+            extendee_spec.package.modify_module(self.pkg, extendee_spec, self.spec)
+
         # Package-specific environment modifications
         self.spec.package.setup_environment(env)
         # TODO : implement site-specific modifications and filters
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 185e3ad2ee..72c84ec624 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1261,16 +1261,6 @@ class Package(object):
         """Get the rpath args as a string, with -Wl,-rpath, for each element."""
         return " ".join("-Wl,-rpath,%s" % p for p in self.rpath)
 
-
-class PythonExtension(Package):
-    def setup_dependent_environment(self, env, dependent_spec):
-        pass
-
-    def setup_environment(self, env):
-        site_packages = glob.glob(join_path(self.spec.prefix.lib, "python*/site-packages"))
-        if site_packages:
-            env.prepend_path('PYTHONPATH', site_packages[0])
-
 def validate_package_url(url_string):
     """Determine whether spack can handle a particular URL or not."""
     url = urlparse(url_string)
diff --git a/var/spack/repos/builtin/packages/py-nose/package.py b/var/spack/repos/builtin/packages/py-nose/package.py
index e817b8eb51..4fee99098e 100644
--- a/var/spack/repos/builtin/packages/py-nose/package.py
+++ b/var/spack/repos/builtin/packages/py-nose/package.py
@@ -1,12 +1,12 @@
 from spack import *
-from spack.package import PythonExtension
 
-class PyNose(PythonExtension):
+
+class PyNose(Package):
     """nose extends the test loading and running features of unittest,
     making it easier to write, find and run tests."""
 
     homepage = "https://pypi.python.org/pypi/nose"
-    url      = "https://pypi.python.org/packages/source/n/nose/nose-1.3.4.tar.gz"
+    url = "https://pypi.python.org/packages/source/n/nose/nose-1.3.4.tar.gz"
 
     version('1.3.4', '6ed7169887580ddc9a8e16048d38274d')
     version('1.3.6', '0ca546d81ca8309080fc80cb389e7a16')
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index d46e1068b6..c445d26369 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -1,11 +1,14 @@
+import functools
+import glob
+import inspect
 import os
 import re
 from contextlib import closing
-from llnl.util.lang import match_predicate
-from spack.util.environment import *
 
-from spack import *
 import spack
+from llnl.util.lang import match_predicate
+from spack import *
+from spack.util.environment import *
 
 
 class Python(Package):
@@ -111,6 +114,31 @@ class Python(Package):
         else:
             module.python = Executable(join_path(spec.prefix.bin, 'python'))
 
+        # The code below patches the any python extension to have good defaults for `setup_dependent_environment` and
+        # `setup_environment` only if the extension didn't override any of these functions explicitly.
+        def _setup_env(self, env):
+            site_packages = glob.glob(join_path(self.spec.prefix.lib, "python*/site-packages"))
+            if site_packages:
+                env.prepend_path('PYTHONPATH', site_packages[0])
+
+        def _setup_denv(self, env, extension_spec):
+            pass
+
+        pkg_cls = type(ext_spec.package)  # Retrieve the type we may want to patch
+        if 'python' in pkg_cls.extendees:
+            # List of overrides we are interested in
+            interesting_overrides = ['setup_environment', 'setup_dependent_environment']
+            overrides_found = [
+                (name, defining_cls) for name, _, defining_cls, _, in inspect.classify_class_attrs(pkg_cls)
+                if
+                name in interesting_overrides and  # The attribute has the right name
+                issubclass(defining_cls, Package) and defining_cls is not Package  # and is an actual override
+            ]
+            if not overrides_found:
+                # If no override were found go on patching
+                pkg_cls.setup_environment = functools.wraps(Package.setup_environment)(_setup_env)
+                pkg_cls.setup_dependent_environment = functools.wraps(Package.setup_dependent_environment)(_setup_denv)
+
         # Add variables for lib/pythonX.Y and lib/pythonX.Y/site-packages dirs.
         module.python_lib_dir     = os.path.join(ext_spec.prefix, self.python_lib_dir)
         module.python_include_dir = os.path.join(ext_spec.prefix, self.python_include_dir)
-- 
cgit v1.2.3-70-g09d2


From 67ca2c704b9c367b41ce29342a06427e156a30e9 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 18 Mar 2016 15:18:26 +0100
Subject: modules : fixed bug in `modify_module` arguments

---
 lib/spack/spack/modules.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 7303844229..f5bf213845 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -165,7 +165,7 @@ class EnvModule(object):
         # Let the extendee modify their extensions before asking for package-specific modifications
         for extendee in self.pkg.extendees:
             extendee_spec = self.spec[extendee]
-            extendee_spec.package.modify_module(self.pkg, extendee_spec, self.spec)
+            extendee_spec.package.modify_module(self.pkg.module, extendee_spec, self.spec)
 
         # Package-specific environment modifications
         self.spec.package.setup_environment(env)
-- 
cgit v1.2.3-70-g09d2


From ccd90df62ffd69b5c3b4e1f8dbaf82146721a52a Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 18 Mar 2016 15:41:14 +0100
Subject: modules : turned category into a property (logic can be extended
 later)

---
 lib/spack/spack/modules.py | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index f5bf213845..bb1a444e5a 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -122,10 +122,6 @@ class EnvModule(object):
         self.spec = spec
         self.pkg = spec.package  # Just stored for convenience
 
-        # category in the modules system
-        # TODO: come up with smarter category names.
-        self.category = "spack"
-
         # short description default is just the package + version
         # packages can provide this optional attribute
         self.short_description = spec.format("$_ $@")
@@ -137,6 +133,17 @@ class EnvModule(object):
         if self.spec.package.__doc__:
             self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
 
+    @property
+    def category(self):
+        # Anything defined at the package level takes precedence
+        if hasattr(self.pkg, 'category'):
+            return self.pkg.category
+        # Extensions
+        for extendee in self.pkg.extendees:
+            return '{extendee} extension'.format(extendee=extendee)
+        # Not very descriptive fallback
+        return 'spack installed package'
+
     # @property
     # def paths(self):
     #         # Add python path unless it's an actual python installation
-- 
cgit v1.2.3-70-g09d2


From 1e468c55414822365b4ba7f7c52fbfdb5f30d3b2 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 18 Mar 2016 16:02:44 +0100
Subject: modules : added formats mapping

---
 lib/spack/spack/modules.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index bb1a444e5a..0378110c8c 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -256,7 +256,10 @@ class TclModule(EnvModule):
 
     formats = {
         PrependPath: 'prepend-path {name} \"{value}\"\n',
-        SetEnv: 'setenv {name} \"{value}\"\n'
+        AppendPath: 'append-path {name} \"{value}\"\n',
+        RemovePath: 'remove-path {name} \"{value}\"\n',
+        SetEnv: 'setenv {name} \"{value}\"\n',
+        UnsetEnv: 'unsetenv {name}\n'
     }
 
     @property
-- 
cgit v1.2.3-70-g09d2


From 491babd5cd435fc307b6e977b850e4e64d2b0ccf Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 18 Mar 2016 17:09:20 +0100
Subject: env modifications : added a validation rule

---
 lib/spack/spack/build_environment.py |  6 +++---
 lib/spack/spack/environment.py       | 30 ++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index c51fa58477..59b234624c 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -34,8 +34,9 @@ import shutil
 import sys
 
 import spack
+import llnl.util.tty as tty
 from llnl.util.filesystem import *
-from spack.environment import EnvironmentModifications, concatenate_paths
+from spack.environment import EnvironmentModifications, concatenate_paths, validate
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -288,8 +289,7 @@ def setup_package(pkg):
     # Allow dependencies to set up environment as well
     for dependency_spec in pkg.spec.traverse(root=False):
         dependency_spec.package.setup_dependent_environment(env, pkg.spec)
-    # TODO : implement validation
-    #validate(env)
+    validate(env, tty.warn)
     env.apply_modifications()
 
 
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 6d214595a3..74aef57fe8 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -202,3 +202,33 @@ def concatenate_paths(paths):
         string
     """
     return ':'.join(str(item) for item in paths)
+
+
+def set_or_unset_not_first(variable, changes, errstream):
+    """
+    Check if we are going to set or unset something after other modifications have already been requested
+    """
+    indexes = [ii for ii, item in enumerate(changes) if ii != 0 and type(item) in [SetEnv, UnsetEnv]]
+    if indexes:
+        good = '\t    \t{context} at {filename}:{lineno}'
+        nogood = '\t--->\t{context} at {filename}:{lineno}'
+        errstream('Suspicious requests to set or unset the variable \'{var}\' found'.format(var=variable))
+        for ii, item in enumerate(changes):
+            print_format = nogood if ii in indexes else good
+            errstream(print_format.format(**item.args))
+
+
+def validate(env, errstream):
+    """
+    Validates the environment modifications to check for the presence of suspicious patterns. Prompts a warning for
+    everything that was found
+
+    Current checks:
+    - set or unset variables after other changes on the same variable
+
+    Args:
+        env: list of environment modifications
+    """
+    modifications = env.group_by_name()
+    for variable, list_of_changes in sorted(modifications.items()):
+        set_or_unset_not_first(variable, list_of_changes, errstream)
-- 
cgit v1.2.3-70-g09d2


From 1fa38689d87480bb0d291af9b9cf0dbcf7557eb5 Mon Sep 17 00:00:00 2001
From: Gregory Becker <becker33@llnl.gov>
Date: Fri, 18 Mar 2016 09:35:56 -0700
Subject: Created flatten_dependencies function that dummy packages can use to
 create sane install environments.

---
 lib/spack/spack/directory_layout.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

(limited to 'lib')

diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py
index 242eb1afa0..9f17404062 100644
--- a/lib/spack/spack/directory_layout.py
+++ b/lib/spack/spack/directory_layout.py
@@ -33,6 +33,7 @@ import yaml
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import join_path, mkdirp
+from llnl.util.link_tree import *
 
 from spack.spec import Spec
 from spack.error import SpackError
@@ -131,6 +132,23 @@ class DirectoryLayout(object):
         raise NotImplementedError()
 
 
+    def flatten_dependencies(self, spec, flat_dir):
+        """Make each dependency of spec present in dir via symlink."""
+        for dep in spec.traverse(root=False):
+            name = dep.name
+
+            dep_path = self.path_for_spec(dep)
+            dep_files = LinkTree(dep_path)
+
+            os.mkdir(flat_dir+'/'+name)
+
+            conflict = dep_files.find_conflict(flat_dir+'/'+name)
+            if conflict:
+                raise DependencyConflictError(conflict)
+
+            dep_files.merge(flat_dir+'/'+name)
+
+
     def path_for_spec(self, spec):
         """Return an absolute path from the root to a directory for the spec."""
         _check_concrete(spec)
-- 
cgit v1.2.3-70-g09d2


From 80495e50f9b3a5de6d0aadb054a770a685939e79 Mon Sep 17 00:00:00 2001
From: Gregory Becker <becker33@llnl.gov>
Date: Fri, 18 Mar 2016 09:46:18 -0700
Subject: added error class for error that should never come up

---
 lib/spack/spack/directory_layout.py | 7 +++++++
 1 file changed, 7 insertions(+)

(limited to 'lib')

diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py
index 9f17404062..dfd0dc42e4 100644
--- a/lib/spack/spack/directory_layout.py
+++ b/lib/spack/spack/directory_layout.py
@@ -498,3 +498,10 @@ class NoSuchExtensionError(DirectoryLayoutError):
         super(NoSuchExtensionError, self).__init__(
             "%s cannot be removed from %s because it's not activated."% (
                 ext_spec.short_spec, spec.short_spec))
+
+class DependencyConflictError(SpackError):
+    """Raised when the dependencies cannot be flattened as asked for."""
+    def __init__(self, conflict):
+        super(DependencyConflictError, self).__init__(
+            "%s conflicts with another file in the flattened directory." %(
+                conflict))
-- 
cgit v1.2.3-70-g09d2


From 6acb830263e663b57bc8c077b348c897b0d1f612 Mon Sep 17 00:00:00 2001
From: "Adam J. Stewart" <ajstewart426@gmail.com>
Date: Fri, 18 Mar 2016 13:03:55 -0500
Subject: Add support for .tar files

---
 lib/spack/spack/url.py              | 2 +-
 lib/spack/spack/util/compression.py | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/url.py b/lib/spack/spack/url.py
index ad551a6ded..f51f05cad7 100644
--- a/lib/spack/spack/url.py
+++ b/lib/spack/spack/url.py
@@ -142,7 +142,7 @@ def split_url_extension(path):
 
 def downloaded_file_extension(path):
     """This returns the type of archive a URL refers to.  This is
-       sometimes confusing becasue of URLs like:
+       sometimes confusing because of URLs like:
 
            (1) https://github.com/petdance/ack/tarball/1.93_02
 
diff --git a/lib/spack/spack/util/compression.py b/lib/spack/spack/util/compression.py
index ea1f233bce..5ae5867428 100644
--- a/lib/spack/spack/util/compression.py
+++ b/lib/spack/spack/util/compression.py
@@ -27,13 +27,12 @@ import os
 from itertools import product
 from spack.util.executable import which
 
-# Supported archvie extensions.
+# Supported archive extensions.
 PRE_EXTS = ["tar"]
 EXTS     = ["gz", "bz2", "xz", "Z", "zip", "tgz"]
 
-# Add EXTS last so that .tar.gz is matched *before* tar.gz
-ALLOWED_ARCHIVE_TYPES = [".".join(l) for l in product(PRE_EXTS, EXTS)] + EXTS
-
+# Add PRE_EXTS and EXTS last so that .tar.gz is matched *before* .tar or .gz
+ALLOWED_ARCHIVE_TYPES = [".".join(l) for l in product(PRE_EXTS, EXTS)] + PRE_EXTS + EXTS
 
 def allowed_archive(path):
     return any(path.endswith(t) for t in ALLOWED_ARCHIVE_TYPES)
-- 
cgit v1.2.3-70-g09d2


From 76672a4e34e37aec0223e67d140fd19e9115b5a6 Mon Sep 17 00:00:00 2001
From: Gregory Becker <becker33@llnl.gov>
Date: Fri, 18 Mar 2016 11:28:44 -0700
Subject: Refactoring flat_install

---
 lib/spack/spack/__init__.py         |  3 +++
 lib/spack/spack/directory_layout.py | 24 ------------------------
 lib/spack/spack/package.py          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 24 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py
index ab78ecef30..155e190597 100644
--- a/lib/spack/spack/__init__.py
+++ b/lib/spack/spack/__init__.py
@@ -174,3 +174,6 @@ __all__ += spack.directives.__all__
 import spack.util.executable
 from spack.util.executable import *
 __all__ += spack.util.executable.__all__
+
+from spack.package import flat_install, flatten_dependencies, DependencyConflictError
+__all__ += ['flat_install', 'flatten_dependencies', 'DependencyConflictError']
diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py
index dfd0dc42e4..e275031387 100644
--- a/lib/spack/spack/directory_layout.py
+++ b/lib/spack/spack/directory_layout.py
@@ -132,23 +132,6 @@ class DirectoryLayout(object):
         raise NotImplementedError()
 
 
-    def flatten_dependencies(self, spec, flat_dir):
-        """Make each dependency of spec present in dir via symlink."""
-        for dep in spec.traverse(root=False):
-            name = dep.name
-
-            dep_path = self.path_for_spec(dep)
-            dep_files = LinkTree(dep_path)
-
-            os.mkdir(flat_dir+'/'+name)
-
-            conflict = dep_files.find_conflict(flat_dir+'/'+name)
-            if conflict:
-                raise DependencyConflictError(conflict)
-
-            dep_files.merge(flat_dir+'/'+name)
-
-
     def path_for_spec(self, spec):
         """Return an absolute path from the root to a directory for the spec."""
         _check_concrete(spec)
@@ -498,10 +481,3 @@ class NoSuchExtensionError(DirectoryLayoutError):
         super(NoSuchExtensionError, self).__init__(
             "%s cannot be removed from %s because it's not activated."% (
                 ext_spec.short_spec, spec.short_spec))
-
-class DependencyConflictError(SpackError):
-    """Raised when the dependencies cannot be flattened as asked for."""
-    def __init__(self, conflict):
-        super(DependencyConflictError, self).__init__(
-            "%s conflicts with another file in the flattened directory." %(
-                conflict))
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 972a0410b9..1a962268cf 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1211,6 +1211,28 @@ class Package(object):
         return " ".join("-Wl,-rpath,%s" % p for p in self.rpath)
 
 
+def flat_install(pkg, spec, prefix):
+    """Execute a dummy install and flatten dependencies"""
+    os.mkdir(prefix+'/libs')
+    flatten_dependencies(spec, prefix+'/libs')
+
+def flatten_dependencies(spec, flat_dir):
+    """Make each dependency of spec present in dir via symlink."""
+    for dep in spec.traverse(root=False):
+        name = dep.name
+
+        dep_path = spack.install_layout.path_for_spec(dep)
+        dep_files = LinkTree(dep_path)
+
+        os.mkdir(flat_dir+'/'+name)
+
+        conflict = dep_files.find_conflict(flat_dir+'/'+name)
+        if conflict:
+            raise DependencyConflictError(conflict)
+
+        dep_files.merge(flat_dir+'/'+name)
+
+
 def validate_package_url(url_string):
     """Determine whether spack can handle a particular URL or not."""
     url = urlparse(url_string)
@@ -1348,3 +1370,11 @@ class ExtensionConflictError(ExtensionError):
 class ActivationError(ExtensionError):
     def __init__(self, msg, long_msg=None):
         super(ActivationError, self).__init__(msg, long_msg)
+
+
+class DependencyConflictError(spack.error.SpackError):
+    """Raised when the dependencies cannot be flattened as asked for."""
+    def __init__(self, conflict):
+        super(DependencyConflictError, self).__init__(
+            "%s conflicts with another file in the flattened directory." %(
+                conflict))
-- 
cgit v1.2.3-70-g09d2


From af7e3cadde02b2ab9b6de2c588a41663141928dd Mon Sep 17 00:00:00 2001
From: Gregory Becker <becker33@llnl.gov>
Date: Fri, 18 Mar 2016 11:34:07 -0700
Subject: cleanup

---
 lib/spack/spack/directory_layout.py | 1 -
 1 file changed, 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py
index e275031387..242eb1afa0 100644
--- a/lib/spack/spack/directory_layout.py
+++ b/lib/spack/spack/directory_layout.py
@@ -33,7 +33,6 @@ import yaml
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import join_path, mkdirp
-from llnl.util.link_tree import *
 
 from spack.spec import Spec
 from spack.error import SpackError
-- 
cgit v1.2.3-70-g09d2


From 151b04637ef01b8cde5967df6165bd245311b90a Mon Sep 17 00:00:00 2001
From: Gregory Becker <becker33@llnl.gov>
Date: Fri, 18 Mar 2016 11:55:31 -0700
Subject: changed function name and removed hardcoded libs dir

---
 lib/spack/spack/__init__.py | 4 ++--
 lib/spack/spack/package.py  | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py
index 155e190597..5d62d597cb 100644
--- a/lib/spack/spack/__init__.py
+++ b/lib/spack/spack/__init__.py
@@ -175,5 +175,5 @@ import spack.util.executable
 from spack.util.executable import *
 __all__ += spack.util.executable.__all__
 
-from spack.package import flat_install, flatten_dependencies, DependencyConflictError
-__all__ += ['flat_install', 'flatten_dependencies', 'DependencyConflictError']
+from spack.package import install_dependency_symlinks, flatten_dependencies, DependencyConflictError
+__all__ += ['install_dependency_symlinks', 'flatten_dependencies', 'DependencyConflictError']
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 1a962268cf..4c458522e0 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1211,10 +1211,9 @@ class Package(object):
         return " ".join("-Wl,-rpath,%s" % p for p in self.rpath)
 
 
-def flat_install(pkg, spec, prefix):
+def install_dependency_symlinks(pkg, spec, prefix):
     """Execute a dummy install and flatten dependencies"""
-    os.mkdir(prefix+'/libs')
-    flatten_dependencies(spec, prefix+'/libs')
+    flatten_dependencies(spec, prefix)
 
 def flatten_dependencies(spec, flat_dir):
     """Make each dependency of spec present in dir via symlink."""
-- 
cgit v1.2.3-70-g09d2


From 23f3f1adfe01f7b1301959e7e54584724679c7b3 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Fri, 18 Mar 2016 13:32:37 -0700
Subject: Make InstallError and ExternalPackageError visiible in packages.

---
 lib/spack/spack/__init__.py | 8 ++++++--
 lib/spack/spack/package.py  | 4 ++++
 2 files changed, 10 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py
index 0ba42bbbfc..aee11f061f 100644
--- a/lib/spack/spack/__init__.py
+++ b/lib/spack/spack/__init__.py
@@ -189,5 +189,9 @@ import spack.util.executable
 from spack.util.executable import *
 __all__ += spack.util.executable.__all__
 
-from spack.package import install_dependency_symlinks, flatten_dependencies, DependencyConflictError
-__all__ += ['install_dependency_symlinks', 'flatten_dependencies', 'DependencyConflictError']
+from spack.package import \
+    install_dependency_symlinks, flatten_dependencies, DependencyConflictError, \
+    InstallError, ExternalPackageError
+__all__ += [
+    'install_dependency_symlinks', 'flatten_dependencies', 'DependencyConflictError',
+    'InstallError', 'ExternalPackageError']
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index b488e4c49d..dafad0b184 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1351,6 +1351,10 @@ class InstallError(spack.error.SpackError):
         super(InstallError, self).__init__(message, long_msg)
 
 
+class ExternalPackageError(InstallError):
+    """Raised by install() when a package is only for external use."""
+
+
 class PackageStillNeededError(InstallError):
     """Raised when package is still needed by another on uninstall."""
     def __init__(self, spec, dependents):
-- 
cgit v1.2.3-70-g09d2


From e9126baaabbfaa5e12641b3d44a5c5fd49e43ca3 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Fri, 18 Mar 2016 13:42:45 -0700
Subject: Add docs for InstallError.

---
 lib/spack/docs/packaging_guide.rst | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

(limited to 'lib')

diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index c1077e4497..519c0da232 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -1844,6 +1844,20 @@ dedicated process.
 
 .. _prefix-objects:
 
+
+Failing the build
+----------------------
+
+Sometimes you don't want a package to successfully install unless some
+condition is true.  You can explicitly cause the build to fail from
+``install()`` by raising an ``InstallError``, for example:
+
+.. code-block:: python
+
+   if spec.architecture.startswith('darwin'):
+       raise InstallError('This package does not build on Mac OS X!')
+
+
 Prefix objects
 ----------------------
 
-- 
cgit v1.2.3-70-g09d2


From aef6d50b08daf893174046ba14c09a7019fe1212 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Mon, 21 Mar 2016 09:23:25 +0100
Subject: uninstall : fixed typo (print statement vs. print function)

---
 lib/spack/spack/cmd/uninstall.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index 2fab8f6f2a..1ece838612 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/lib/spack/spack/cmd/uninstall.py
@@ -96,9 +96,9 @@ def uninstall(parser, args):
                 pkg.do_uninstall(force=args.force)
             except PackageStillNeededError as e:
                 tty.error("Will not uninstall %s" % e.spec.format("$_$@$%@$#", color=True))
-                print()
+                print('')
                 print("The following packages depend on it:")
                 display_specs(e.dependents, long=True)
-                print()
+                print('')
                 print("You can use spack uninstall -f to force this action.")
                 sys.exit(1)
-- 
cgit v1.2.3-70-g09d2


From b79fce76cb4d412cf718b48b05d549470e4c55ab Mon Sep 17 00:00:00 2001
From: Matthew LeGendre <legendre1@llnl.gov>
Date: Mon, 21 Mar 2016 14:28:07 -0700
Subject: Fix issue 573 where Spack was ignoring user's compiler preference in
 concretization

---
 lib/spack/spack/concretize.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index 8083f91982..2e576743ec 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -241,7 +241,7 @@ class DefaultConcretizer(object):
             return False
 
         #Find the another spec that has a compiler, or the root if none do
-        other_spec = find_spec(spec, lambda(x) : x.compiler)
+        other_spec = spec if spec.compiler else find_spec(spec, lambda(x) : x.compiler)
         if not other_spec:
             other_spec = spec.root
         other_compiler = other_spec.compiler
@@ -288,7 +288,7 @@ def find_spec(spec, condition):
     if condition(spec):
         return spec
 
-    return None   # Nohting matched the condition.
+    return None   # Nothing matched the condition.
 
 
 def cmp_specs(lhs, rhs):
-- 
cgit v1.2.3-70-g09d2


From 5d06daeb5eed28b4b91ba62a8f99165d87b5ef86 Mon Sep 17 00:00:00 2001
From: Matthew LeGendre <legendre1@llnl.gov>
Date: Mon, 21 Mar 2016 14:28:34 -0700
Subject: Add test for issue 573, child with compiler not respected in
 concretization

---
 lib/spack/spack/test/concretize.py | 7 +++++++
 1 file changed, 7 insertions(+)

(limited to 'lib')

diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index f264faf17a..08cce09674 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -309,3 +309,10 @@ class ConcretizeTest(MockPackagesTest):
                       Spec('d')),
                  Spec('e'))
         self.assertEqual(None, find_spec(s['b'], lambda s: '+foo' in s))
+
+
+    def test_compiler_child(self):
+        s = Spec('mpileaks%clang ^dyninst%gcc')
+        s.concretize()
+        self.assertTrue(s['mpileaks'].satisfies('%clang'))
+        self.assertTrue(s['dyninst'].satisfies('%gcc'))
-- 
cgit v1.2.3-70-g09d2


From 48b35bb495bc154c64d23d1ad364506c4572b913 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 21 Mar 2016 00:11:18 -0700
Subject: Fix print function in uninstall.py

---
 lib/spack/spack/cmd/uninstall.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index 1ece838612..350ef372cb 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/lib/spack/spack/cmd/uninstall.py
@@ -22,6 +22,7 @@
 # along with this program; if not, write to the Free Software Foundation,
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
+from __future__ import print_function
 import sys
 import argparse
 
@@ -63,12 +64,12 @@ def uninstall(parser, args):
             matching_specs = spack.installed_db.query(spec)
             if not args.all and len(matching_specs) > 1:
                 tty.error("%s matches multiple packages:" % spec)
-                print
+                print()
                 display_specs(matching_specs, long=True)
-                print
-                print "You can either:"
-                print "  a) Use a more specific spec, or"
-                print "  b) use spack uninstall -a to uninstall ALL matching specs."
+                print()
+                print("You can either:")
+                print("  a) Use a more specific spec, or")
+                print("  b) use spack uninstall -a to uninstall ALL matching specs.")
                 sys.exit(1)
 
             if len(matching_specs) == 0:
-- 
cgit v1.2.3-70-g09d2


From e88df95b42fdcaa49552811853f8ca4ecc52cf9f Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 21 Mar 2016 00:35:13 -0700
Subject: Remove unused code from modules.py

---
 lib/spack/spack/modules.py | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 0378110c8c..4e98d50001 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -133,6 +133,7 @@ class EnvModule(object):
         if self.spec.package.__doc__:
             self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
 
+
     @property
     def category(self):
         # Anything defined at the package level takes precedence
@@ -144,21 +145,6 @@ class EnvModule(object):
         # Not very descriptive fallback
         return 'spack installed package'
 
-    # @property
-    # def paths(self):
-    #         # Add python path unless it's an actual python installation
-    #         # TODO : is there a better way to do this?
-    #         # FIXME : add PYTHONPATH to every python package
-    #         if self.spec.name != 'python':
-    #             site_packages = glob(join_path(self.spec.prefix.lib, "python*/site-packages"))
-    #             if site_packages:
-    #                 add_path('PYTHONPATH', site_packages[0])
-    #
-    #         # FIXME : Same for GEM_PATH
-    #         if self.spec.package.extends(spack.spec.Spec('ruby')):
-    #             add_path('GEM_PATH', self.spec.prefix)
-    #
-    #     return self._paths
 
     def write(self):
         """Write out a module file for this object."""
@@ -166,16 +152,20 @@ class EnvModule(object):
         if not os.path.exists(module_dir):
             mkdirp(module_dir)
 
-        # Environment modifications guessed by inspecting the installation prefix
+        # Environment modifications guessed by inspecting the
+        # installation prefix
         env = inspect_path(self.spec.prefix)
 
-        # Let the extendee modify their extensions before asking for package-specific modifications
+        # Let the extendee modify their extensions before asking for
+        # package-specific modifications
         for extendee in self.pkg.extendees:
             extendee_spec = self.spec[extendee]
-            extendee_spec.package.modify_module(self.pkg.module, extendee_spec, self.spec)
+            extendee_spec.package.modify_module(
+                self.pkg.module, extendee_spec, self.spec)
 
         # Package-specific environment modifications
         self.spec.package.setup_environment(env)
+
         # TODO : implement site-specific modifications and filters
         if not env:
             return
@@ -232,7 +222,7 @@ class Dotkit(EnvModule):
     def use_name(self):
       return "%s-%s-%s-%s-%s" % (self.spec.name, self.spec.version,
                                  self.spec.compiler.name,
-                                 self.spec.compiler.version, 
+                                 self.spec.compiler.version,
                                  self.spec.dag_hash())
 
     def write_header(self, dk_file):
@@ -270,7 +260,7 @@ class TclModule(EnvModule):
     def use_name(self):
       return "%s-%s-%s-%s-%s" % (self.spec.name, self.spec.version,
                                  self.spec.compiler.name,
-                                 self.spec.compiler.version, 
+                                 self.spec.compiler.version,
                                  self.spec.dag_hash())
 
     def write_header(self, module_file):
-- 
cgit v1.2.3-70-g09d2


From 439d47b4e45c674ab9aa4ebd0c2bfaf6911ade60 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 21 Mar 2016 01:48:18 -0700
Subject: Refactor environment setup.

- Gave setup_environment and setup_dependent_environment more similar
  signatures. They now allows editing the Spack env and the runtime
  env for *this* package and dependents, respectively.

- modify_module renamed to setup_dependent_python_module for symmetry
  with setup_dependent_environment and to avoid confusion with
  environment modules.

- removed need for patching Package objects at runtime.

- adjust packages to reflect these changes.
---
 lib/spack/spack/build_environment.py               |  35 +++---
 lib/spack/spack/modules.py                         |   3 +-
 lib/spack/spack/package.py                         | 120 ++++++++++++++++-----
 var/spack/repos/builtin/packages/mpich/package.py  |   9 +-
 .../builtin/packages/netlib-scalapack/package.py   |   2 +-
 .../repos/builtin/packages/openmpi/package.py      |  18 ++--
 var/spack/repos/builtin/packages/python/package.py |  39 ++-----
 var/spack/repos/builtin/packages/qt/package.py     |  10 +-
 var/spack/repos/builtin/packages/ruby/package.py   |  12 +--
 9 files changed, 151 insertions(+), 97 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 59b234624c..5688d47e2d 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -84,7 +84,7 @@ class MakeExecutable(Executable):
         return super(MakeExecutable, self).__call__(*args, **kwargs)
 
 
-def set_compiler_environment_variables(pkg):
+def set_compiler_environment_variables(pkg, env):
     assert pkg.spec.concrete
     # Set compiler variables used by CMake and autotools
     assert all(key in pkg.compiler.link_paths for key in ('cc', 'cxx', 'f77', 'fc'))
@@ -92,7 +92,6 @@ def set_compiler_environment_variables(pkg):
     # Populate an object with the list of environment modifications
     # and return it
     # TODO : add additional kwargs for better diagnostics, like requestor, ttyout, ttyerr, etc.
-    env = EnvironmentModifications()
     link_dir = spack.build_env_path
     env.set_env('CC', join_path(link_dir, pkg.compiler.link_paths['cc']))
     env.set_env('CXX', join_path(link_dir, pkg.compiler.link_paths['cxx']))
@@ -113,7 +112,7 @@ def set_compiler_environment_variables(pkg):
     return env
 
 
-def set_build_environment_variables(pkg):
+def set_build_environment_variables(pkg, env):
     """
     This ensures a clean install environment when we build packages
     """
@@ -134,7 +133,6 @@ def set_build_environment_variables(pkg):
         if os.path.isdir(ci):
             env_paths.append(ci)
 
-    env = EnvironmentModifications()
     for item in reversed(env_paths):
         env.prepend_path('PATH', item)
     env.set_env(SPACK_ENV_PATH, concatenate_paths(env_paths))
@@ -180,7 +178,7 @@ def set_build_environment_variables(pkg):
     return env
 
 
-def set_module_variables_for_package(pkg, m):
+def set_module_variables_for_package(pkg, module):
     """Populate the module scope of install() with some useful functions.
        This makes things easier for package writers.
     """
@@ -190,6 +188,8 @@ def set_module_variables_for_package(pkg, m):
         jobs = 1
     elif pkg.make_jobs:
         jobs = pkg.make_jobs
+
+    m = module
     m.make_jobs = jobs
 
     # TODO: make these build deps that can be installed if not found.
@@ -271,9 +271,12 @@ def parent_class_modules(cls):
 
 def setup_package(pkg):
     """Execute all environment setup routines."""
-    env = EnvironmentModifications()
-    env.extend(set_compiler_environment_variables(pkg))
-    env.extend(set_build_environment_variables(pkg))
+    spack_env = EnvironmentModifications()
+    run_env   = EnvironmentModifications()
+
+    set_compiler_environment_variables(pkg, spack_env)
+    set_build_environment_variables(pkg, spack_env)
+
     # If a user makes their own package repo, e.g.
     # spack.repos.mystuff.libelf.Libelf, and they inherit from
     # an existing class like spack.repos.original.libelf.Libelf,
@@ -285,12 +288,20 @@ def setup_package(pkg):
 
     # Allow dependencies to modify the module
     for dependency_spec in pkg.spec.traverse(root=False):
-        dependency_spec.package.modify_module(pkg.module, dependency_spec, pkg.spec)
+        dpkg = dependency_spec.package
+        dpkg.setup_dependent_python_module(pkg.module, pkg.spec)
+
     # Allow dependencies to set up environment as well
     for dependency_spec in pkg.spec.traverse(root=False):
-        dependency_spec.package.setup_dependent_environment(env, pkg.spec)
-    validate(env, tty.warn)
-    env.apply_modifications()
+        dpkg = dependency_spec.package
+        dpkg.setup_dependent_environment(spack_env, run_env, pkg.spec)
+
+    # Allow the package to apply some settings.
+    pkg.setup_environment(spack_env, run_env)
+
+    # Make sure nothing's strange about the Spack environment.
+    validate(spack_env, tty.warn)
+    spack_env.apply_modifications()
 
 
 def fork(pkg, function):
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 4e98d50001..05c93cd3e6 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -164,7 +164,8 @@ class EnvModule(object):
                 self.pkg.module, extendee_spec, self.spec)
 
         # Package-specific environment modifications
-        self.spec.package.setup_environment(env)
+        spack_env = EnvironmentModifications()
+        self.spec.package.setup_environment(spack_env, env)
 
         # TODO : implement site-specific modifications and filters
         if not env:
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index acad5a28f6..9d8ac87bd7 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1002,56 +1002,120 @@ class Package(object):
         return __import__(self.__class__.__module__,
                           fromlist=[self.__class__.__name__])
 
-    def setup_environment(self, env):
-        """
-        Appends in `env` the list of environment modifications needed to use this package outside of spack.
+    def setup_environment(self, spack_env, run_env):
+        """Set up the compile and runtime environemnts for a package.
 
-        Default implementation does nothing, but this can be overridden if the package needs a particular environment.
+        `spack_env` and `run_env` are `EnvironmentModifications`
+        objects.  Package authors can call methods on them to alter
+        the environment within Spack and at runtime.
 
-        Example :
+        Both `spack_env` and `run_env` are applied within the build
+        process, before this package's `install()` method is called.
+
+        Modifications in `run_env` will *also* be added to the
+        generated environment modules for this package.
+
+        Default implementation does nothing, but this can be
+        overridden if the package needs a particular environment.
 
-        1. A lot of Qt extensions need `QTDIR` set.  This can be used to do that.
+        Examples:
+
+            1. Qt extensions need `QTDIR` set.
 
         Args:
-            env: list of environment modifications to be updated
+            spack_env (EnvironmentModifications): list of
+                modifications to be applied when this package is built
+                within Spack.
+
+            run_env (EnvironmentModifications): list of environment
+                changes to be applied when this package is run outside
+                of Spack.
+
         """
         pass
 
-    def setup_dependent_environment(self, env, dependent_spec):
-        """
-        Called before the install() method of dependents.
 
-        Appends in `env` the list of environment modifications needed by dependents (or extensions) during the
-        installation of a package. The default implementation delegates to `setup_environment`, but can be overridden
-        if the modifications to the environment happen to be different from the one needed to use the package outside
-        of spack.
+    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
+        """Set up the environment of packages that depend on this one.
+
+        This is similar to `setup_environment`, but it is used to
+        modify the compile and runtime environments of packages that
+        *depend* on this one. This gives packages like Python and
+        others that follow the extension model a way to implement
+        common environment or compile-time settings for dependencies.
 
-        This is useful if there are some common steps to installing all extensions for a certain package.
+        By default, this delegates to self.setup_environment()
 
         Example :
 
-        1. Installing python modules generally requires `PYTHONPATH` to point to the lib/pythonX.Y/site-packages
-        directory in the module's install prefix.  This could set that variable.
+            1. Installing python modules generally requires
+              `PYTHONPATH` to point to the lib/pythonX.Y/site-packages
+              directory in the module's install prefix.  This could
+              set that variable.
 
         Args:
-            env: list of environment modifications to be updated
-            dependent_spec: dependent (or extension) of this spec
-        """
-        self.setup_environment(env)
 
-    def modify_module(self, module, spec, dependent_spec):
+            spack_env (EnvironmentModifications): list of
+                modifications to be applied when the dependent package
+                is bulit within Spack.
+
+            run_env (EnvironmentModifications): list of environment
+                changes to be applied when the dependent package is
+                run outside of Spack.
+
+            dependent_spec (Spec): The spec of the dependent package
+                about to be built. This allows the extendee (self) to
+                query the dependent's state. Note that *this*
+                package's spec is available as `self.spec`.
+
+        This is useful if there are some common steps to installing
+        all extensions for a certain package.
+
         """
+        self.setup_environment(spack_env, run_env)
+
+
+    def setup_dependent_python_module(self, module, dependent_spec):
+        """Set up Python module-scope variables for dependent packages.
+
         Called before the install() method of dependents.
 
-        Default implementation does nothing, but this can be overridden by an extendable package to set up the module of
-        its extensions. This is useful if there are some common steps to installing all extensions for a
-        certain package.
+        Default implementation does nothing, but this can be
+        overridden by an extendable package to set up the module of
+        its extensions. This is useful if there are some common steps
+        to installing all extensions for a certain package.
 
         Example :
 
-        1. Extensions often need to invoke the 'python' interpreter from the Python installation being extended.
-        This routine can put a 'python' Executable object in the module scope for the extension package to simplify
-        extension installs.
+            1. Extensions often need to invoke the `python`
+               interpreter from the Python installation being
+               extended.  This routine can put a 'python' Executable
+               object in the module scope for the extension package to
+               simplify extension installs.
+
+            2. MPI compilers could set some variables in the
+               dependent's scope that point to `mpicc`, `mpicxx`,
+               etc., allowing them to be called by common names
+               regardless of which MPI is used.
+
+            3. BLAS/LAPACK implementations can set some variables
+               indicating the path to their libraries, since these
+               paths differ by BLAS/LAPACK implementation.
+
+        Args:
+
+            module (module): The Python `module` object of the
+                dependent package. Packages can use this to set
+                module-scope variables for the dependent to use.
+
+            dependent_spec (Spec): The spec of the dependent package
+                about to be built. This allows the extendee (self) to
+                query the dependent's state.  Note that *this*
+                package's spec is available as `self.spec`.
+
+        This is useful if there are some common steps to installing
+        all extensions for a certain package.
+
         """
         pass
 
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index 5af9b585ea..90b5d42eab 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -47,13 +47,6 @@ class Mpich(Package):
     provides('mpi@:3.0', when='@3:')
     provides('mpi@:1.3', when='@1:')
 
-    def setup_environment(self, env):
-        env.set_env('MPICH_CC', self.compiler.cc)
-        env.set_env('MPICH_CXX', self.compiler.cxx)
-        env.set_env('MPICH_F77', self.compiler.f77)
-        env.set_env('MPICH_F90', self.compiler.fc)
-        env.set_env('MPICH_FC', self.compiler.fc)
-
     def setup_dependent_environment(self, env, dependent_spec):
         env.set_env('MPICH_CC', spack_cc)
         env.set_env('MPICH_CXX', spack_cxx)
@@ -61,7 +54,7 @@ class Mpich(Package):
         env.set_env('MPICH_F90', spack_f90)
         env.set_env('MPICH_FC', spack_fc)
 
-    def modify_module(self, module, spec, dep_spec):
+    def setup_dependent_python_module(self, module, spec, dep_spec):
         """For dependencies, make mpicc's use spack wrapper."""
         # FIXME : is this necessary ? Shouldn't this be part of a contract with MPI providers?
         module.mpicc = join_path(self.prefix.bin, 'mpicc')
diff --git a/var/spack/repos/builtin/packages/netlib-scalapack/package.py b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
index 36f506f7cd..62abfcc48e 100644
--- a/var/spack/repos/builtin/packages/netlib-scalapack/package.py
+++ b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
@@ -40,7 +40,7 @@ class NetlibScalapack(Package):
             make()
             make("install")
 
-    def modify_module(self, module, spec, dependent_spec):
+    def setup_dependent_python_module(self, module, spec, dependent_spec):
         lib_dsuffix = '.dylib' if sys.platform == 'darwin' else '.so'
         lib_suffix = lib_dsuffix if '+shared' in spec['scalapack'] else '.a'
 
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index 7783ca8766..c91a13e376 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -41,17 +41,13 @@ class Openmpi(Package):
     def url_for_version(self, version):
         return "http://www.open-mpi.org/software/ompi/v%s/downloads/openmpi-%s.tar.bz2" % (version.up_to(2), version)
 
-    def setup_environment(self, env):
-        env.set_env('OMPI_CC', self.compiler.cc)
-        env.set_env('OMPI_CXX', self.compiler.cxx)
-        env.set_env('OMPI_FC', self.compiler.fc)
-        env.set_env('OMPI_F77', self.compiler.f77)
-
-    def setup_dependent_environment(self, env, dependent_spec):
-        env.set_env('OMPI_CC', spack_cc)
-        env.set_env('OMPI_CXX', spack_cxx)
-        env.set_env('OMPI_FC', spack_fc)
-        env.set_env('OMPI_F77', spack_f77)
+
+    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
+        spack_env.set_env('OMPI_CC', spack_cc)
+        spack_env.set_env('OMPI_CXX', spack_cxx)
+        spack_env.set_env('OMPI_FC', spack_fc)
+        spack_env.set_env('OMPI_F77', spack_f77)
+
 
     def install(self, spec, prefix):
         config_args = ["--prefix=%s" % prefix,
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index c445d26369..593a27708c 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -92,13 +92,21 @@ class Python(Package):
     def site_packages_dir(self):
         return os.path.join(self.python_lib_dir, 'site-packages')
 
-    def setup_dependent_environment(self, env, extension_spec):
-        # Set PYTHONPATH to include site-packages dir for the extension and any other python extensions it depends on.
+
+    def setup_dependent_environment(self, spack_env, run_env, extension_spec):
+        # TODO: do this only for actual extensions.
+
+        # Set PYTHONPATH to include site-packages dir for the
+        # extension and any other python extensions it depends on.
         python_paths = []
         for d in extension_spec.traverse():
             if d.package.extends(self.spec):
                 python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
-        env.set_env('PYTHONPATH', ':'.join(python_paths))
+
+        pythonpath = ':'.join(python_paths)
+        spack_env.set_env('PYTHONPATH', pythonpath)
+        run_env.set_env('PYTHONPATH', pythonpath)
+
 
     def modify_module(self, module, spec, ext_spec):
         """
@@ -114,31 +122,6 @@ class Python(Package):
         else:
             module.python = Executable(join_path(spec.prefix.bin, 'python'))
 
-        # The code below patches the any python extension to have good defaults for `setup_dependent_environment` and
-        # `setup_environment` only if the extension didn't override any of these functions explicitly.
-        def _setup_env(self, env):
-            site_packages = glob.glob(join_path(self.spec.prefix.lib, "python*/site-packages"))
-            if site_packages:
-                env.prepend_path('PYTHONPATH', site_packages[0])
-
-        def _setup_denv(self, env, extension_spec):
-            pass
-
-        pkg_cls = type(ext_spec.package)  # Retrieve the type we may want to patch
-        if 'python' in pkg_cls.extendees:
-            # List of overrides we are interested in
-            interesting_overrides = ['setup_environment', 'setup_dependent_environment']
-            overrides_found = [
-                (name, defining_cls) for name, _, defining_cls, _, in inspect.classify_class_attrs(pkg_cls)
-                if
-                name in interesting_overrides and  # The attribute has the right name
-                issubclass(defining_cls, Package) and defining_cls is not Package  # and is an actual override
-            ]
-            if not overrides_found:
-                # If no override were found go on patching
-                pkg_cls.setup_environment = functools.wraps(Package.setup_environment)(_setup_env)
-                pkg_cls.setup_dependent_environment = functools.wraps(Package.setup_dependent_environment)(_setup_denv)
-
         # Add variables for lib/pythonX.Y and lib/pythonX.Y/site-packages dirs.
         module.python_lib_dir     = os.path.join(ext_spec.prefix, self.python_lib_dir)
         module.python_include_dir = os.path.join(ext_spec.prefix, self.python_include_dir)
diff --git a/var/spack/repos/builtin/packages/qt/package.py b/var/spack/repos/builtin/packages/qt/package.py
index 35b9d68462..039aeb3c31 100644
--- a/var/spack/repos/builtin/packages/qt/package.py
+++ b/var/spack/repos/builtin/packages/qt/package.py
@@ -55,8 +55,14 @@ class Qt(Package):
     depends_on("mesa", when='@4:+mesa')
     depends_on("libxcb")
 
-    def setup_environment(self, env):
-        env.set_env['QTDIR'] = self.prefix
+
+    def setup_environment(self, spack_env, env):
+        env.set_env('QTDIR', self.prefix)
+
+
+    def setup_dependent_environment(self, spack_env, run_env, dspec):
+        spack_env.set_env('QTDIR', self.prefix)
+
 
     def patch(self):
         if self.spec.satisfies('@4'):
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 2d1da8c9af..39f65f51d2 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -2,7 +2,7 @@ from spack import *
 
 
 class Ruby(Package):
-    """A dynamic, open source programming language with a focus on 
+    """A dynamic, open source programming language with a focus on
     simplicity and productivity."""
 
     homepage = "https://www.ruby-lang.org/"
@@ -17,15 +17,17 @@ class Ruby(Package):
         make()
         make("install")
 
-    def setup_dependent_environment(self, env, extension_spec):
+    def setup_dependent_environment(self, spack_env, run_env, extension_spec):
+        # TODO: do this only for actual extensions.
         # Set GEM_PATH to include dependent gem directories
         ruby_paths = []
         for d in extension_spec.traverse():
             if d.package.extends(self.spec):
                 ruby_paths.append(d.prefix)
-        env.set_env('GEM_PATH', concatenate_paths(ruby_paths))
+
+        spack_env.set_env('GEM_PATH', concatenate_paths(ruby_paths))
         # The actual installation path for this gem
-        env.set_env('GEM_HOME', extension_spec.prefix)
+        spack_env.set_env('GEM_HOME', extension_spec.prefix)
 
     def modify_module(self, module, spec, ext_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME
@@ -38,5 +40,3 @@ class Ruby(Package):
         # Ruby extension builds have global ruby and gem functions
         module.ruby = Executable(join_path(spec.prefix.bin, 'ruby'))
         module.gem = Executable(join_path(spec.prefix.bin, 'gem'))
-
-
-- 
cgit v1.2.3-70-g09d2


From b1516f64eb75c108eded1e9ee7e0480a4552236a Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 21 Mar 2016 02:21:31 -0700
Subject: Rename some environment methods to be less repetitive, add set_path.

---
 lib/spack/spack/build_environment.py               | 45 +++++++++++-----------
 lib/spack/spack/environment.py                     | 22 ++++++++++-
 lib/spack/spack/test/environment.py                | 22 +++++++----
 var/spack/repos/builtin/packages/mpich/package.py  | 10 ++---
 .../repos/builtin/packages/openmpi/package.py      |  8 ++--
 var/spack/repos/builtin/packages/python/package.py |  4 +-
 var/spack/repos/builtin/packages/qt/package.py     |  4 +-
 var/spack/repos/builtin/packages/ruby/package.py   |  5 ++-
 8 files changed, 73 insertions(+), 47 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 5688d47e2d..fc5b7d6207 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -36,7 +36,7 @@ import sys
 import spack
 import llnl.util.tty as tty
 from llnl.util.filesystem import *
-from spack.environment import EnvironmentModifications, concatenate_paths, validate
+from spack.environment import EnvironmentModifications, validate
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -93,22 +93,23 @@ def set_compiler_environment_variables(pkg, env):
     # and return it
     # TODO : add additional kwargs for better diagnostics, like requestor, ttyout, ttyerr, etc.
     link_dir = spack.build_env_path
-    env.set_env('CC', join_path(link_dir, pkg.compiler.link_paths['cc']))
-    env.set_env('CXX', join_path(link_dir, pkg.compiler.link_paths['cxx']))
-    env.set_env('F77', join_path(link_dir, pkg.compiler.link_paths['f77']))
-    env.set_env('FC', join_path(link_dir, pkg.compiler.link_paths['fc']))
+    env.set('CC', join_path(link_dir, pkg.compiler.link_paths['cc']))
+    env.set('CXX', join_path(link_dir, pkg.compiler.link_paths['cxx']))
+    env.set('F77', join_path(link_dir, pkg.compiler.link_paths['f77']))
+    env.set('FC', join_path(link_dir, pkg.compiler.link_paths['fc']))
+
     # Set SPACK compiler variables so that our wrapper knows what to call
     compiler = pkg.compiler
     if compiler.cc:
-        env.set_env('SPACK_CC', compiler.cc)
+        env.set('SPACK_CC', compiler.cc)
     if compiler.cxx:
-        env.set_env('SPACK_CXX', compiler.cxx)
+        env.set('SPACK_CXX', compiler.cxx)
     if compiler.f77:
-        env.set_env('SPACK_F77', compiler.f77)
+        env.set('SPACK_F77', compiler.f77)
     if compiler.fc:
-        env.set_env('SPACK_FC', compiler.fc)
+        env.set('SPACK_FC', compiler.fc)
 
-    env.set_env('SPACK_COMPILER_SPEC', str(pkg.spec.compiler))
+    env.set('SPACK_COMPILER_SPEC', str(pkg.spec.compiler))
     return env
 
 
@@ -135,25 +136,25 @@ def set_build_environment_variables(pkg, env):
 
     for item in reversed(env_paths):
         env.prepend_path('PATH', item)
-    env.set_env(SPACK_ENV_PATH, concatenate_paths(env_paths))
+    env.set_path(SPACK_ENV_PATH, env_paths)
 
     # Prefixes of all of the package's dependencies go in SPACK_DEPENDENCIES
     dep_prefixes = [d.prefix for d in pkg.spec.traverse(root=False)]
-    env.set_env(SPACK_DEPENDENCIES, concatenate_paths(dep_prefixes))
-    env.set_env('CMAKE_PREFIX_PATH', concatenate_paths(dep_prefixes))  # Add dependencies to CMAKE_PREFIX_PATH
+    env.set_path(SPACK_DEPENDENCIES, dep_prefixes)
+    env.set_path('CMAKE_PREFIX_PATH', dep_prefixes)  # Add dependencies to CMAKE_PREFIX_PATH
 
     # Install prefix
-    env.set_env(SPACK_PREFIX, pkg.prefix)
+    env.set(SPACK_PREFIX, pkg.prefix)
 
     # Install root prefix
-    env.set_env(SPACK_INSTALL, spack.install_path)
+    env.set(SPACK_INSTALL, spack.install_path)
 
     # Remove these vars from the environment during build because they
     # can affect how some packages find libraries.  We want to make
     # sure that builds never pull in unintended external dependencies.
-    env.unset_env('LD_LIBRARY_PATH')
-    env.unset_env('LD_RUN_PATH')
-    env.unset_env('DYLD_LIBRARY_PATH')
+    env.unset('LD_LIBRARY_PATH')
+    env.unset('LD_RUN_PATH')
+    env.unset('DYLD_LIBRARY_PATH')
 
     # Add bin directories from dependencies to the PATH for the build.
     bin_dirs = reversed(filter(os.path.isdir, ['%s/bin' % prefix for prefix in dep_prefixes]))
@@ -162,9 +163,9 @@ def set_build_environment_variables(pkg, env):
 
     # Working directory for the spack command itself, for debug logs.
     if spack.debug:
-        env.set_env(SPACK_DEBUG, 'TRUE')
-    env.set_env(SPACK_SHORT_SPEC, pkg.spec.short_spec)
-    env.set_env(SPACK_DEBUG_LOG_DIR, spack.spack_working_dir)
+        env.set(SPACK_DEBUG, 'TRUE')
+    env.set(SPACK_SHORT_SPEC, pkg.spec.short_spec)
+    env.set(SPACK_DEBUG_LOG_DIR, spack.spack_working_dir)
 
     # Add any pkgconfig directories to PKG_CONFIG_PATH
     pkg_config_dirs = []
@@ -173,7 +174,7 @@ def set_build_environment_variables(pkg, env):
             pcdir = join_path(p, libdir, 'pkgconfig')
             if os.path.isdir(pcdir):
                 pkg_config_dirs.append(pcdir)
-    env.set_env('PKG_CONFIG_PATH', concatenate_paths(pkg_config_dirs))
+    env.set_path('PKG_CONFIG_PATH', pkg_config_dirs)
 
     return env
 
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 74aef57fe8..72aafa4e2d 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -29,6 +29,12 @@ class UnsetEnv(NameModifier):
         os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
 
 
+class SetPath(NameValueModifier):
+    def execute(self):
+        string_path = concatenate_paths(self.value)
+        os.environ[self.name] = string_path
+
+
 class AppendPath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
@@ -103,7 +109,7 @@ class EnvironmentModifications(object):
         }
         return args
 
-    def set_env(self, name, value, **kwargs):
+    def set(self, name, value, **kwargs):
         """
         Stores in the current object a request to set an environment variable
 
@@ -115,7 +121,7 @@ class EnvironmentModifications(object):
         item = SetEnv(name, value, **kwargs)
         self.env_modifications.append(item)
 
-    def unset_env(self, name, **kwargs):
+    def unset(self, name, **kwargs):
         """
         Stores in the current object a request to unset an environment variable
 
@@ -126,6 +132,18 @@ class EnvironmentModifications(object):
         item = UnsetEnv(name, **kwargs)
         self.env_modifications.append(item)
 
+    def set_path(self, name, elts, **kwargs):
+        """
+        Stores a request to set a path generated from a list.
+
+        Args:
+            name: name o the environment variable to be set.
+            elts: elements of the path to set.
+        """
+        kwargs.update(self._get_outside_caller_attributes())
+        item = SetPath(name, elts, **kwargs)
+        self.env_modifications.append(item)
+
     def append_path(self, name, path, **kwargs):
         """
         Stores in the current object a request to append a path to a path list
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index 3e03760c01..6c8f5ea43c 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -11,21 +11,27 @@ class EnvironmentTest(unittest.TestCase):
         os.environ['PATH_LIST'] = '/path/second:/path/third'
         os.environ['REMOVE_PATH_LIST'] = '/a/b:/duplicate:/a/c:/remove/this:/a/d:/duplicate/:/f/g'
 
-    def test_set_env(self):
+    def test_set(self):
         env = EnvironmentModifications()
-        env.set_env('A', 'dummy value')
-        env.set_env('B', 3)
+        env.set('A', 'dummy value')
+        env.set('B', 3)
         env.apply_modifications()
         self.assertEqual('dummy value', os.environ['A'])
         self.assertEqual(str(3), os.environ['B'])
 
-    def test_unset_env(self):
+    def test_unset(self):
         env = EnvironmentModifications()
         self.assertEqual('foo', os.environ['UNSET_ME'])
-        env.unset_env('UNSET_ME')
+        env.unset('UNSET_ME')
         env.apply_modifications()
         self.assertRaises(KeyError, os.environ.__getitem__, 'UNSET_ME')
 
+    def test_set_path(self):
+        env = EnvironmentModifications()
+        env.set_path('A', ['foo', 'bar', 'baz'])
+        env.apply_modifications()
+        self.assertEqual('foo:bar:baz', os.environ['A'])
+
     def test_path_manipulation(self):
         env = EnvironmentModifications()
 
@@ -51,7 +57,7 @@ class EnvironmentTest(unittest.TestCase):
 
     def test_extra_arguments(self):
         env = EnvironmentModifications()
-        env.set_env('A', 'dummy value', who='Pkg1')
+        env.set('A', 'dummy value', who='Pkg1')
         for x in env:
             assert 'who' in x.args
         env.apply_modifications()
@@ -59,8 +65,8 @@ class EnvironmentTest(unittest.TestCase):
 
     def test_extend(self):
         env = EnvironmentModifications()
-        env.set_env('A', 'dummy value')
-        env.set_env('B', 3)
+        env.set('A', 'dummy value')
+        env.set('B', 3)
         copy_construct = EnvironmentModifications(env)
         self.assertEqual(len(copy_construct), 2)
         for x, y in zip(env, copy_construct):
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index 90b5d42eab..c4d9940bb7 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -48,11 +48,11 @@ class Mpich(Package):
     provides('mpi@:1.3', when='@1:')
 
     def setup_dependent_environment(self, env, dependent_spec):
-        env.set_env('MPICH_CC', spack_cc)
-        env.set_env('MPICH_CXX', spack_cxx)
-        env.set_env('MPICH_F77', spack_f77)
-        env.set_env('MPICH_F90', spack_f90)
-        env.set_env('MPICH_FC', spack_fc)
+        env.set('MPICH_CC', spack_cc)
+        env.set('MPICH_CXX', spack_cxx)
+        env.set('MPICH_F77', spack_f77)
+        env.set('MPICH_F90', spack_f90)
+        env.set('MPICH_FC', spack_fc)
 
     def setup_dependent_python_module(self, module, spec, dep_spec):
         """For dependencies, make mpicc's use spack wrapper."""
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index c91a13e376..9a127f1812 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -43,10 +43,10 @@ class Openmpi(Package):
 
 
     def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
-        spack_env.set_env('OMPI_CC', spack_cc)
-        spack_env.set_env('OMPI_CXX', spack_cxx)
-        spack_env.set_env('OMPI_FC', spack_fc)
-        spack_env.set_env('OMPI_F77', spack_f77)
+        spack_env.set('OMPI_CC', spack_cc)
+        spack_env.set('OMPI_CXX', spack_cxx)
+        spack_env.set('OMPI_FC', spack_fc)
+        spack_env.set('OMPI_F77', spack_f77)
 
 
     def install(self, spec, prefix):
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 593a27708c..4f55bc803e 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -104,8 +104,8 @@ class Python(Package):
                 python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
 
         pythonpath = ':'.join(python_paths)
-        spack_env.set_env('PYTHONPATH', pythonpath)
-        run_env.set_env('PYTHONPATH', pythonpath)
+        spack_env.set('PYTHONPATH', pythonpath)
+        run_env.set('PYTHONPATH', pythonpath)
 
 
     def modify_module(self, module, spec, ext_spec):
diff --git a/var/spack/repos/builtin/packages/qt/package.py b/var/spack/repos/builtin/packages/qt/package.py
index 039aeb3c31..d08e8e81e1 100644
--- a/var/spack/repos/builtin/packages/qt/package.py
+++ b/var/spack/repos/builtin/packages/qt/package.py
@@ -57,11 +57,11 @@ class Qt(Package):
 
 
     def setup_environment(self, spack_env, env):
-        env.set_env('QTDIR', self.prefix)
+        env.set('QTDIR', self.prefix)
 
 
     def setup_dependent_environment(self, spack_env, run_env, dspec):
-        spack_env.set_env('QTDIR', self.prefix)
+        spack_env.set('QTDIR', self.prefix)
 
 
     def patch(self):
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 39f65f51d2..7ff1898ce9 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -25,9 +25,10 @@ class Ruby(Package):
             if d.package.extends(self.spec):
                 ruby_paths.append(d.prefix)
 
-        spack_env.set_env('GEM_PATH', concatenate_paths(ruby_paths))
+        spack_env.set_path('GEM_PATH', ruby_paths)
+
         # The actual installation path for this gem
-        spack_env.set_env('GEM_HOME', extension_spec.prefix)
+        spack_env.set('GEM_HOME', extension_spec.prefix)
 
     def modify_module(self, module, spec, ext_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME
-- 
cgit v1.2.3-70-g09d2


From a26992ef55fed958c15b45b989fc0a4d57f02251 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Tue, 22 Mar 2016 01:56:16 -0700
Subject: Change from PR #552: rename setup_dependent_python_module ->
 setup_dependent_package

- Fixed in package.py
- Fixed wrong prototypes in packages that use it.
- Fixed build_environment to set module variables properly
  - added hacky fix to ensure spec/package consistency in build processes.
  - Need to think about defensive spec copy done by `Repo.get`.  May be
    time to think about an immutable spec implementation.
---
 lib/spack/spack/build_environment.py               | 52 ++++++++++++++++------
 lib/spack/spack/package.py                         |  2 +-
 var/spack/repos/builtin/packages/mpich/package.py  |  2 +-
 .../builtin/packages/netlib-scalapack/package.py   | 22 ++++-----
 4 files changed, 52 insertions(+), 26 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index fc5b7d6207..119a255a34 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -225,7 +225,7 @@ def set_module_variables_for_package(pkg, module):
     m.spack_cc  = join_path(link_dir, pkg.compiler.link_paths['cc'])
     m.spack_cxx = join_path(link_dir, pkg.compiler.link_paths['cxx'])
     m.spack_f77 = join_path(link_dir, pkg.compiler.link_paths['f77'])
-    m.spack_f90 = join_path(link_dir, pkg.compiler.link_paths['fc'])
+    m.spack_fc  = join_path(link_dir, pkg.compiler.link_paths['fc'])
 
     # Emulate some shell commands for convenience
     m.pwd          = os.getcwd
@@ -270,32 +270,56 @@ def parent_class_modules(cls):
     return result
 
 
+def setup_module_variables_for_dag(pkg):
+    """Set module-scope variables for all packages in the DAG."""
+    for spec in pkg.spec.traverse(order='post'):
+        # If a user makes their own package repo, e.g.
+        # spack.repos.mystuff.libelf.Libelf, and they inherit from
+        # an existing class like spack.repos.original.libelf.Libelf,
+        # then set the module variables for both classes so the
+        # parent class can still use them if it gets called.
+        spkg = spec.package
+        modules = parent_class_modules(spkg.__class__)
+        for mod in modules:
+            set_module_variables_for_package(spkg, mod)
+        set_module_variables_for_package(spkg, spkg.module)
+
+
 def setup_package(pkg):
     """Execute all environment setup routines."""
     spack_env = EnvironmentModifications()
     run_env   = EnvironmentModifications()
 
+    # Before proceeding, ensure that specs and packages are consistent
+    #
+    # This is a confusing behavior due to how packages are
+    # constructed.  `setup_dependent_package` may set attributes on
+    # specs in the DAG for use by other packages' install
+    # method. However, spec.package will look up a package via
+    # spack.repo, which defensively copies specs into packages.  This
+    # code ensures that all packages in the DAG have pieces of the
+    # same spec object at build time.
+    #
+    # This is safe for the build process, b/c the build process is a
+    # throwaway environment, but it is kind of dirty.
+    #
+    # TODO: Think about how to avoid this fix and do something cleaner.
+    for s in pkg.spec.traverse(): s.package.spec = s
+
     set_compiler_environment_variables(pkg, spack_env)
     set_build_environment_variables(pkg, spack_env)
-
-    # If a user makes their own package repo, e.g.
-    # spack.repos.mystuff.libelf.Libelf, and they inherit from
-    # an existing class like spack.repos.original.libelf.Libelf,
-    # then set the module variables for both classes so the
-    # parent class can still use them if it gets called.
-    modules = parent_class_modules(pkg.__class__)
-    for mod in modules:
-        set_module_variables_for_package(pkg, mod)
+    setup_module_variables_for_dag(pkg)
 
     # Allow dependencies to modify the module
-    for dependency_spec in pkg.spec.traverse(root=False):
+    spec = pkg.spec
+    for dependency_spec in spec.traverse(root=False):
         dpkg = dependency_spec.package
-        dpkg.setup_dependent_python_module(pkg.module, pkg.spec)
+        dpkg.setup_dependent_package(pkg.module, spec)
 
     # Allow dependencies to set up environment as well
-    for dependency_spec in pkg.spec.traverse(root=False):
+    for dependency_spec in spec.traverse(root=False):
         dpkg = dependency_spec.package
-        dpkg.setup_dependent_environment(spack_env, run_env, pkg.spec)
+        dpkg.setup_dependent_environment(spack_env, run_env, spec)
 
     # Allow the package to apply some settings.
     pkg.setup_environment(spack_env, run_env)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 9d8ac87bd7..9af3221837 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1075,7 +1075,7 @@ class Package(object):
         self.setup_environment(spack_env, run_env)
 
 
-    def setup_dependent_python_module(self, module, dependent_spec):
+    def setup_dependent_package(self, module, dependent_spec):
         """Set up Python module-scope variables for dependent packages.
 
         Called before the install() method of dependents.
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index c4d9940bb7..b20dc8dd60 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -54,7 +54,7 @@ class Mpich(Package):
         env.set('MPICH_F90', spack_f90)
         env.set('MPICH_FC', spack_fc)
 
-    def setup_dependent_python_module(self, module, spec, dep_spec):
+    def setup_dependent_package(self, module, dep_spec):
         """For dependencies, make mpicc's use spack wrapper."""
         # FIXME : is this necessary ? Shouldn't this be part of a contract with MPI providers?
         module.mpicc = join_path(self.prefix.bin, 'mpicc')
diff --git a/var/spack/repos/builtin/packages/netlib-scalapack/package.py b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
index 62abfcc48e..c3e6822cdf 100644
--- a/var/spack/repos/builtin/packages/netlib-scalapack/package.py
+++ b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
@@ -1,4 +1,5 @@
 from spack import *
+import sys
 
 class NetlibScalapack(Package):
     """ScaLAPACK is a library of high-performance linear algebra routines for parallel distributed memory machines"""
@@ -11,16 +12,16 @@ class NetlibScalapack(Package):
     version('2.0.0', '9e76ae7b291be27faaad47cfc256cbfe')
     # versions before 2.0.0 are not using cmake and requires blacs as
     # a separated package
-    
+
     variant('shared', default=True, description='Build the shared library version')
     variant('fpic', default=False, description="Build with -fpic compiler option")
-    
+
     provides('scalapack')
-    
+
     depends_on('mpi')
     depends_on('lapack')
-    
-    def install(self, spec, prefix):       
+
+    def install(self, spec, prefix):
         options = [
             "-DBUILD_SHARED_LIBS:BOOL=%s" % ('ON' if '+shared' in spec else 'OFF'),
             "-DBUILD_STATIC_LIBS:BOOL=%s" % ('OFF' if '+shared' in spec else 'ON'),
@@ -40,10 +41,11 @@ class NetlibScalapack(Package):
             make()
             make("install")
 
-    def setup_dependent_python_module(self, module, spec, dependent_spec):
+    def setup_dependent_package(self, module, dependent_spec):
+        spec = self.spec
         lib_dsuffix = '.dylib' if sys.platform == 'darwin' else '.so'
-        lib_suffix = lib_dsuffix if '+shared' in spec['scalapack'] else '.a'
+        lib_suffix = lib_dsuffix if '+shared' in spec else '.a'
 
-        spec['scalapack'].fc_link = '-L%s -lscalapack' % spec['scalapack'].prefix.lib
-        spec['scalapack'].cc_link = spec['scalapack'].fc_link
-        spec['scalapack'].libraries = [join_path(spec['scalapack'].prefix.lib, 'libscalapack%s' % lib_suffix)]
+        spec.fc_link = '-L%s -lscalapack' % spec.prefix.lib
+        spec.cc_link = spec.fc_link
+        spec.libraries = [join_path(spec.prefix.lib, 'libscalapack%s' % lib_suffix)]
-- 
cgit v1.2.3-70-g09d2


From c4134ee71e43a6a2a38822e25cc72ca73efdcfea Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Wed, 23 Mar 2016 00:36:32 -0700
Subject: Fix #608: broken numpy build.

- Failed to catch all instances of modify_module when it was renamed
  to setup_dependent_package.

- Refactored remaining modify_module calls.

- Also modified Python's setup_dependent_package slightly: only
  creates empty site-packages directory for Python extensions now, not
  for all dependents.
---
 lib/spack/spack/modules.py                         |  4 ++--
 lib/spack/spack/preferred_packages.py              |  4 +++-
 var/spack/repos/builtin/packages/python/package.py | 11 ++++++-----
 var/spack/repos/builtin/packages/ruby/package.py   |  6 +++---
 4 files changed, 14 insertions(+), 11 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 05c93cd3e6..8ed98e5d38 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -160,8 +160,8 @@ class EnvModule(object):
         # package-specific modifications
         for extendee in self.pkg.extendees:
             extendee_spec = self.spec[extendee]
-            extendee_spec.package.modify_module(
-                self.pkg.module, extendee_spec, self.spec)
+            extendee_spec.package.setup_dependent_package(
+                self.pkg.module, self.spec)
 
         # Package-specific environment modifications
         spack_env = EnvironmentModifications()
diff --git a/lib/spack/spack/preferred_packages.py b/lib/spack/spack/preferred_packages.py
index 4d8526c75f..f0a5382dc9 100644
--- a/lib/spack/spack/preferred_packages.py
+++ b/lib/spack/spack/preferred_packages.py
@@ -150,7 +150,9 @@ class PreferredPackages(object):
     def version_compare(self, pkgname, a, b):
         """Return less-than-0, 0, or greater than 0 if version a of pkgname is
            respecively less-than, equal-to, or greater-than version b of pkgname.
-           One version is less-than another if it is preferred over the other."""
+           Versions marked 'preferred=True' in package.py take precedence over any
+           versions not marked preferred.
+        """
         return self._spec_compare(pkgname, 'version', a, b, True, None)
 
 
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 4f55bc803e..6d9030805b 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -108,7 +108,7 @@ class Python(Package):
         run_env.set('PYTHONPATH', pythonpath)
 
 
-    def modify_module(self, module, spec, ext_spec):
+    def setup_dependent_package(self, module, ext_spec):
         """
         Called before python modules' install() methods.
 
@@ -118,17 +118,18 @@ class Python(Package):
         """
         # Python extension builds can have a global python executable function
         if self.version >= Version("3.0.0") and self.version < Version("4.0.0"):
-            module.python = Executable(join_path(spec.prefix.bin, 'python3'))
+            module.python = Executable(join_path(self.spec.prefix.bin, 'python3'))
         else:
-            module.python = Executable(join_path(spec.prefix.bin, 'python'))
+            module.python = Executable(join_path(self.spec.prefix.bin, 'python'))
 
         # Add variables for lib/pythonX.Y and lib/pythonX.Y/site-packages dirs.
         module.python_lib_dir     = os.path.join(ext_spec.prefix, self.python_lib_dir)
         module.python_include_dir = os.path.join(ext_spec.prefix, self.python_include_dir)
         module.site_packages_dir  = os.path.join(ext_spec.prefix, self.site_packages_dir)
 
-        # Make the site packages directory if it does not exist already.
-        mkdirp(module.site_packages_dir)
+        # Make the site packages directory for extensions, if it does not exist already.
+        if ext_spec.package.is_extension:
+            mkdirp(module.site_packages_dir)
 
     # ========================================================================
     # Handle specifics of activating and deactivating python modules.
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 7ff1898ce9..e13677e4d2 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -30,7 +30,7 @@ class Ruby(Package):
         # The actual installation path for this gem
         spack_env.set('GEM_HOME', extension_spec.prefix)
 
-    def modify_module(self, module, spec, ext_spec):
+    def setup_dependent_package(self, module, ext_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME
         and GEM_PATH to values appropriate for the package being built.
 
@@ -39,5 +39,5 @@ class Ruby(Package):
             gem('install', '<gem-name>.gem')
         """
         # Ruby extension builds have global ruby and gem functions
-        module.ruby = Executable(join_path(spec.prefix.bin, 'ruby'))
-        module.gem = Executable(join_path(spec.prefix.bin, 'gem'))
+        module.ruby = Executable(join_path(self.spec.prefix.bin, 'ruby'))
+        module.gem = Executable(join_path(self.spec.prefix.bin, 'gem'))
-- 
cgit v1.2.3-70-g09d2


From 38350ae33d29e108803cebdf13b90b6898947328 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Wed, 23 Mar 2016 01:32:54 -0700
Subject: resurrect preferred=True option for packages lost in merge of
 externals support.

- Pyton 2.7.11 is preferred again.
---
 lib/spack/spack/concretize.py         | 4 ++++
 lib/spack/spack/preferred_packages.py | 4 +---
 lib/spack/spack/test/concretize.py    | 9 +++++++++
 3 files changed, 14 insertions(+), 3 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index 2e576743ec..ed9bf79868 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -159,6 +159,10 @@ class DefaultConcretizer(object):
              if any(v.satisfies(sv) for sv in spec.versions)],
             cmp=cmp_versions)
 
+        def prefer_key(v):
+            return pkg.versions.get(Version(v)).get('preferred', False)
+        valid_versions.sort(key=prefer_key, reverse=True)
+
         if valid_versions:
             spec.versions = ver([valid_versions[0]])
         else:
diff --git a/lib/spack/spack/preferred_packages.py b/lib/spack/spack/preferred_packages.py
index f0a5382dc9..4d8526c75f 100644
--- a/lib/spack/spack/preferred_packages.py
+++ b/lib/spack/spack/preferred_packages.py
@@ -150,9 +150,7 @@ class PreferredPackages(object):
     def version_compare(self, pkgname, a, b):
         """Return less-than-0, 0, or greater than 0 if version a of pkgname is
            respecively less-than, equal-to, or greater-than version b of pkgname.
-           Versions marked 'preferred=True' in package.py take precedence over any
-           versions not marked preferred.
-        """
+           One version is less-than another if it is preferred over the other."""
         return self._spec_compare(pkgname, 'version', a, b, True, None)
 
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 08cce09674..9cd8c969ae 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -24,6 +24,7 @@
 ##############################################################################
 import spack
 from spack.spec import Spec, CompilerSpec
+from spack.version import ver
 from spack.concretize import find_spec
 from spack.test.mock_packages_test import *
 
@@ -77,6 +78,14 @@ class ConcretizeTest(MockPackagesTest):
         self.check_concretize('mpich')
 
 
+    def test_concretize_preferred_version(self):
+        spec = self.check_concretize('python')
+        self.assertEqual(spec.versions, ver('2.7.11'))
+
+        spec = self.check_concretize('python@3.5.1')
+        self.assertEqual(spec.versions, ver('3.5.1'))
+
+
     def test_concretize_with_virtual(self):
         self.check_concretize('mpileaks ^mpi')
         self.check_concretize('mpileaks ^mpi@:1.1')
-- 
cgit v1.2.3-70-g09d2


From 0fbdb3f65d234d6ed8f77d3732c134962ad47b35 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 23 Mar 2016 15:43:16 +0100
Subject: modules : added configuration file with disable keyword

---
 lib/spack/spack/config.py  | 28 +++++++++++++++++++++++++---
 lib/spack/spack/modules.py | 12 ++++++++++--
 2 files changed, 35 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 6afd69b3ac..6ef79c70b1 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -237,7 +237,29 @@ section_schemas = {
                                 'type' : 'object',
                                 'default' : {},
                             }
-                        },},},},},}
+                        },},},},},},
+    'modules': {
+        '$schema': 'http://json-schema.org/schema#',
+        'title': 'Spack module file configuration file schema',
+        'type': 'object',
+        'additionalProperties': False,
+        'patternProperties': {
+            r'modules:?': {
+                'type': 'object',
+                'default': {},
+                'additionalProperties': False,
+                'properties': {
+                    'disable': {
+                        'type': 'array',
+                        'default': [],
+                        'items': {
+                            'type': 'string'
+                        }
+                    }
+                }
+            },
+        },
+    },
 }
 
 """OrderedDict of config scopes keyed by name.
@@ -405,11 +427,11 @@ def _read_config_file(filename, schema):
             validate_section(data, schema)
         return data
 
-    except MarkedYAMLError, e:
+    except MarkedYAMLError as e:
         raise ConfigFileError(
             "Error parsing yaml%s: %s" % (str(e.context_mark), e.problem))
 
-    except IOError, e:
+    except IOError as e:
         raise ConfigFileError(
             "Error reading configuration file %s: %s" % (filename, str(e)))
 
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 8ed98e5d38..639f1101b8 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -48,6 +48,7 @@ import textwrap
 
 import llnl.util.tty as tty
 import spack
+import spack.config
 from llnl.util.filesystem import join_path, mkdirp
 from spack.environment import *
 
@@ -57,6 +58,13 @@ __all__ = ['EnvModule', 'Dotkit', 'TclModule']
 module_types = {}
 
 
+def read_configuration_file():
+    f = spack.config.get_config('modules')
+    f.setdefault('disable', [])  # Default : disable nothing
+    return f
+
+CONFIGURATION = read_configuration_file()
+
 def print_help():
     """For use by commands to tell user how to activate shell support."""
 
@@ -115,8 +123,8 @@ class EnvModule(object):
     class __metaclass__(type):
         def __init__(cls, name, bases, dict):
             type.__init__(cls, name, bases, dict)
-            if cls.name != 'env_module':
-                module_types[cls.name] = cls
+            if cls.name != 'env_module' and cls.name not in CONFIGURATION['disable']:
+                    module_types[cls.name] = cls
 
     def __init__(self, spec=None):
         self.spec = spec
-- 
cgit v1.2.3-70-g09d2


From d93f2b335d5a5198b943cc6293a1d24d614c979b Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 23 Mar 2016 16:04:36 +0100
Subject: modules : fixed annoying indent

---
 lib/spack/spack/modules.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 639f1101b8..d354c8bb71 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -124,7 +124,7 @@ class EnvModule(object):
         def __init__(cls, name, bases, dict):
             type.__init__(cls, name, bases, dict)
             if cls.name != 'env_module' and cls.name not in CONFIGURATION['disable']:
-                    module_types[cls.name] = cls
+                module_types[cls.name] = cls
 
     def __init__(self, spec=None):
         self.spec = spec
-- 
cgit v1.2.3-70-g09d2


From f095e619b985a9271ff96cd469086d4654edf489 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 24 Mar 2016 09:31:28 +0100
Subject: module files configuration : enable/disable logic is now positive

---
 lib/spack/spack/config.py  | 2 +-
 lib/spack/spack/modules.py | 9 ++-------
 2 files changed, 3 insertions(+), 8 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 6ef79c70b1..14e5aaf4fb 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -249,7 +249,7 @@ section_schemas = {
                 'default': {},
                 'additionalProperties': False,
                 'properties': {
-                    'disable': {
+                    'enable': {
                         'type': 'array',
                         'default': [],
                         'items': {
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index d354c8bb71..6c32937c3c 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -57,13 +57,8 @@ __all__ = ['EnvModule', 'Dotkit', 'TclModule']
 # Registry of all types of modules.  Entries created by EnvModule's metaclass
 module_types = {}
 
+CONFIGURATION = spack.config.get_config('modules')
 
-def read_configuration_file():
-    f = spack.config.get_config('modules')
-    f.setdefault('disable', [])  # Default : disable nothing
-    return f
-
-CONFIGURATION = read_configuration_file()
 
 def print_help():
     """For use by commands to tell user how to activate shell support."""
@@ -123,7 +118,7 @@ class EnvModule(object):
     class __metaclass__(type):
         def __init__(cls, name, bases, dict):
             type.__init__(cls, name, bases, dict)
-            if cls.name != 'env_module' and cls.name not in CONFIGURATION['disable']:
+            if cls.name != 'env_module' and cls.name in CONFIGURATION['enable']:
                 module_types[cls.name] = cls
 
     def __init__(self, spec=None):
-- 
cgit v1.2.3-70-g09d2


From 758a9c9096bd6c306f2dba0e40b46544dcec1992 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 23 Mar 2016 11:18:11 +0100
Subject: python extensions : create PYTHONPATH in module files

---
 lib/spack/spack/modules.py                         | 10 +++++-----
 var/spack/repos/builtin/packages/python/package.py |  5 ++++-
 2 files changed, 9 insertions(+), 6 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 6c32937c3c..d45fdde703 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -161,13 +161,13 @@ class EnvModule(object):
 
         # Let the extendee modify their extensions before asking for
         # package-specific modifications
-        for extendee in self.pkg.extendees:
-            extendee_spec = self.spec[extendee]
-            extendee_spec.package.setup_dependent_package(
-                self.pkg.module, self.spec)
+        spack_env = EnvironmentModifications()
+        for item in self.pkg.extendees:
+            package = self.spec[item].package
+            package.setup_dependent_package(self.pkg.module, self.spec)
+            package.setup_dependent_environment(spack_env, env, self.spec)
 
         # Package-specific environment modifications
-        spack_env = EnvironmentModifications()
         self.spec.package.setup_environment(spack_env, env)
 
         # TODO : implement site-specific modifications and filters
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 6d9030805b..f5237c3b57 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -105,7 +105,10 @@ class Python(Package):
 
         pythonpath = ':'.join(python_paths)
         spack_env.set('PYTHONPATH', pythonpath)
-        run_env.set('PYTHONPATH', pythonpath)
+
+        # For run time environment set only the path for extension_spec and prepend it to PYTHONPATH
+        if extension_spec.package.extends(self.spec):
+            run_env.prepend_path('PYTHONPATH', os.path.join(extension_spec.prefix, self.site_packages_dir))
 
 
     def setup_dependent_package(self, module, ext_spec):
-- 
cgit v1.2.3-70-g09d2