Skip to content

Added a function that concretizes specs together#11158

Merged
becker33 merged 9 commits intospack:developfrom
alalazo:features/concretize_specs_together
May 3, 2019
Merged

Added a function that concretizes specs together#11158
becker33 merged 9 commits intospack:developfrom
alalazo:features/concretize_specs_together

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Apr 10, 2019

Refers to #9902
Refers to #11095

This PR adds a function that concretizes specs together or fails trying to do so.

Rationale

In many contexts (e.g. when using Spack environments to develop software or sometimes when deploying applications in a container) there could be the need to concretize specs together - meaning there will be a single configuration for each package in the DAG. This PR adds a function that permits to do just that:

import spack.concretize

# Hdf5 below will depend on `mpich` and `[email protected]`
concrete_specs = spack.concretize.concretize_specs_together('hdf5+mpi', '[email protected]', 'mpich')

The function comes with unit tests and can be used later to solve issues like #9902

Description

The implementation of this functionality relies on the current state of the concretizer and repository modules in Spack. This involves:

  1. Creating a temporary repository on the fly that contains a fake package, whose only purpose is to depend on all the specs passed as input.
  2. Concretize this fake package and extract its direct dependencies

Being factored within a single function, it shouldn't be difficult to adapt this once the new concretizer will be in place. If need be error handling (in particular user messages) can be improved either in this PR or in a follow up.

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Apr 11, 2019

@becker33 Ready for another review

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Apr 17, 2019

@becker33

Copy link
Copy Markdown
Member

@becker33 becker33 left a comment

Choose a reason for hiding this comment

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

@alalazo

One addition to the test, and an idea to clean up the helper function. Otherwise looks good

@alalazo alalazo force-pushed the features/concretize_specs_together branch from 7b65478 to de551bc Compare April 20, 2019 07:00
Copy link
Copy Markdown
Member

@becker33 becker33 left a comment

Choose a reason for hiding this comment

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

I'm a little concerned that the tests are hard to read. Posted an idea for a rewrite of the test. It could also be refactored further to keep the invariant as a function to check with.

@alalazo alalazo mentioned this pull request Apr 26, 2019
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented May 1, 2019

@becker33 ping

@becker33 becker33 merged commit 5ffb270 into spack:develop May 3, 2019
@alalazo alalazo deleted the features/concretize_specs_together branch May 3, 2019 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

concretization environments tests General test capability(ies)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants