Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure correct compiler command/flags are used for SAMtools #966

Merged
merged 2 commits into from
Jul 7, 2016

Conversation

boegel
Copy link
Member

@boegel boegel commented Jul 6, 2016

@boegel boegel added this to the v2.8.2 milestone Jul 6, 2016
@boegel
Copy link
Member Author

boegel commented Jul 6, 2016

@wpoely86 please review

tested with all 29 existing SAMtools easyconfigs, works like a charm

@fgeorgatos
Copy link
Collaborator

entertaining how long this escaped under the radar;

do we have any means to verify when icc compiler was really picked up? is there any kind of stamp?

"""Ensure correct compiler command & flags are used via arguments to 'make' build command"""
for var in ['CC', 'CFLAGS']:
if var in os.environ:
self.cfg.update('buildopts', '%s="%s"' % (var, os.getenv(var)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

lgtm, btw

@wpoely86
Copy link
Member

wpoely86 commented Jul 7, 2016

@wpoely86 feels a warm glow of happiness

@boegel
Copy link
Member Author

boegel commented Jul 7, 2016

Going in, thanks for the review @fgeorgatos an @wpoely86!

@fgeorgatos I think @pforai knows of a way to check with which compiler stuff was built, so we could look into a generic detection mechanism in framework to catch occurrences like this...

@boegel boegel merged commit 1128a45 into easybuilders:develop Jul 7, 2016
@boegel boegel deleted the samtools_buildopts branch July 7, 2016 08:35
@backelj
Copy link

backelj commented Jul 7, 2016

I submitted the original ticket in Sep 10, 2015 already (https://github.com/hpcugent/easybuild-easyconfigs/issues/1959)…

I don’t recall whether we’ve discussed this somewhere, but I do recall thinking “how many other easyconfigs will also have this issue”?

Anyway, thanks for the fix, Kenneth…

— Franky

Op 7 jul. 2016, om 10:07 heeft Fotis Georgatos notifications@github.com het volgende geschreven:

entertaining how long this escaped under the radar;

do we have any means to verify when icc compiler was really picked up? is there any kind of stamp?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #966 (comment), or mute the thread https://github.com/notifications/unsubscribe/AFLw3YmXXtuVCE3FDMvb3r-rokoWz34Lks5qTLOpgaJpZM4JGe0J.

@pforai
Copy link
Contributor

pforai commented Jul 7, 2016

I've played around parsing ELF structures where compilers insert identification strings (usually .comment) and the basic approach was to generate a dummy ELF with the toolchain that is selected and check the ELF for whatever string was inserted by the compiler, then go through all of the generated binaries and do string comparison there.

The problem is that some compilers insert very precise infos and you can easily do comparisons while others are just inserting strings like "Intel Corporation" by default but compilation settings can be changed to get better visibility (-sox for ICC etc.).

GCC for example inserts additional ELF sections regarding ABI compatibility etc.

I guess we can make this work as part of sanity checking.

@wpoely86
Copy link
Member

wpoely86 commented Jul 7, 2016

If we're going to do compiler wrappers for rpath, we might as well use them for this too?

@boegel
Copy link
Member Author

boegel commented Jul 7, 2016

@wpoely86 where do compilers wrappers come into play here?

@wpoely86
Copy link
Member

wpoely86 commented Jul 7, 2016

with the compiler wrappers, we know if icc or gcc are used?

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.

5 participants