Separate setting build environment and run environment in packages (#11115)

* Methods setting the environment now do it separately for build and run

Before this commit the `*_environment` methods were setting
modifications to both the build-time and run-time environment
simultaneously. This might cause issues as the two environments
inherently rely on different preconditions:

1. The build-time environment is set before building a package, thus
the package prefix doesn't exist and can't be inspected

2. The run-time environment instead is set assuming the target package
has been already installed

Here we split each of these functions into two: one setting the
build-time environment, one the run-time.

We also adopt a fallback strategy that inspects for old methods and
executes them as before, but prints a deprecation warning to tty. This
permits to port packages to use the new methods in a distributed way,
rather than having to modify all the packages at once.

* Added a test that fails if any package uses the old API

Marked the test xfail for now as we have a lot of packages in that
state.

* Added a test to check that a package modified by a PR is up to date

This test can be used any time we deprecate a method call to ensure
that during the first modification of the package we update also
the deprecated calls.

* Updated documentation

6 files changed, 56 insertions, 53 deletions

diff --git a/var/spack/repos/builtin/packages/docbook-xml/package.py b/var/spack/repos/builtin/packages/docbook-xml/package.py
index d654f790ea..0a24a5d2b6 100644
--- a/var/spack/repos/builtin/packages/docbook-xml/package.py
+++ b/var/spack/repos/builtin/packages/docbook-xml/package.py
@@ -20,6 +20,6 @@ class DocbookXml(Package):
     def install(self, spec, prefix):
         install_tree('.', prefix)
 
-    def setup_environment(self, spack_env, run_env):
+    def setup_run_environment(self, env):
         catalog = os.path.join(self.prefix, 'catalog.xml')
-        run_env.set('XML_CATALOG_FILES', catalog, separator=' ')
+        env.set('XML_CATALOG_FILES', catalog, separator=' ')
diff --git a/var/spack/repos/builtin/packages/elpa/package.py b/var/spack/repos/builtin/packages/elpa/package.py
index 19fdf7a383..5633594c8f 100644
--- a/var/spack/repos/builtin/packages/elpa/package.py
+++ b/var/spack/repos/builtin/packages/elpa/package.py
@@ -61,16 +61,16 @@ class Elpa(AutotoolsPackage):
 
     build_directory = 'spack-build'
 
-    def setup_environment(self, spack_env, run_env):
+    def setup_build_environment(self, env):
         spec = self.spec
 
-        spack_env.set('CC', spec['mpi'].mpicc)
-        spack_env.set('FC', spec['mpi'].mpifc)
-        spack_env.set('CXX', spec['mpi'].mpicxx)
+        env.set('CC', spec['mpi'].mpicc)
+        env.set('FC', spec['mpi'].mpifc)
+        env.set('CXX', spec['mpi'].mpicxx)
 
-        spack_env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags)
-        spack_env.append_flags('LIBS', spec['lapack'].libs.link_flags)
-        spack_env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined())
+        env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags)
+        env.append_flags('LIBS', spec['lapack'].libs.link_flags)
+        env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined())
 
     def configure_args(self):
         # TODO: set optimum flags for platform+compiler combo, see
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index 5b92fbafee..5d7a74cb59 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -110,29 +110,28 @@ spack package at this time.''',
     conflicts('pmi=pmi2', when='device=ch3 netmod=ofi')
     conflicts('pmi=pmix', when='device=ch3')
 
-    def setup_environment(self, spack_env, run_env):
-        # mpich configure fails when F90 and F90FLAGS are set
-        spack_env.unset('F90')
-        spack_env.unset('F90FLAGS')
+    def setup_build_environment(self, env):
+        env.unset('F90')
+        env.unset('F90FLAGS')
 
-    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
+    def setup_dependent_build_environment(self, env, dependent_spec):
         # On Cray, the regular compiler wrappers *are* the MPI wrappers.
         if 'platform=cray' in self.spec:
-            spack_env.set('MPICC',  spack_cc)
-            spack_env.set('MPICXX', spack_cxx)
-            spack_env.set('MPIF77', spack_fc)
-            spack_env.set('MPIF90', spack_fc)
+            env.set('MPICC', spack_cc)
+            env.set('MPICXX', spack_cxx)
+            env.set('MPIF77', spack_fc)
+            env.set('MPIF90', spack_fc)
         else:
-            spack_env.set('MPICC',  join_path(self.prefix.bin, 'mpicc'))
-            spack_env.set('MPICXX', join_path(self.prefix.bin, 'mpic++'))
-            spack_env.set('MPIF77', join_path(self.prefix.bin, 'mpif77'))
-            spack_env.set('MPIF90', join_path(self.prefix.bin, 'mpif90'))
-
-        spack_env.set('MPICH_CC', spack_cc)
-        spack_env.set('MPICH_CXX', spack_cxx)
-        spack_env.set('MPICH_F77', spack_f77)
-        spack_env.set('MPICH_F90', spack_fc)
-        spack_env.set('MPICH_FC', spack_fc)
+            env.set('MPICC', join_path(self.prefix.bin, 'mpicc'))
+            env.set('MPICXX', join_path(self.prefix.bin, 'mpic++'))
+            env.set('MPIF77', join_path(self.prefix.bin, 'mpif77'))
+            env.set('MPIF90', join_path(self.prefix.bin, 'mpif90'))
+
+        env.set('MPICH_CC', spack_cc)
+        env.set('MPICH_CXX', spack_cxx)
+        env.set('MPICH_F77', spack_f77)
+        env.set('MPICH_F90', spack_fc)
+        env.set('MPICH_FC', spack_fc)
 
     def setup_dependent_package(self, module, dependent_spec):
         if 'platform=cray' in self.spec:
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index d353761181..b05bd6dd9b 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -183,7 +183,7 @@ class Python(AutotoolsPackage):
             r'\1setup.py\2 --no-user-cfg \3\6'
         )
 
-    def setup_environment(self, spack_env, run_env):
+    def setup_build_environment(self, env):
         spec = self.spec
 
         # TODO: The '--no-user-cfg' option for Python installation is only in
@@ -195,7 +195,7 @@ class Python(AutotoolsPackage):
                       'user configurations are present.').format(self.version))
 
         # Need this to allow python build to find the Python installation.
-        spack_env.set('MACOSX_DEPLOYMENT_TARGET', platform.mac_ver()[0])
+        env.set('MACOSX_DEPLOYMENT_TARGET', platform.mac_ver()[0])
 
     def configure_args(self):
         spec = self.spec
@@ -672,7 +672,7 @@ class Python(AutotoolsPackage):
     def easy_install_file(self):
         return join_path(self.site_packages_dir, "easy-install.pth")
 
-    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
+    def setup_dependent_build_environment(self, env, dependent_spec):
         """Set PYTHONPATH to include the site-packages directory for the
         extension and any other python extensions it depends on."""
 
@@ -680,11 +680,11 @@ class Python(AutotoolsPackage):
         # python is found in the build environment. This to prevent cases
         # where a system provided python is run against the standard libraries
         # of a Spack built python. See issue #7128
-        spack_env.set('PYTHONHOME', self.home)
+        env.set('PYTHONHOME', self.home)
 
         path = os.path.dirname(self.command.path)
         if not is_system_path(path):
-            spack_env.prepend_path('PATH', path)
+            env.prepend_path('PATH', path)
 
         python_paths = []
         for d in dependent_spec.traverse(
@@ -694,12 +694,13 @@ class Python(AutotoolsPackage):
                                               self.site_packages_dir))
 
         pythonpath = ':'.join(python_paths)
-        spack_env.set('PYTHONPATH', pythonpath)
+        env.set('PYTHONPATH', pythonpath)
 
+    def setup_dependent_run_environment(self, env, dependent_spec):
         # For run time environment set only the path for
         # dependent_spec and prepend it to PYTHONPATH
         if dependent_spec.package.extends(self.spec):
-            run_env.prepend_path('PYTHONPATH', join_path(
+            env.prepend_path('PYTHONPATH', join_path(
                 dependent_spec.prefix, self.site_packages_dir))
 
     def setup_dependent_package(self, module, dependent_spec):
diff --git a/var/spack/repos/builtin/packages/qt/package.py b/var/spack/repos/builtin/packages/qt/package.py
index 7a4232ee22..0405aa4902 100644
--- a/var/spack/repos/builtin/packages/qt/package.py
+++ b/var/spack/repos/builtin/packages/qt/package.py
@@ -206,12 +206,14 @@ class Qt(Package):
 
         return url
 
-    def setup_environment(self, spack_env, run_env):
-        spack_env.set('MAKEFLAGS', '-j{0}'.format(make_jobs))
-        run_env.set('QTDIR', self.prefix)
+    def setup_build_environment(self, env):
+        env.set('MAKEFLAGS', '-j{0}'.format(make_jobs))
 
-    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
-        spack_env.set('QTDIR', self.prefix)
+    def setup_run_environment(self, env):
+        env.set('QTDIR', self.prefix)
+
+    def setup_dependent_build_environment(self, env, dependent_spec):
+        env.set('QTDIR', self.prefix)
 
     def setup_dependent_package(self, module, dependent_spec):
         module.qmake = Executable(join_path(self.spec.prefix.bin, 'qmake'))
diff --git a/var/spack/repos/builtin/packages/r/package.py b/var/spack/repos/builtin/packages/r/package.py
index 6d68e72bc3..3755fb594f 100644
--- a/var/spack/repos/builtin/packages/r/package.py
+++ b/var/spack/repos/builtin/packages/r/package.py
@@ -168,7 +168,7 @@ class R(AutotoolsPackage):
     def r_lib_dir(self):
         return join_path('rlib', 'R', 'library')
 
-    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
+    def setup_dependent_build_environment(self, env, dependent_spec):
         # Set R_LIBS to include the library dir for the
         # extension and any other R extensions it depends on.
         r_libs_path = []
@@ -178,27 +178,28 @@ class R(AutotoolsPackage):
                 r_libs_path.append(join_path(d.prefix, self.r_lib_dir))
 
         r_libs_path = ':'.join(r_libs_path)
-        spack_env.set('R_LIBS', r_libs_path)
-        spack_env.set('R_MAKEVARS_SITE',
-                      join_path(self.etcdir, 'Makeconf.spack'))
+        env.set('R_LIBS', r_libs_path)
+        env.set('R_MAKEVARS_SITE',
+                join_path(self.etcdir, 'Makeconf.spack'))
 
         # Use the number of make_jobs set in spack. The make program will
         # determine how many jobs can actually be started.
-        spack_env.set('MAKEFLAGS', '-j{0}'.format(make_jobs))
+        env.set('MAKEFLAGS', '-j{0}'.format(make_jobs))
 
+    def setup_dependent_run_environment(self, env, dependent_spec):
         # For run time environment set only the path for dependent_spec and
         # prepend it to R_LIBS
         if dependent_spec.package.extends(self.spec):
-            run_env.prepend_path('R_LIBS', join_path(
+            env.prepend_path('R_LIBS', join_path(
                 dependent_spec.prefix, self.r_lib_dir))
 
-    def setup_environment(self, spack_env, run_env):
-        run_env.prepend_path('LIBRARY_PATH',
-                             join_path(self.prefix, 'rlib', 'R', 'lib'))
-        run_env.prepend_path('LD_LIBRARY_PATH',
-                             join_path(self.prefix, 'rlib', 'R', 'lib'))
-        run_env.prepend_path('CPATH',
-                             join_path(self.prefix, 'rlib', 'R', 'include'))
+    def setup_run_environment(self, env):
+        env.prepend_path('LIBRARY_PATH',
+                         join_path(self.prefix, 'rlib', 'R', 'lib'))
+        env.prepend_path('LD_LIBRARY_PATH',
+                         join_path(self.prefix, 'rlib', 'R', 'lib'))
+        env.prepend_path('CPATH',
+                         join_path(self.prefix, 'rlib', 'R', 'include'))
 
     def setup_dependent_package(self, module, dependent_spec):
         """Called before R modules' install() methods. In most cases,