Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bugfix] Do not set up Spack shell support #2424

Merged
merged 3 commits into from
Feb 17, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions docs/tutorial_build_automation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,10 @@ Here is what ReFrame generates as a build script for this example:

.. code:: bash

. "$(spack location --spack-root)/share/spack/setup-env.sh"
spack env create -d rfm_spack_env
spack env activate -V -d rfm_spack_env
spack config add "config:install_tree:root:opt/spack"
spack add [email protected]
spack install
spack -e rfm_spack_env config add "config:install_tree:root:opt/spack"
spack -e rfm_spack_env add [email protected]
spack -e rfm_spack_env install

As you might have noticed ReFrame expects that Spack is already installed on the system.
The packages specified in the environment and the tests will be installed in the test's stage directory, where the environment is copied before building.
Expand Down Expand Up @@ -155,10 +153,8 @@ Finally, here is the generated run script that ReFrame uses to run the test, onc
.. code-block:: bash

#!/bin/bash
. "$(spack location --spack-root)/share/spack/setup-env.sh"
spack env create -d rfm_spack_env
spack env activate -V -d rfm_spack_env
spack load [email protected]
eval `spack -e rfm_spack_env load --sh [email protected]`
Copy link

@haampie haampie Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is reasonable; either you use spack env activate, or you use spack load, but not both, cause spack env activate --sh also returns shell-environment variables changes for the spack-environment root specs and their transient run/link type dependencies.

so, it's either eval "$(spack -e [env name] load --sh [spec name])" where you use the environment as a filter to pick the just-built bzip2 from the spack installs, or assuming you only have one root spec in the environment, you do eval "$(spack env activate --sh [env name])"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also put concretize: together instead of the default concretize: separately in the environment, and then you can be sure that spack env activate --sh ... always works (cant have multiple bzip2's in one env then)

Copy link

@haampie haampie Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if you can change the value spack:concretize:... from the command line though :(

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users can pass a custom environment through the self.build_system.environment option and reframe will not auto-generate the environment if they do.

bzip2 --help

From this point on, sanity and performance checking are exactly identical to any other ReFrame test.
Expand Down
29 changes: 15 additions & 14 deletions reframe/core/buildsystems.py
Original file line number Diff line number Diff line change
Expand Up @@ -850,38 +850,39 @@ def __init__(self):
self._auto_env = False

def emit_build_commands(self, environ):
ret = self._env_activate_cmds()
ret = self._create_env_cmds()

if self._auto_env:
install_tree = self.install_tree or 'opt/spack'
ret.append(f'spack config add '
ret.append(f'spack -e {self.environment} config add '
f'"config:install_tree:root:{install_tree}"')

if self.specs:
specs_str = ' '.join(self.specs)
ret.append(f'spack add {specs_str}')
ret.append(f'spack -e {self.environment} add {specs_str}')

install_cmd = 'spack install'
install_cmd = f'spack -e {self.environment} install'
if self.install_opts:
install_cmd += ' ' + ' '.join(self.install_opts)

ret.append(install_cmd)
return ret

def _env_activate_cmds(self):
cmds = ['. "$(spack location --spack-root)/share/spack/setup-env.sh"']
if not self.environment:
self.environment = 'rfm_spack_env'
cmds.append(f'spack env create -d {self.environment}')
self._auto_env = True
def _create_env_cmds(self):
if self.environment:
return []

cmds.append(f'spack env activate -V -d {self.environment}')
return cmds
self.environment = 'rfm_spack_env'
self._auto_env = True
return [f'spack env create -d {self.environment}']

def prepare_cmds(self):
cmds = self._env_activate_cmds()
cmds = self._create_env_cmds()
if self.specs and self.emit_load_cmds:
cmds.append('spack load ' + ' '.join(s for s in self.specs))
cmds.append(
f'eval `spack -e {self.environment} load '
f'--sh {" ".join(self.specs)}`'
)

return cmds

Expand Down
23 changes: 7 additions & 16 deletions unittests/test_buildsystems.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,13 +249,9 @@ def test_spack(environ, tmp_path):
build_system.install_opts = ['-j 10']
with osext.change_dir(tmp_path):
assert build_system.emit_build_commands(environ) == [
f'. "$(spack location --spack-root)/share/spack/setup-env.sh"',
f'spack env activate -V -d {build_system.environment}',
f'spack install -j 10'
f'spack -e {build_system.environment} install -j 10'
]
assert build_system.prepare_cmds() == [
f'. "$(spack location --spack-root)/share/spack/setup-env.sh"',
f'spack env activate -V -d {build_system.environment}',
]


Expand All @@ -266,27 +262,22 @@ def test_spack_with_spec(environ, tmp_path):
specs_str = ' '.join(build_system.specs)
with osext.change_dir(tmp_path):
assert build_system.emit_build_commands(environ) == [
f'. "$(spack location --spack-root)/share/spack/setup-env.sh"',
f'spack env activate -V -d {build_system.environment}',
f'spack add {specs_str}',
f'spack install'
f'spack -e {build_system.environment} add {specs_str}',
f'spack -e {build_system.environment} install'
]
assert build_system.prepare_cmds() == [
f'. "$(spack location --spack-root)/share/spack/setup-env.sh"',
f'spack env activate -V -d {build_system.environment}',
f'spack load {specs_str}',
f'eval `spack -e {build_system.environment} load --sh {specs_str}`'
]


def test_spack_no_env(environ, tmp_path):
build_system = bs.Spack()
with osext.change_dir(tmp_path):
assert build_system.emit_build_commands(environ) == [
f'. "$(spack location --spack-root)/share/spack/setup-env.sh"',
f'spack env create -d rfm_spack_env',
f'spack env activate -V -d rfm_spack_env',
f'spack config add "config:install_tree:root:opt/spack"',
f'spack install'
f'spack -e rfm_spack_env config add '
'"config:install_tree:root:opt/spack"',
f'spack -e rfm_spack_env install'
]

assert build_system.environment == 'rfm_spack_env'
Expand Down