Skip to content

enforce that hiddendependencies is a subset of dependencies#1078

Merged
boegel merged 3 commits intoeasybuilders:developfrom
boegel:hiddendeps_subset_of_deps
Nov 4, 2014
Merged

enforce that hiddendependencies is a subset of dependencies#1078
boegel merged 3 commits intoeasybuilders:developfrom
boegel:hiddendeps_subset_of_deps

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Oct 29, 2014

by request of @stdweird

@ocaisa, @geimer: heads up on this one, since this makes things stricter compared to EasyBuild v1.15.x

We're enforcing here that all entries of hiddendependencies are also listed in the dependencies list.
Before EasyBuild resolving dependencies, it will filter out the dependencies that are listed as hidden from the dependencies list, so semantics w.r.t. installing missing modules hidden is retained.

We want to avoid that contributed easyconfig files contain site-specific decision w.r.t. whether modules should be installed hidden or not.

By ensuring that hiddendependencies is a subset of dependencies, it becomes trivial to make easyconfig files using hiddendependencies site-agnostic: simply remove the hiddendependencies specification.

The hiddendependencies easyconfig parameter still has its merit (to indicate to --robot which modules must be installed hidden), and will not disappear.

Sometime in the future, we will also add support for resolving entries in the dependencies list with an existing hidden module, as a fallback in case the module is not available non-hidden. When neither are available, the module will be installed non-hidden via the robot, unless it is included in the hiddendependencies list (in which case it has to be available as a hidden module).

@ocaisa, @geimer: thoughs on this?

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 29, 2014

issue for resolving deps via an existing hidden @ #1079

@hpcugentbot
Copy link
Copy Markdown

Test FAILed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clarify what is allowed in hiddendependencies: the module name, software name, or the whole dependency tuple.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For now the entries of hiddendependencies are passes through _parse_dependency, so they have to be name/version tuples (and can include suffix/toolchain optionally), like other dependencies in dependencies or builddependencies.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add this to the docstring (and is there an issue for a more human-friendly version with just the package names?)

@hpcugentbot
Copy link
Copy Markdown

Test PASSed.

@geimer
Copy link
Copy Markdown
Contributor

geimer commented Oct 30, 2014

@boegel: Your motivation for this change makes sense to me. Though, frankly speaking, I was never really a friend of these hidden dependencies... They were "invented" as a workaround for the Intel/Clang STL problem in the context of hierarchical modules. And I still believe this should be handled differently in the long run (by "collapsing" the GCC STL into the Intel/Clang install directories -- note: only the STL, not the full compiler).

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 30, 2014

@geimer: true, support for hidden modules was added in the context of pushing GCC under icc/ifort in the intel/2014.08 toolchain (cfr. easybuilders/easybuild-easyconfigs#1014)

Collapsing GCC into icc/ifort (or Clang) is probably better, agreed, but still needs to be looked into.

Afterwards implementing support for hidden modules, with some input by @rtmclay, we realised this wasn't the best approach, but supporting hidden modules is still useful, e.g., as an easy way to hide an installation from users before declaring it production-quality.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 30, 2014

@stdweird: remarks fixed, pls rereview?

@hpcugentbot
Copy link
Copy Markdown

Test PASSed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's a bit brute force. do you expect multiple entries to be removed here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

wel, dependencies should be unique, but this is not enforced anywhere imho

@stdweird
Copy link
Copy Markdown
Contributor

@boegel minor remarks

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Nov 4, 2014

merging this in, thanks for the feedback @geimer and for the review @stdweird

boegel added a commit that referenced this pull request Nov 4, 2014
enforce that hiddendependencies is a subset of dependencies
@boegel boegel merged commit a0177d4 into easybuilders:develop Nov 4, 2014
@boegel boegel deleted the hiddendeps_subset_of_deps branch November 4, 2014 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants