From 4d7e7f25c46d33f4e7c5b671aa006aac7b5f4fe4 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 16 Jan 2018 15:03:10 +0100 Subject: Fix installing specs from yaml file (#6906) The feature added in #4611 is currently broken. This commit fixes the behavior of the command and adds unit tests to ensure the basic semantic is maintained. It also changes slightly the behavior of Spec.concretized to avoid copying caches before the concretization (as this may result in a wrong hash computation for the DAG). --- lib/spack/spack/cmd/install.py | 13 +++++++++++-- lib/spack/spack/cmd/spec.py | 2 +- lib/spack/spack/spec.py | 2 +- lib/spack/spack/test/cmd/install.py | 21 +++++++++++++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index e4fb008172..e4fd74f6b5 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -417,11 +417,20 @@ def install(parser, args, **kwargs): if args.file: for file in args.package: with open(file, 'r') as f: - specs.append(spack.spec.Spec.from_yaml(f)) + s = spack.spec.Spec.from_yaml(f) + + if s.concretized().dag_hash() != s.dag_hash(): + msg = 'skipped invalid file "{0}". ' + msg += 'The file does not contain a concrete spec.' + tty.warn(msg.format(file)) + continue + + specs.append(s.concretized()) + else: specs = spack.cmd.parse_specs(args.package, concretize=True) if len(specs) == 0: - tty.error('The `spack install` command requires a spec to install.') + tty.die('The `spack install` command requires a spec to install.') if args.overwrite: # If we asked to overwrite an existing spec we must ensure that: diff --git a/lib/spack/spack/cmd/spec.py b/lib/spack/spack/cmd/spec.py index 39705d34f2..e6199f6ff3 100644 --- a/lib/spack/spack/cmd/spec.py +++ b/lib/spack/spack/cmd/spec.py @@ -69,7 +69,7 @@ def spec(parser, args): for spec in spack.cmd.parse_specs(args.specs): # With -y, just print YAML to output. if args.yaml: - if spec.name in spack.repo: + if spec.name in spack.repo or spec.virtual: spec.concretize() print(spec.to_yaml()) continue diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index eb40559ec5..9f25dd0487 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1897,7 +1897,7 @@ class Spec(object): """This is a non-destructive version of concretize(). First clones, then returns a concrete version of this package without modifying this package. """ - clone = self.copy() + clone = self.copy(caches=False) clone.concretize() return clone diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index 54ef1e97db..8e5502049d 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/lib/spack/spack/test/cmd/install.py @@ -245,3 +245,24 @@ def test_install_conflicts(conflict_spec): install(conflict_spec) assert install.returncode == 1 + + +@pytest.mark.usefixtures('noop_install', 'config') +@pytest.mark.parametrize('spec,concretize,error_code', [ + (Spec('mpi'), False, 1), + (Spec('mpi'), True, 0), + (Spec('boost'), False, 1), + (Spec('boost'), True, 0) +]) +def test_install_from_file(spec, concretize, error_code, tmpdir): + + if concretize: + spec.concretize() + + with fs.working_dir(str(tmpdir)): + # A non-concrete spec will fail to be installed + with open('spec.yaml', 'w') as f: + spec.to_yaml(f) + install('-f', 'spec.yaml', fail_on_error=False) + + assert install.returncode == error_code -- cgit v1.2.3-60-g2f50