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

8 files changed, 275 insertions, 160 deletions

diff --git a/lib/spack/docs/module_file_support.rst b/lib/spack/docs/module_file_support.rst
index 7ce2398af2..1b5de3a0e8 100644
--- a/lib/spack/docs/module_file_support.rst
+++ b/lib/spack/docs/module_file_support.rst
@@ -303,8 +303,7 @@ content of the module files generated by Spack. The first one:
 
 .. code-block:: python
 
-   def setup_environment(self, spack_env, run_env):
-       """Set up the compile and runtime environments for a package."""
+   def setup_run_environment(self, env):
        pass
 
 can alter the content of the module file associated with the same package where it is overridden.
@@ -312,16 +311,15 @@ The second method:
 
 .. code-block:: python
 
-   def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
-       """Set up the environment of packages that depend on this one"""
+   def setup_dependent_run_environment(self, env, dependent_spec):
        pass
 
 can instead inject run-time environment modifications in the module files of packages
 that depend on it. In both cases you need to fill ``run_env`` with the desired
 list of environment modifications.
 
-.. note::
- The ``r`` package and callback APIs
+.. admonition:: The ``r`` package and callback APIs
+
   An example in which it is crucial to override both methods
   is given by the ``r`` package. This package installs libraries and headers
   in non-standard locations and it is possible to prepend the appropriate directory
@@ -336,14 +334,14 @@ list of environment modifications.
   with the following snippet:
 
   .. literalinclude:: _spack_root/var/spack/repos/builtin/packages/r/package.py
-     :pyobject: R.setup_environment
+     :pyobject: R.setup_run_environment
 
   The ``r`` package also knows which environment variable should be modified
   to make language extensions provided by other packages available, and modifies
   it appropriately in the override of the second method:
 
   .. literalinclude:: _spack_root/var/spack/repos/builtin/packages/r/package.py
-     :pyobject: R.setup_dependent_environment
+     :pyobject: R.setup_dependent_run_environment
 
 .. _modules-yaml:
 
diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index 5adb996232..767a744baa 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -2032,55 +2032,58 @@ appear in the package file (or in this case, in the list).
 
 .. _setup-dependent-environment:
 
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-``setup_dependent_environment()``
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-Spack provides a mechanism for dependencies to provide variables that
-can be used in their dependents' build.  Any package can declare a
-``setup_dependent_environment()`` function, and this function will be
-called before the ``install()`` method of any dependent packages.
-This allows dependencies to set up environment variables and other
-properties to be used by dependents.
-
-The function declaration should look like this:
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Influence how dependents are built or run
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Spack provides a mechanism for dependencies to influence the
+environment of their dependents by overriding  the
+:meth:`setup_dependent_run_environment <spack.package.PackageBase.setup_dependent_run_environment>`
+or the
+:meth:`setup_dependent_build_environment <spack.package.PackageBase.setup_dependent_build_environment>`
+methods.
+The Qt package, for instance, uses this call:
 
 .. literalinclude:: _spack_root/var/spack/repos/builtin/packages/qt/package.py
-   :pyobject: Qt.setup_dependent_environment
+   :pyobject: Qt.setup_dependent_build_environment
    :linenos:
 
-Here, the Qt package sets the ``QTDIR`` environment variable so that
-packages that depend on a particular Qt installation will find it.
-
-The arguments to this function are:
+to set the ``QTDIR`` environment variable so that packages
+that depend on a particular Qt installation will find it.
+Another good example of how a dependency can influence
+the build environment of dependents is the Python package:
 
-* **spack_env**: List of environment modifications to be applied when
-  the dependent package is built within Spack.
-* **run_env**: List of environment modifications to be applied when
-  the dependent package is run outside of Spack. These are added to the
-  resulting module file.
-* **dependent_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``.
+.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py
+   :pyobject: Python.setup_dependent_build_environment
+   :linenos:
 
-A good example of using these is in the Python package:
+In the method above it is ensured that any package that depends on Python
+will have the ``PYTHONPATH``, ``PYTHONHOME`` and ``PATH`` environment
+variables set appropriately before starting the installation. To make things
+even simpler the ``python setup.py`` command is also inserted into the module
+scope of dependents by overriding a third method called
+:meth:`setup_dependent_package <spack.package.PackageBase.setup_dependent_package>`
+:
 
 .. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py
-   :pyobject: Python.setup_dependent_environment
+   :pyobject: Python.setup_dependent_package
    :linenos:
 
-The first thing that happens here is that the ``python`` command is
-inserted into module scope of the dependent.  This allows most python
-packages to have a very simple install method, like this:
+This allows most python packages to have a very simple install procedure,
+like the following:
 
 .. code-block:: python
 
    def install(self, spec, prefix):
-       python('setup.py', 'install', '--prefix={0}'.format(prefix))
+       setup_py('install', '--prefix={0}'.format(prefix))
+
+Finally the Python package takes also care of the modifications to ``PYTHONPATH``
+to allow dependencies to run correctly:
+
+.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py
+    :pyobject: Python.setup_dependent_run_environment
+    :linenos:
 
-Python's ``setup_dependent_environment`` method also sets up some
-other variables, creates a directory, and sets up the ``PYTHONPATH``
-so that dependent packages can find their dependencies at build time.
 
 .. _packaging_conflicts:
 
diff --git a/lib/spack/docs/tutorial_advanced_packaging.rst b/lib/spack/docs/tutorial_advanced_packaging.rst
index bd90f724a7..876a6e8f70 100644
--- a/lib/spack/docs/tutorial_advanced_packaging.rst
+++ b/lib/spack/docs/tutorial_advanced_packaging.rst
@@ -90,21 +90,23 @@ like py-numpy, Spack's ``python`` package will add it to ``PYTHONPATH``
 so it is available at build time; this is required because the default setup
 that spack does is not sufficient for python to import modules.
 
-To provide environment setup for a dependent, a package can implement the
-:py:func:`setup_dependent_environment <spack.package.PackageBase.setup_dependent_environment>`
-function. This function takes as a parameter a :py:class:`EnvironmentModifications <spack.util.environment.EnvironmentModifications>`
+Any package can override the
+:py:func:`setup_dependent_build_environment <spack.package.PackageBase.setup_dependent_build_environment>`
+method to setup the build environment for a dependent.
+This method takes as an argument a :py:class:`EnvironmentModifications <spack.util.environment.EnvironmentModifications>`
 object which includes convenience methods to update the environment. For
 example, an MPI implementation can set ``MPICC`` for packages that depend on it:
 
 .. code-block:: python
 
-  def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
-      spack_env.set('MPICC', join_path(self.prefix.bin, 'mpicc'))
+  def setup_dependent_build_environment(self, env, dependent_spec):
+      env.set('MPICC', join_path(self.prefix.bin, 'mpicc'))
 
 In this case packages that depend on ``mpi`` will have ``MPICC`` defined in
-their environment when they build. This section is focused on modifying the
-build-time environment represented by ``spack_env``, but it's worth noting that
-modifications to ``run_env`` are included in Spack's automatically-generated
+their environment when they build. This section is focused on setting up the
+build-time environment but it's worth noting that a similar method called
+:py:func:`setup_dependent_run_environment <spack.package.PackageBase.setup_dependent_run_environment>`
+can be used to code modifications that will be included in Spack's automatically-generated
 module files.
 
 We can practice by editing the ``mpich`` package to set the ``MPICC``
@@ -118,17 +120,17 @@ Once you're finished, the method should look like this:
 
 .. code-block:: python
 
-  def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
-      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'))
+  def setup_dependent_build_environment(self, env, dependent_spec):
+      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'))
 
-      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('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)
 
 At this point we can, for instance, install ``netlib-scalapack`` with
 ``mpich``:
@@ -155,25 +157,32 @@ set to the correct value.
 Set environment variables in your own package
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-Packages can modify their own build-time environment by implementing the
-:py:func:`setup_environment <spack.package.PackageBase.setup_environment>` function.
-For ``qt`` this looks like:
+Packages can override the
+:py:func:`setup_build_environment <spack.package.PackageBase.setup_build_environment>`
+or the
+:py:func:`setup_run_environment <spack.package.PackageBase.setup_run_environment>`
+methods to modify their own build-time or run-time environment respectively.
+An example of a package that overrides both methods is ``qt``:
 
 .. code-block:: python
 
-    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))
 
-When ``qt`` builds, ``MAKEFLAGS`` will be defined in the environment.
+    def setup_run_environment(self, env):
+        env.set('QTDIR', self.prefix)
 
-To contrast with ``qt``'s :py:func:`setup_dependent_environment <spack.package.PackageBase.setup_dependent_environment>`
+When ``qt`` builds, ``MAKEFLAGS`` will be defined in the environment. Likewise, when a
+module file is created for ``qt`` it will contain commands to define ``QTDIR`` appropriately.
+
+To contrast with ``qt``'s
+:py:func:`setup_dependent_build_environment <spack.package.PackageBase.setup_dependent_build_environment>`
 function:
 
 .. code-block:: python
 
-    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
-        spack_env.set('QTDIR', self.prefix)
+    def setup_dependent_build_environment(self, env, dependent_spec):
+        env.set('QTDIR', self.prefix)
 
 Let's see how it works by completing the ``elpa`` package:
 
@@ -185,16 +194,16 @@ In the end your method should look like:
 
 .. code-block:: python
 
-  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)
-      spack_env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined())
+      env.set('CC', spec['mpi'].mpicc)
+      env.set('FC', spec['mpi'].mpifc)
+      env.set('CXX', spec['mpi'].mpicxx)
+      env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined())
 
-      spack_env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags)
-      spack_env.append_flags('LIBS', spec['lapack'].libs.link_flags)
+      env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags)
+      env.append_flags('LIBS', spec['lapack'].libs.link_flags)
 
 At this point it's possible to proceed with the installation of ``elpa ^mpich``
 
diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 3702a989c3..db99a7a4c1 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -673,35 +673,26 @@ def load_external_modules(pkg):
 
 def setup_package(pkg, dirty):
     """Execute all environment setup routines."""
-    spack_env = EnvironmentModifications()
-    run_env = EnvironmentModifications()
+    build_env = EnvironmentModifications()
 
     if not dirty:
         clean_environment()
 
-    set_compiler_environment_variables(pkg, spack_env)
-    set_build_environment_variables(pkg, spack_env, dirty)
-    pkg.architecture.platform.setup_platform_environment(pkg, spack_env)
+    set_compiler_environment_variables(pkg, build_env)
+    set_build_environment_variables(pkg, build_env, dirty)
+    pkg.architecture.platform.setup_platform_environment(pkg, build_env)
 
-    # traverse in postorder so package can use vars from its dependencies
-    spec = pkg.spec
-    for dspec in pkg.spec.traverse(order='post', root=False,
-                                   deptype=('build', 'test')):
-        spkg = dspec.package
-        set_module_variables_for_package(spkg)
-
-        # Allow dependencies to modify the module
-        dpkg = dspec.package
-        dpkg.setup_dependent_package(pkg.module, spec)
-        dpkg.setup_dependent_environment(spack_env, run_env, spec)
+    build_env.extend(
+        modifications_from_dependencies(pkg.spec, context='build')
+    )
 
-    if (not dirty) and (not spack_env.is_unset('CPATH')):
+    if (not dirty) and (not build_env.is_unset('CPATH')):
         tty.debug("A dependency has updated CPATH, this may lead pkg-config"
                   " to assume that the package is part of the system"
                   " includes and omit it when invoked with '--cflags'.")
 
     set_module_variables_for_package(pkg)
-    pkg.setup_environment(spack_env, run_env)
+    pkg.setup_build_environment(build_env)
 
     # Loading modules, in particular if they are meant to be used outside
     # of Spack, can change environment variables that are relevant to the
@@ -711,7 +702,7 @@ def setup_package(pkg, dirty):
     # unnecessary. Modules affecting these variables will be overwritten anyway
     with preserve_environment('CC', 'CXX', 'FC', 'F77'):
         # All module loads that otherwise would belong in previous
-        # functions have to occur after the spack_env object has its
+        # functions have to occur after the build_env object has its
         # modifications applied. Otherwise the environment modifications
         # could undo module changes, such as unsetting LD_LIBRARY_PATH
         # after a module changes it.
@@ -727,8 +718,39 @@ def setup_package(pkg, dirty):
         load_external_modules(pkg)
 
     # Make sure nothing's strange about the Spack environment.
-    validate(spack_env, tty.warn)
-    spack_env.apply_modifications()
+    validate(build_env, tty.warn)
+    build_env.apply_modifications()
+
+
+def modifications_from_dependencies(spec, context):
+    """Returns the environment modifications that are required by
+    the dependencies of a spec and also applies modifications
+    to this spec's package at module scope, if need be.
+
+    Args:
+        spec (Spec): spec for which we want the modifications
+        context (str): either 'build' for build-time modifications or 'run'
+            for run-time modifications
+    """
+    env = EnvironmentModifications()
+    pkg = spec.package
+
+    # Maps the context to deptype and method to be called
+    deptype_and_method = {
+        'build': (('build', 'link', 'test'),
+                  'setup_dependent_build_environment'),
+        'run': (('link', 'run'), 'setup_dependent_run_environment')
+    }
+    deptype, method = deptype_and_method[context]
+
+    for dspec in spec.traverse(order='post', root=False, deptype=deptype):
+        dpkg = dspec.package
+        set_module_variables_for_package(dpkg)
+        # Allow dependencies to modify the module
+        dpkg.setup_dependent_package(pkg.module, spec)
+        getattr(dpkg, method)(env, spec)
+
+    return env
 
 
 def fork(pkg, function, dirty, fake):
diff --git a/lib/spack/spack/cmd/flake8.py b/lib/spack/spack/cmd/flake8.py
index ebcad88a06..61f7dd567f 100644
--- a/lib/spack/spack/cmd/flake8.py
+++ b/lib/spack/spack/cmd/flake8.py
@@ -93,12 +93,11 @@ pattern_exemptions = dict(
     for file_pattern, error_dict in pattern_exemptions.items())
 
 
-def changed_files(args):
+def changed_files(base=None, untracked=True, all_files=False):
     """Get list of changed files in the Spack repository."""
 
     git = which('git', required=True)
 
-    base = args.base
     if base is None:
         base = os.environ.get('TRAVIS_BRANCH', 'develop')
 
@@ -114,11 +113,11 @@ def changed_files(args):
     ]
 
     # Add new files that are untracked
-    if args.untracked:
+    if untracked:
         git_args.append(['ls-files', '--exclude-standard', '--other'])
 
     # add everything if the user asked for it
-    if args.all:
+    if all_files:
         git_args.append(['ls-files', '--exclude-standard'])
 
     excludes = [os.path.realpath(f) for f in exclude_directories]
@@ -246,7 +245,7 @@ def flake8(parser, args):
 
         with working_dir(spack.paths.prefix):
             if not file_list:
-                file_list = changed_files(args)
+                file_list = changed_files(args.base, args.untracked, args.all)
 
         print('=======================================================')
         print('flake8: running flake8 code checks on spack.')
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 116d4f5471..d38ede1385 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -634,23 +634,16 @@ class BaseContext(tengine.Context):
             exclude=spack.util.environment.is_system_path
         )
 
-        # Modifications that are coded at package level
-        _ = spack.util.environment.EnvironmentModifications()
-        # TODO : the code down below is quite similar to
-        # TODO : build_environment.setup_package and needs to be factored out
-        # TODO : to a single place
         # Let the extendee/dependency modify their extensions/dependencies
         # before asking for package-specific modifications
-        for item in dependencies(self.spec, 'all'):
-            package = self.spec[item.name].package
-            build_environment.set_module_variables_for_package(package)
-            package.setup_dependent_package(
-                self.spec.package.module, self.spec
+        env.extend(
+            build_environment.modifications_from_dependencies(
+                self.spec, context='run'
             )
-            package.setup_dependent_environment(_, env, self.spec)
+        )
         # Package specific modifications
         build_environment.set_module_variables_for_package(self.spec.package)
-        self.spec.package.setup_environment(_, env)
+        self.spec.package.setup_run_environment(env)
 
         # Modifications required from modules.yaml
         env.extend(self.conf.env)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index ecff265508..e3d4f1b6e9 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -45,6 +45,7 @@ import spack.mixins
 import spack.multimethod
 import spack.repo
 import spack.url
+import spack.util.environment
 import spack.util.web
 import spack.multimethod
 import spack.binary_distribution as binary_distribution
@@ -1951,68 +1952,108 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)):
         """
         return (None, None, flags)
 
-    def setup_environment(self, spack_env, run_env):
-        """Set up the compile and runtime environments for a package.
+    def _get_legacy_environment_method(self, method_name):
+        legacy_fn = getattr(self, method_name, None)
+        name_prefix = method_name.split('_environment')[0]
+        if legacy_fn:
+            msg = '[DEPRECATED METHOD]\n"{0}" ' \
+                  'still defines the deprecated method "{1}" ' \
+                  '[should be split into "{2}_build_environment" and ' \
+                  '"{2}_run_environment"]'
+            tty.debug(msg.format(self.name, method_name, name_prefix))
+        return legacy_fn
 
-        ``spack_env`` and ``run_env`` are ``EnvironmentModifications``
-        objects. Package authors can call methods on them to alter
-        the environment within Spack and at runtime.
+    def setup_build_environment(self, env):
+        """Sets up the build environment for a package.
 
-        Both ``spack_env`` and ``run_env`` are applied within the build
-        process, before this package's ``install()`` method is called.
+        This method will be called before the current package prefix exists in
+        Spack's store.
 
-        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.
-
-        Example:
+        Args:
+            env (EnvironmentModifications): environment modifications to be
+                applied when the package is built. Package authors can call
+                methods on it to alter the build environment.
+        """
+        legacy_fn = self._get_legacy_environment_method('setup_environment')
+        if legacy_fn:
+            _ = spack.util.environment.EnvironmentModifications()
+            legacy_fn(env, _)
 
-        1. Qt extensions need ``QTDIR`` set.
+    def setup_run_environment(self, env):
+        """Sets up the run environment for a package.
 
         Args:
-            spack_env (EnvironmentModifications): List of environment
-                modifications to be applied when this package is built
-                within Spack.
-            run_env (EnvironmentModifications): List of environment
-                modifications to be applied when this package is run outside
-                of Spack. These are added to the resulting module file.
+            env (EnvironmentModifications): environment modifications to be
+                applied when the package is run. Package authors can call
+                methods on it to alter the run environment.
         """
-        pass
+        legacy_fn = self._get_legacy_environment_method('setup_environment')
+        if legacy_fn:
+            _ = spack.util.environment.EnvironmentModifications()
+            legacy_fn(_, env)
 
-    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
-        """Set up the environment of packages that depend on this one.
+    def setup_dependent_build_environment(self, env, dependent_spec):
+        """Sets up the build 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 similar to ``setup_build_environment``, but it is used to
+        modify the build environments of packages that *depend* on this one.
 
-        This is useful if there are some common steps to installing
-        all extensions for a certain package.
+        This gives packages like Python and others that follow the extension
+        model a way to implement common environment or compile-time settings
+        for dependencies.
 
-        Example:
+        This method will be called before the dependent package prefix exists
+        in Spack's store.
 
-        1. Installing python modules generally requires ``PYTHONPATH`` to point
-           to the ``lib/pythonX.Y/site-packages`` directory in the module's
-           install prefix. This method could be used to set that variable.
+        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 method could be used to set that
+            variable.
 
         Args:
-            spack_env (EnvironmentModifications): List of environment
-                modifications to be applied when the dependent package is
-                built within Spack.
-            run_env (EnvironmentModifications): List of environment
-                modifications to be applied when the dependent package is
-                run outside of Spack. These are added to the resulting
-                module file.
-            dependent_spec (Spec): The spec of the dependent package
+            env (EnvironmentModifications): environment modifications to be
+                applied when the dependent package is built. Package authors
+                can call methods on it to alter the build environment.
+
+            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``.
+                available as ``self.spec``
         """
-        pass
+        legacy_fn = self._get_legacy_environment_method(
+            'setup_dependent_environment'
+        )
+        if legacy_fn:
+            _ = spack.util.environment.EnvironmentModifications()
+            legacy_fn(env, _, dependent_spec)
+
+    def setup_dependent_run_environment(self, env, dependent_spec):
+        """Sets up the run environment of packages that depend on this one.
+
+        This is similar to ``setup_run_environment``, but it is used to
+        modify the run 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 run-time settings
+        for dependencies.
+
+        Args:
+            env (EnvironmentModifications): environment modifications to be
+                applied when the dependent package is run. Package authors
+                can call methods on it to alter the build environment.
+
+            dependent_spec (Spec): The spec of the dependent package
+                about to be run. This allows the extendee (self) to query
+                the dependent's state. Note that *this* package's spec is
+                available as ``self.spec``
+        """
+        legacy_fn = self._get_legacy_environment_method(
+            'setup_dependent_environment'
+        )
+        if legacy_fn:
+            _ = spack.util.environment.EnvironmentModifications()
+            legacy_fn(_, env, dependent_spec)
 
     def setup_dependent_package(self, module, dependent_spec):
         """Set up Python module-scope variables for dependent packages.
diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py
index 1dd96dccec..f83a2dbb8e 100644
--- a/lib/spack/spack/test/package_sanity.py
+++ b/lib/spack/spack/test/package_sanity.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 """This test does sanity checks on Spack's builtin package database."""
+import os.path
 import re
 
 import pytest
@@ -11,6 +12,10 @@ import pytest
 import spack.fetch_strategy
 import spack.paths
 import spack.repo
+import spack.util.executable as executable
+# A few functions from this module are used to
+# do sanity checks only on packagess modified by a PR
+import spack.cmd.flake8 as flake8
 import spack.util.crypto as crypto
 
 
@@ -134,3 +139,48 @@ def test_all_packages_use_sha256_checksums():
                     )
 
     assert [] == errors
+
+
+@pytest.mark.xfail
+def test_api_for_build_and_run_environment():
+    """Ensure that every package uses the correct API to set build and
+    run environment, and not the old one.
+    """
+    failing = []
+    for pkg in spack.repo.path.all_packages():
+        add_to_list = (hasattr(pkg, 'setup_environment') or
+                       hasattr(pkg, 'setup_dependent_environment'))
+        if add_to_list:
+            failing.append(pkg)
+
+    msg = ('there are {0} packages using the old API to set build '
+           'and run environment [{1}]')
+    assert not failing, msg.format(
+        len(failing), ','.join(x.name for x in failing)
+    )
+
+
+@pytest.mark.skipif(
+    not executable.which('git'), reason='requires git to be installed'
+)
+def test_prs_update_old_api():
+    """Ensures that every package modified in a PR doesn't contain
+    deprecated calls to any method.
+    """
+    changed_package_files = [
+        x for x in flake8.changed_files() if flake8.is_package(x)
+    ]
+    failing = []
+    for file in changed_package_files:
+        name = os.path.basename(os.path.dirname(file))
+        pkg = spack.repo.get(name)
+
+        # Check for old APIs
+        failed = (hasattr(pkg, 'setup_environment') or
+                  hasattr(pkg, 'setup_dependent_environment'))
+        if failed:
+            failing.append(pkg)
+    msg = 'there are {0} packages still using old APIs in this PR [{1}]'
+    assert not failing, msg.format(
+        len(failing), ','.join(x.name for x in failing)
+    )