Skip to content

Commit

Permalink
Fix installing specs from yaml file (#6906)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
alalazo authored and adamjstewart committed Jan 16, 2018
1 parent 50ca497 commit 4d7e7f2
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
13 changes: 11 additions & 2 deletions lib/spack/spack/cmd/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion lib/spack/spack/cmd/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/spack/spack/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -1897,7 +1897,7 @@ def concretized(self):
"""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

Expand Down
21 changes: 21 additions & 0 deletions lib/spack/spack/test/cmd/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 4d7e7f2

Please sign in to comment.