Expand relative dev paths in environment files (#22045)

* Rewrite relative dev_spec paths internally to absolute paths in case of relocation of the environment file

* Test relative paths for dev_path in environments

* Add a --keep-relative flag to spack env create

This ensures that relative paths of develop paths are not expanded to
absolute paths when initializing the environment in a different location
from the spack.yaml init file.
This commit is contained in:
Harmen Stoppels 2021-03-15 21:38:35 +01:00 committed by GitHub
parent 4f1d9d6095
commit 195341113e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 139 additions and 20 deletions

View file

@ -157,6 +157,10 @@ def env_create_setup_parser(subparser):
subparser.add_argument(
'-d', '--dir', action='store_true',
help='create an environment in a specific directory')
subparser.add_argument(
'--keep-relative', action='store_true',
help='copy relative develop paths verbatim into the new environment'
' when initializing from envfile')
view_opts = subparser.add_mutually_exclusive_group()
view_opts.add_argument(
'--without-view', action='store_true',
@ -184,13 +188,14 @@ def env_create(args):
if args.envfile:
with open(args.envfile) as f:
_env_create(args.create_env, f, args.dir,
with_view=with_view)
with_view=with_view, keep_relative=args.keep_relative)
else:
_env_create(args.create_env, None, args.dir,
with_view=with_view)
def _env_create(name_or_path, init_file=None, dir=False, with_view=None):
def _env_create(name_or_path, init_file=None, dir=False, with_view=None,
keep_relative=False):
"""Create a new environment, with an optional yaml description.
Arguments:
@ -199,15 +204,18 @@ def _env_create(name_or_path, init_file=None, dir=False, with_view=None):
spack.yaml or spack.lock
dir (bool): if True, create an environment in a directory instead
of a named environment
keep_relative (bool): if True, develop paths are copied verbatim into
the new environment file, otherwise they may be made absolute if the
new environment is in a different location
"""
if dir:
env = ev.Environment(name_or_path, init_file, with_view)
env = ev.Environment(name_or_path, init_file, with_view, keep_relative)
env.write()
tty.msg("Created environment in %s" % env.path)
tty.msg("You can activate this environment with:")
tty.msg(" spack env activate %s" % env.path)
else:
env = ev.create(name_or_path, init_file, with_view)
env = ev.create(name_or_path, init_file, with_view, keep_relative)
env.write()
tty.msg("Created environment '%s' in %s" % (name_or_path, env.path))
tty.msg("You can activate this environment with:")

View file

@ -73,9 +73,7 @@ def concretize_develop(self, spec):
if not dev_info:
return False
path = dev_info['path']
path = path if os.path.isabs(path) else os.path.join(
env.path, path)
path = os.path.normpath(os.path.join(env.path, dev_info['path']))
if 'dev_path' in spec.variants:
assert spec.variants['dev_path'].value == path

View file

@ -132,7 +132,7 @@ def activate(
if use_env_repo:
spack.repo.path.put_first(_active_environment.repo)
tty.debug("Using environmennt '%s'" % _active_environment.name)
tty.debug("Using environment '%s'" % _active_environment.name)
# Construct the commands to run
cmds = ''
@ -393,12 +393,12 @@ def read(name):
return Environment(root(name))
def create(name, init_file=None, with_view=None):
def create(name, init_file=None, with_view=None, keep_relative=False):
"""Create a named environment in Spack."""
validate_env_name(name)
if exists(name):
raise SpackEnvironmentError("'%s': environment already exists" % name)
return Environment(root(name), init_file, with_view)
return Environment(root(name), init_file, with_view, keep_relative)
def config_dict(yaml_data):
@ -587,7 +587,7 @@ def regenerate(self, all_specs, roots):
class Environment(object):
def __init__(self, path, init_file=None, with_view=None):
def __init__(self, path, init_file=None, with_view=None, keep_relative=False):
"""Create a new environment.
The environment can be optionally initialized with either a
@ -600,6 +600,10 @@ def __init__(self, path, init_file=None, with_view=None):
with_view (str or bool): whether a view should be maintained for
the environment. If the value is a string, it specifies the
path to the view.
keep_relative (bool): if True, develop paths are copied verbatim
into the new environment file, otherwise they are made absolute
when the environment path is different from init_file's
directory.
"""
self.path = os.path.abspath(path)
@ -621,6 +625,13 @@ def __init__(self, path, init_file=None, with_view=None):
self._set_user_specs_from_lockfile()
else:
self._read_manifest(f, raw_yaml=default_manifest_yaml)
# Rewrite relative develop paths when initializing a new
# environment in a different location from the spack.yaml file.
if not keep_relative and hasattr(f, 'name') and \
f.name.endswith('.yaml'):
init_file_dir = os.path.abspath(os.path.dirname(f.name))
self._rewrite_relative_paths_on_relocation(init_file_dir)
else:
with lk.ReadTransaction(self.txlock):
self._read()
@ -637,6 +648,27 @@ def __init__(self, path, init_file=None, with_view=None):
# If with_view is None, then defer to the view settings determined by
# the manifest file
def _rewrite_relative_paths_on_relocation(self, init_file_dir):
"""When initializing the environment from a manifest file and we plan
to store the environment in a different directory, we have to rewrite
relative paths to absolute ones."""
if init_file_dir == self.path:
return
for name, entry in self.dev_specs.items():
dev_path = entry['path']
expanded_path = os.path.normpath(os.path.join(
init_file_dir, entry['path']))
# Skip if the expanded path is the same (e.g. when absolute)
if dev_path == expanded_path:
continue
tty.debug("Expanding develop path for {0} to {1}".format(
name, expanded_path))
self.dev_specs[name]['path'] = expanded_path
def _re_read(self):
"""Reinitialize the environment object if it has been written (this
may not be true if the environment was just created in this running
@ -1044,8 +1076,7 @@ def develop(self, spec, path, clone=False):
if clone:
# "steal" the source code via staging API
abspath = path if os.path.isabs(path) else os.path.join(
self.path, path)
abspath = os.path.normpath(os.path.join(self.path, path))
stage = spec.package.stage
stage.steal_source(abspath)

View file

@ -1615,8 +1615,7 @@ def _develop_specs_from_env(spec, env):
if not dev_info:
return
path = dev_info['path']
path = path if os.path.isabs(path) else os.path.join(env.path, path)
path = os.path.normpath(os.path.join(env.path, dev_info['path']))
if 'dev_path' in spec.variants:
assert spec.variants['dev_path'].value == path

View file

@ -202,7 +202,7 @@ def test_dev_build_env(tmpdir, mock_packages, install_mockery,
dev-build-test-install:
spec: dev-build-test-install@0.0.0
path: %s
""" % build_dir)
""" % os.path.relpath(str(build_dir), start=str(envdir)))
env('create', 'test', './spack.yaml')
with ev.read('test'):
@ -328,7 +328,7 @@ def test_dev_build_env_dependency(tmpdir, mock_packages, install_mockery,
dev-build-test-install:
spec: dev-build-test-install@0.0.0
path: %s
""" % build_dir)
""" % os.path.relpath(str(build_dir), start=str(envdir)))
env('create', 'test', './spack.yaml')
with ev.read('test'):
@ -343,7 +343,7 @@ def test_dev_build_env_dependency(tmpdir, mock_packages, install_mockery,
assert dep_spec.package.filename in os.listdir(dep_spec.prefix)
assert os.path.exists(spec.prefix)
# Ensure variants set properly
# Ensure variants set properly; ensure build_dir is absolute and normalized
for dep in (dep_spec, spec['dev-build-test-install']):
assert dep.satisfies('dev_path=%s' % build_dir)
assert spec.satisfies('^dev_path=*')

View file

@ -2113,7 +2113,11 @@ def test_env_activate_default_view_root_unconditional(env_deactivate,
viewdir = e.default_view.root
out = env('activate', '--sh', 'test')
assert 'PATH=%s' % os.path.join(viewdir, 'bin') in out
viewdir_bin = os.path.join(viewdir, 'bin')
assert "export PATH={0}".format(viewdir_bin) in out or \
"export PATH='{0}".format(viewdir_bin) in out or \
'export PATH="{0}'.format(viewdir_bin) in out
def test_concretize_user_specs_together():
@ -2369,3 +2373,82 @@ def _write_helper_raise(self, x, y):
e.clear()
e.write()
assert os.path.exists(str(spack_lock))
def _setup_develop_packages(tmpdir):
"""Sets up a structure ./init_env/spack.yaml, ./build_folder, ./dest_env
where spack.yaml has a relative develop path to build_folder"""
init_env = tmpdir.join('init_env')
build_folder = tmpdir.join('build_folder')
dest_env = tmpdir.join('dest_env')
fs.mkdirp(str(init_env))
fs.mkdirp(str(build_folder))
fs.mkdirp(str(dest_env))
raw_yaml = """
spack:
specs: ['mypkg1', 'mypkg2']
develop:
mypkg1:
path: ../build_folder
spec: mypkg@main
mypkg2:
path: /some/other/path
spec: mypkg@main
"""
spack_yaml = init_env.join('spack.yaml')
spack_yaml.write(raw_yaml)
return init_env, build_folder, dest_env, spack_yaml
def test_rewrite_rel_dev_path_new_dir(tmpdir):
"""Relative develop paths should be rewritten for new environments in
a different directory from the original manifest file"""
_, build_folder, dest_env, spack_yaml = _setup_develop_packages(tmpdir)
env('create', '-d', str(dest_env), str(spack_yaml))
with ev.Environment(str(dest_env)) as e:
assert e.dev_specs['mypkg1']['path'] == str(build_folder)
assert e.dev_specs['mypkg2']['path'] == '/some/other/path'
def test_rewrite_rel_dev_path_named_env(tmpdir):
"""Relative develop paths should by default be rewritten for new named
environment"""
_, build_folder, _, spack_yaml = _setup_develop_packages(tmpdir)
env('create', 'named_env', str(spack_yaml))
with ev.read('named_env') as e:
assert e.dev_specs['mypkg1']['path'] == str(build_folder)
assert e.dev_specs['mypkg2']['path'] == '/some/other/path'
def test_rewrite_rel_dev_path_original_dir(tmpdir):
"""Relative devevelop paths should not be rewritten when initializing an
environment with root path set to the same directory"""
init_env, _, _, spack_yaml = _setup_develop_packages(tmpdir)
with ev.Environment(str(init_env), str(spack_yaml)) as e:
assert e.dev_specs['mypkg1']['path'] == '../build_folder'
assert e.dev_specs['mypkg2']['path'] == '/some/other/path'
def test_rewrite_rel_dev_path_create_original_dir(tmpdir):
"""Relative develop paths should not be rewritten when creating an
environment in the original directory"""
init_env, _, _, spack_yaml = _setup_develop_packages(tmpdir)
env('create', '-d', str(init_env), str(spack_yaml))
with ev.Environment(str(init_env)) as e:
assert e.dev_specs['mypkg1']['path'] == '../build_folder'
assert e.dev_specs['mypkg2']['path'] == '/some/other/path'
def test_does_not_rewrite_rel_dev_path_when_keep_relative_is_set(tmpdir):
"""Relative develop paths should not be rewritten when --keep-relative is
passed to create"""
_, _, _, spack_yaml = _setup_develop_packages(tmpdir)
env('create', '--keep-relative', 'named_env', str(spack_yaml))
with ev.read('named_env') as e:
print(e.dev_specs)
assert e.dev_specs['mypkg1']['path'] == '../build_folder'
assert e.dev_specs['mypkg2']['path'] == '/some/other/path'

View file

@ -793,7 +793,7 @@ _spack_env_deactivate() {
_spack_env_create() {
if $list_options
then
SPACK_COMPREPLY="-h --help -d --dir --without-view --with-view"
SPACK_COMPREPLY="-h --help -d --dir --keep-relative --without-view --with-view"
else
_environments
fi