Skip to content

Update HierarchicalMNS for GCCcore toolchain#2870

Merged
boegel merged 4 commits intoeasybuilders:developfrom
ocaisa:update_hmns_for_GCCcore
May 14, 2019
Merged

Update HierarchicalMNS for GCCcore toolchain#2870
boegel merged 4 commits intoeasybuilders:developfrom
ocaisa:update_hmns_for_GCCcore

Conversation

@ocaisa
Copy link
Copy Markdown
Member

@ocaisa ocaisa commented May 9, 2019

@Micket With our custom naming scheme (related to HMNS) I was also seeing an issue with the Clang easyconfig in easybuilders/easybuild-easyconfigs#8254 This PR fixes that for me, however the reported error (for me) was:

File "/tmp/eb-KByFqx/included-module-naming-schemes/easybuild/tools/module_naming_scheme/custom_hierarchical_mns.py", line 143, in det_modpath_extensions
    comp_name_ver = [comp_name, comp_ver_tmpl % comp_versions]
KeyError: 'GCC'

This is quite different from what you reported in
easybuilders/easybuild-easyconfigs#8254 (comment)
Does this PR fix your problem?

@ocaisa ocaisa mentioned this pull request May 9, 2019
Merged
@ocaisa
Copy link
Copy Markdown
Member Author

ocaisa commented May 9, 2019

Ok, I reproduced the error that was seen for @Micket , this fixes it.

@boegel boegel added this to the next release (3.9.1) milestone May 9, 2019
@boegel boegel added the bug fix label May 9, 2019
boegel
boegel approved these changes May 9, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm, but I'd like to get confirmation from @Micket on this before merging...

@boegel boegel changed the title Update hmns for GCCcore Update HierarchicalMNS for GCCcore toolchain May 9, 2019
@akesandgren
Copy link
Copy Markdown
Contributor

akesandgren commented May 9, 2019

This change can perhaps also make it possible to combine CUDA with GCCcore instead of GCC to make the GCC-CUDA based stuff.... might be worth looking at in the future...

akesandgren
akesandgren reviewed May 14, 2019
View reviewed changes
Comment thread easybuild/tools/module_naming_scheme/hierarchical_mns.py
tc_comp_name, tc_comp_ver = tc_comp_info
# Stick to name GCC for GCCcore
if tc_comp_name == GCCCORE:
tc_comp_name = 'GCC'
Copy link
Copy Markdown
Contributor

@akesandgren akesandgren May 14, 2019

Choose a reason for hiding this comment

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

Are you sure this won't cause things built with GCCcore to end up in .../Compiler/GCC instead of Compiler/GCCcore

I'm not sure exactly what this affects.

Copy link
Copy Markdown
Member Author

@ocaisa ocaisa May 14, 2019

Choose a reason for hiding this comment

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

No, it won't affect that since GCCcore is built with the dummy compiler (so the condition non_dummy_tc 4 lines up is not satisfied). This is only relevant to compilers built with GCCcore and only affects the searching for matches in COMP_NAME_VERSION_TEMPLATES (see the for loop around this block).

I'm not overly confident that this has no impact though but I don't believe it does. As far as I know Clang is the only compiler build from source using GCCcore (rather than GCC).

Copy link
Copy Markdown
Contributor

@Micket Micket May 14, 2019

Choose a reason for hiding this comment

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

I just tried this out with Clang, and then with a normal package
eb Ghostscript-9.27-GCCcore-8.2.0.eb --include-module-naming-schemes hierarchical_mns.py
and it was placed correctly (under software/Compiler/GCCcore/Ghostscript )

@Micket
Copy link
Copy Markdown
Contributor

Micket commented May 14, 2019 •
edited
Loading

lgtm, but I'd like to get confirmation from @Micket on this before merging...

@boegel
Building clang (on centos7) with this worked. I can't say I understand this part of the code, but, I have no complaints.

@boegel
Copy link
Copy Markdown
Member

boegel commented May 14, 2019

Thanks for confirming the fix @Micket!

@boegel boegel merged commit 8c1055e into easybuilders:develop May 14, 2019
@ocaisa ocaisa deleted the update_hmns_for_GCCcore branch May 14, 2019 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Micket Micket Micket left review comments

@akesandgren akesandgren akesandgren left review comments

@boegel boegel boegel approved these changes

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

3.9.1

Development

Successfully merging this pull request may close these issues.

4 participants

@ocaisa @akesandgren @Micket @boegel