Skip to content

fix logic in robot_find_subtoolchain_for_dep: consider easyconfigs first, before (maybe) considering available modules#2698

Merged
ocaisa merged 2 commits intoeasybuilders:3.8.xfrom
boegel:fix_log_robot_find_subtoolchain_for_dep
Dec 18, 2018
Merged

fix logic in robot_find_subtoolchain_for_dep: consider easyconfigs first, before (maybe) considering available modules#2698
ocaisa merged 2 commits intoeasybuilders:3.8.xfrom
boegel:fix_log_robot_find_subtoolchain_for_dep

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Dec 17, 2018

This fixes a backwards-incompatible change that was introduced in #2690 .

The logic in robot_find_minimal_toolchain_of_dependency was such that the available easyconfig files determined which subtoolchain was used for each dependency, while now in robot_find_subtoolchain_for_dep it is available modules first and only then available easyconfig files.

This creates problems in situations where there are multiple easyconfig files available with different (sub)toolchains that can be used to resolve a dependency, and when only some of them are installed.

For example, if zlib is installed with GCCcore as toolchain, this will determine the subtoolchain used for zlib that's listed as a dependency. When later zlib is also installed with foss, the selected toolchain for a zlib dependency changes when foss is used as a toolchain, which results in a toolchain conflict on zlib.

This problem surfaced during the regression test for EasyBuild v3.8.0, in particular for easyconfigs used the foss/2016* and intel/2016* toolchains (which is when we started pushing "down" dependencies like zlib to the compiler-only subtoolchain rather than the full toolchain).

…rst, before (maybe) considering available modules
@boegel boegel added the bug fix label Dec 17, 2018
@boegel boegel added this to the 3.8.0 milestone Dec 17, 2018
@boegel boegel requested a review from ocaisa December 17, 2018 10:08
Copy link
Copy Markdown
Member

@ocaisa ocaisa left a comment

Choose a reason for hiding this comment

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

LGTM...is there a way we can add a test for this?

…able modules, unless --use-existing-modules is enabled
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Dec 17, 2018

@ocaisa dedicated test added in f0c0c6d

Copy link
Copy Markdown
Member

@ocaisa ocaisa left a comment

Choose a reason for hiding this comment

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

LGTM

@ocaisa ocaisa merged commit 2a66307 into easybuilders:3.8.x Dec 18, 2018
@boegel boegel deleted the fix_log_robot_find_subtoolchain_for_dep branch December 18, 2018 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants