Skip to content

Commit 9ddc98e

Browse files
alalazobecker33
alalazo
authored and
becker33
committed
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
1 parent cf9de05 commit 9ddc98e

14 files changed

Lines changed: 331 additions & 213 deletions

File tree

‎lib/spack/docs/module_file_support.rst‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -303,25 +303,23 @@ content of the module files generated by Spack. The first one:
303303

304304
.. code-block:: python
305305
306-
def setup_environment(self, spack_env, run_env):
307-
"""Set up the compile and runtime environments for a package."""
306+
def setup_run_environment(self, env):
308307
pass
309308
310309
can alter the content of the module file associated with the same package where it is overridden.
311310
The second method:
312311

313312
.. code-block:: python
314313
315-
def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
316-
"""Set up the environment of packages that depend on this one"""
314+
def setup_dependent_run_environment(self, env, dependent_spec):
317315
pass
318316
319317
can instead inject run-time environment modifications in the module files of packages
320318
that depend on it. In both cases you need to fill ``run_env`` with the desired
321319
list of environment modifications.
322320

323-
.. note::
324-
The ``r`` package and callback APIs
321+
.. admonition:: The ``r`` package and callback APIs
322+
325323
An example in which it is crucial to override both methods
326324
is given by the ``r`` package. This package installs libraries and headers
327325
in non-standard locations and it is possible to prepend the appropriate directory
@@ -336,14 +334,14 @@ list of environment modifications.
336334
with the following snippet:
337335

338336
.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/r/package.py
339-
:pyobject: R.setup_environment
337+
:pyobject: R.setup_run_environment
340338

341339
The ``r`` package also knows which environment variable should be modified
342340
to make language extensions provided by other packages available, and modifies
343341
it appropriately in the override of the second method:
344342

345343
.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/r/package.py
346-
:pyobject: R.setup_dependent_environment
344+
:pyobject: R.setup_dependent_run_environment
347345

348346
.. _modules-yaml:
349347

‎lib/spack/docs/packaging_guide.rst‎

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,55 +2032,58 @@ appear in the package file (or in this case, in the list).
20322032

20332033
.. _setup-dependent-environment:
20342034

2035-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2036-
``setup_dependent_environment()``
2037-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2038-
2039-
Spack provides a mechanism for dependencies to provide variables that
2040-
can be used in their dependents' build. Any package can declare a
2041-
``setup_dependent_environment()`` function, and this function will be
2042-
called before the ``install()`` method of any dependent packages.
2043-
This allows dependencies to set up environment variables and other
2044-
properties to be used by dependents.
2045-
2046-
The function declaration should look like this:
2035+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2036+
Influence how dependents are built or run
2037+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2038+
2039+
Spack provides a mechanism for dependencies to influence the
2040+
environment of their dependents by overriding the
2041+
:meth:`setup_dependent_run_environment <spack.package.PackageBase.setup_dependent_run_environment>`
2042+
or the
2043+
:meth:`setup_dependent_build_environment <spack.package.PackageBase.setup_dependent_build_environment>`
2044+
methods.
2045+
The Qt package, for instance, uses this call:
20472046

20482047
.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/qt/package.py
2049-
:pyobject: Qt.setup_dependent_environment
2048+
:pyobject: Qt.setup_dependent_build_environment
20502049
:linenos:
20512050

2052-
Here, the Qt package sets the ``QTDIR`` environment variable so that
2053-
packages that depend on a particular Qt installation will find it.
2054-
2055-
The arguments to this function are:
2051+
to set the ``QTDIR`` environment variable so that packages
2052+
that depend on a particular Qt installation will find it.
2053+
Another good example of how a dependency can influence
2054+
the build environment of dependents is the Python package:
20562055

2057-
* **spack_env**: List of environment modifications to be applied when
2058-
the dependent package is built within Spack.
2059-
* **run_env**: List of environment modifications to be applied when
2060-
the dependent package is run outside of Spack. These are added to the
2061-
resulting module file.
2062-
* **dependent_spec**: The spec of the dependent package about to be
2063-
built. This allows the extendee (self) to query the dependent's state.
2064-
Note that *this* package's spec is available as ``self.spec``.
2056+
.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py
2057+
:pyobject: Python.setup_dependent_build_environment
2058+
:linenos:
20652059

2066-
A good example of using these is in the Python package:
2060+
In the method above it is ensured that any package that depends on Python
2061+
will have the ``PYTHONPATH``, ``PYTHONHOME`` and ``PATH`` environment
2062+
variables set appropriately before starting the installation. To make things
2063+
even simpler the ``python setup.py`` command is also inserted into the module
2064+
scope of dependents by overriding a third method called
2065+
:meth:`setup_dependent_package <spack.package.PackageBase.setup_dependent_package>`
2066+
:
20672067

20682068
.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py
2069-
:pyobject: Python.setup_dependent_environment
2069+
:pyobject: Python.setup_dependent_package
20702070
:linenos:
20712071

2072-
The first thing that happens here is that the ``python`` command is
2073-
inserted into module scope of the dependent. This allows most python
2074-
packages to have a very simple install method, like this:
2072+
This allows most python packages to have a very simple install procedure,
2073+
like the following:
20752074

20762075
.. code-block:: python
20772076
20782077
def install(self, spec, prefix):
2079-
python('setup.py', 'install', '--prefix={0}'.format(prefix))
2078+
setup_py('install', '--prefix={0}'.format(prefix))
2079+
2080+
Finally the Python package takes also care of the modifications to ``PYTHONPATH``
2081+
to allow dependencies to run correctly:
2082+
2083+
.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py
2084+
:pyobject: Python.setup_dependent_run_environment
2085+
:linenos:
20802086

2081-
Python's ``setup_dependent_environment`` method also sets up some
2082-
other variables, creates a directory, and sets up the ``PYTHONPATH``
2083-
so that dependent packages can find their dependencies at build time.
20842087

20852088
.. _packaging_conflicts:
20862089

‎lib/spack/docs/tutorial_advanced_packaging.rst‎

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -90,21 +90,23 @@ like py-numpy, Spack's ``python`` package will add it to ``PYTHONPATH``
9090
so it is available at build time; this is required because the default setup
9191
that spack does is not sufficient for python to import modules.
9292

93-
To provide environment setup for a dependent, a package can implement the
94-
:py:func:`setup_dependent_environment <spack.package.PackageBase.setup_dependent_environment>`
95-
function. This function takes as a parameter a :py:class:`EnvironmentModifications <spack.util.environment.EnvironmentModifications>`
93+
Any package can override the
94+
:py:func:`setup_dependent_build_environment <spack.package.PackageBase.setup_dependent_build_environment>`
95+
method to setup the build environment for a dependent.
96+
This method takes as an argument a :py:class:`EnvironmentModifications <spack.util.environment.EnvironmentModifications>`
9697
object which includes convenience methods to update the environment. For
9798
example, an MPI implementation can set ``MPICC`` for packages that depend on it:
9899

99100
.. code-block:: python
100101
101-
def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
102-
spack_env.set('MPICC', join_path(self.prefix.bin, 'mpicc'))
102+
def setup_dependent_build_environment(self, env, dependent_spec):
103+
env.set('MPICC', join_path(self.prefix.bin, 'mpicc'))
103104
104105
In this case packages that depend on ``mpi`` will have ``MPICC`` defined in
105-
their environment when they build. This section is focused on modifying the
106-
build-time environment represented by ``spack_env``, but it's worth noting that
107-
modifications to ``run_env`` are included in Spack's automatically-generated
106+
their environment when they build. This section is focused on setting up the
107+
build-time environment but it's worth noting that a similar method called
108+
:py:func:`setup_dependent_run_environment <spack.package.PackageBase.setup_dependent_run_environment>`
109+
can be used to code modifications that will be included in Spack's automatically-generated
108110
module files.
109111

110112
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:
118120

119121
.. code-block:: python
120122
121-
def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
122-
spack_env.set('MPICC', join_path(self.prefix.bin, 'mpicc'))
123-
spack_env.set('MPICXX', join_path(self.prefix.bin, 'mpic++'))
124-
spack_env.set('MPIF77', join_path(self.prefix.bin, 'mpif77'))
125-
spack_env.set('MPIF90', join_path(self.prefix.bin, 'mpif90'))
123+
def setup_dependent_build_environment(self, env, dependent_spec):
124+
env.set('MPICC', join_path(self.prefix.bin, 'mpicc'))
125+
env.set('MPICXX', join_path(self.prefix.bin, 'mpic++'))
126+
env.set('MPIF77', join_path(self.prefix.bin, 'mpif77'))
127+
env.set('MPIF90', join_path(self.prefix.bin, 'mpif90'))
126128
127-
spack_env.set('MPICH_CC', spack_cc)
128-
spack_env.set('MPICH_CXX', spack_cxx)
129-
spack_env.set('MPICH_F77', spack_f77)
130-
spack_env.set('MPICH_F90', spack_fc)
131-
spack_env.set('MPICH_FC', spack_fc)
129+
env.set('MPICH_CC', spack_cc)
130+
env.set('MPICH_CXX', spack_cxx)
131+
env.set('MPICH_F77', spack_f77)
132+
env.set('MPICH_F90', spack_fc)
133+
env.set('MPICH_FC', spack_fc)
132134
133135
At this point we can, for instance, install ``netlib-scalapack`` with
134136
``mpich``:
@@ -155,25 +157,32 @@ set to the correct value.
155157
Set environment variables in your own package
156158
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
157159

158-
Packages can modify their own build-time environment by implementing the
159-
:py:func:`setup_environment <spack.package.PackageBase.setup_environment>` function.
160-
For ``qt`` this looks like:
160+
Packages can override the
161+
:py:func:`setup_build_environment <spack.package.PackageBase.setup_build_environment>`
162+
or the
163+
:py:func:`setup_run_environment <spack.package.PackageBase.setup_run_environment>`
164+
methods to modify their own build-time or run-time environment respectively.
165+
An example of a package that overrides both methods is ``qt``:
161166

162167
.. code-block:: python
163168
164-
def setup_environment(self, spack_env, run_env):
165-
spack_env.set('MAKEFLAGS', '-j{0}'.format(make_jobs))
166-
run_env.set('QTDIR', self.prefix)
169+
def setup_build_environment(self, env):
170+
env.set('MAKEFLAGS', '-j{0}'.format(make_jobs))
167171
168-
When ``qt`` builds, ``MAKEFLAGS`` will be defined in the environment.
172+
def setup_run_environment(self, env):
173+
env.set('QTDIR', self.prefix)
169174
170-
To contrast with ``qt``'s :py:func:`setup_dependent_environment <spack.package.PackageBase.setup_dependent_environment>`
175+
When ``qt`` builds, ``MAKEFLAGS`` will be defined in the environment. Likewise, when a
176+
module file is created for ``qt`` it will contain commands to define ``QTDIR`` appropriately.
177+
178+
To contrast with ``qt``'s
179+
:py:func:`setup_dependent_build_environment <spack.package.PackageBase.setup_dependent_build_environment>`
171180
function:
172181

173182
.. code-block:: python
174183
175-
def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
176-
spack_env.set('QTDIR', self.prefix)
184+
def setup_dependent_build_environment(self, env, dependent_spec):
185+
env.set('QTDIR', self.prefix)
177186
178187
Let's see how it works by completing the ``elpa`` package:
179188

@@ -185,16 +194,16 @@ In the end your method should look like:
185194

186195
.. code-block:: python
187196
188-
def setup_environment(self, spack_env, run_env):
197+
def setup_build_environment(self, env):
189198
spec = self.spec
190199
191-
spack_env.set('CC', spec['mpi'].mpicc)
192-
spack_env.set('FC', spec['mpi'].mpifc)
193-
spack_env.set('CXX', spec['mpi'].mpicxx)
194-
spack_env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined())
200+
env.set('CC', spec['mpi'].mpicc)
201+
env.set('FC', spec['mpi'].mpifc)
202+
env.set('CXX', spec['mpi'].mpicxx)
203+
env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined())
195204
196-
spack_env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags)
197-
spack_env.append_flags('LIBS', spec['lapack'].libs.link_flags)
205+
env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags)
206+
env.append_flags('LIBS', spec['lapack'].libs.link_flags)
198207
199208
At this point it's possible to proceed with the installation of ``elpa ^mpich``
200209

‎lib/spack/spack/build_environment.py‎

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -673,35 +673,26 @@ def load_external_modules(pkg):
673673

674674
def setup_package(pkg, dirty):
675675
"""Execute all environment setup routines."""
676-
spack_env = EnvironmentModifications()
677-
run_env = EnvironmentModifications()
676+
build_env = EnvironmentModifications()
678677

679678
if not dirty:
680679
clean_environment()
681680

682-
set_compiler_environment_variables(pkg, spack_env)
683-
set_build_environment_variables(pkg, spack_env, dirty)
684-
pkg.architecture.platform.setup_platform_environment(pkg, spack_env)
681+
set_compiler_environment_variables(pkg, build_env)
682+
set_build_environment_variables(pkg, build_env, dirty)
683+
pkg.architecture.platform.setup_platform_environment(pkg, build_env)
685684

686-
# traverse in postorder so package can use vars from its dependencies
687-
spec = pkg.spec
688-
for dspec in pkg.spec.traverse(order='post', root=False,
689-
deptype=('build', 'test')):
690-
spkg = dspec.package
691-
set_module_variables_for_package(spkg)
692-
693-
# Allow dependencies to modify the module
694-
dpkg = dspec.package
695-
dpkg.setup_dependent_package(pkg.module, spec)
696-
dpkg.setup_dependent_environment(spack_env, run_env, spec)
685+
build_env.extend(
686+
modifications_from_dependencies(pkg.spec, context='build')
687+
)
697688

698-
if (not dirty) and (not spack_env.is_unset('CPATH')):
689+
if (not dirty) and (not build_env.is_unset('CPATH')):
699690
tty.debug("A dependency has updated CPATH, this may lead pkg-config"
700691
" to assume that the package is part of the system"
701692
" includes and omit it when invoked with '--cflags'.")
702693

703694
set_module_variables_for_package(pkg)
704-
pkg.setup_environment(spack_env, run_env)
695+
pkg.setup_build_environment(build_env)
705696

706697
# Loading modules, in particular if they are meant to be used outside
707698
# of Spack, can change environment variables that are relevant to the
@@ -711,7 +702,7 @@ def setup_package(pkg, dirty):
711702
# unnecessary. Modules affecting these variables will be overwritten anyway
712703
with preserve_environment('CC', 'CXX', 'FC', 'F77'):
713704
# All module loads that otherwise would belong in previous
714-
# functions have to occur after the spack_env object has its
705+
# functions have to occur after the build_env object has its
715706
# modifications applied. Otherwise the environment modifications
716707
# could undo module changes, such as unsetting LD_LIBRARY_PATH
717708
# after a module changes it.
@@ -727,8 +718,39 @@ def setup_package(pkg, dirty):
727718
load_external_modules(pkg)
728719

729720
# Make sure nothing's strange about the Spack environment.
730-
validate(spack_env, tty.warn)
731-
spack_env.apply_modifications()
721+
validate(build_env, tty.warn)
722+
build_env.apply_modifications()
723+
724+
725+
def modifications_from_dependencies(spec, context):
726+
"""Returns the environment modifications that are required by
727+
the dependencies of a spec and also applies modifications
728+
to this spec's package at module scope, if need be.
729+
730+
Args:
731+
spec (Spec): spec for which we want the modifications
732+
context (str): either 'build' for build-time modifications or 'run'
733+
for run-time modifications
734+
"""
735+
env = EnvironmentModifications()
736+
pkg = spec.package
737+
738+
# Maps the context to deptype and method to be called
739+
deptype_and_method = {
740+
'build': (('build', 'link', 'test'),
741+
'setup_dependent_build_environment'),
742+
'run': (('link', 'run'), 'setup_dependent_run_environment')
743+
}
744+
deptype, method = deptype_and_method[context]
745+
746+
for dspec in spec.traverse(order='post', root=False, deptype=deptype):
747+
dpkg = dspec.package
748+
set_module_variables_for_package(dpkg)
749+
# Allow dependencies to modify the module
750+
dpkg.setup_dependent_package(pkg.module, spec)
751+
getattr(dpkg, method)(env, spec)
752+
753+
return env
732754

733755

734756
def fork(pkg, function, dirty, fake):

‎lib/spack/spack/cmd/flake8.py‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,11 @@ def is_package(f):
9393
for file_pattern, error_dict in pattern_exemptions.items())
9494

9595

96-
def changed_files(args):
96+
def changed_files(base=None, untracked=True, all_files=False):
9797
"""Get list of changed files in the Spack repository."""
9898

9999
git = which('git', required=True)
100100

101-
base = args.base
102101
if base is None:
103102
base = os.environ.get('TRAVIS_BRANCH', 'develop')
104103

@@ -114,11 +113,11 @@ def changed_files(args):
114113
]
115114

116115
# Add new files that are untracked
117-
if args.untracked:
116+
if untracked:
118117
git_args.append(['ls-files', '--exclude-standard', '--other'])
119118

120119
# add everything if the user asked for it
121-
if args.all:
120+
if all_files:
122121
git_args.append(['ls-files', '--exclude-standard'])
123122

124123
excludes = [os.path.realpath(f) for f in exclude_directories]
@@ -246,7 +245,7 @@ def prefix_relative(path):
246245

247246
with working_dir(spack.paths.prefix):
248247
if not file_list:
249-
file_list = changed_files(args)
248+
file_list = changed_files(args.base, args.untracked, args.all)
250249

251250
print('=======================================================')
252251
print('flake8: running flake8 code checks on spack.')

0 commit comments

Comments
 (0)