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

add support for enabling fallback in sanity check to consider lib64 equivalent for seemingly missing libraries #2477

Merged
merged 4 commits into from
May 18, 2018

Conversation

boegel
Copy link
Member

@boegel boegel commented Apr 18, 2018

This is an alternate implementation of the proposed change by @torbjoernk in #1405, as a fix for #1404.

I prefer keeping this optional, since having the fallback enabled by default may result in the sanity check passing where it perhaps shouldn't (although maybe I'm being too careful here, I'm not sure...).

This is fueled by the issues that @pmatousu has been reporting for failing sanity checks on SUSE systems, cfr. easybuilders/easybuild-easyblocks#1400 and easybuilders/easybuild-easyconfigs#6165 .

@vanzod
Copy link
Member

vanzod commented Apr 18, 2018

Very useful addition. I would even push it forward an make it two-ways, so that lib <--> lib64. This would simplify the sanity check in easyconfig files like the ones in [1] and [2].
Ideally I would like having it as a default behavior, despite the risks of breaking installations that rely on library directory names that are hardcoded in the easyblocks. In such cases it is a "bug" that should be addressed anyway.

[1] easybuilders/easybuild-easyconfigs#4722
[2] easybuilders/easybuild-easyconfigs#5897

@boegel boegel added this to the next release milestone Apr 24, 2018
@migueldiascosta
Copy link
Member

As a more general solution, perhaps for later, wondering if wouldn't be better to put symlinks to all libs found inside installdir in a single folder and only check/expose that one...

@boegel boegel changed the title add support for enabling fallback in sanity check to consider lib64 equivalent for seemingly missing libraries add support for enabling fallback in sanity check to consider lib64 equivalent for seemingly missing libraries (WIP) Apr 30, 2018
wpoely86
wpoely86 previously approved these changes May 4, 2018

# for library files in lib/, also consider fallback to lib64/ equivalent
if not found and build_option('lib64_fallback_sanity_check'):
if all(x.startswith('lib/') for x in xs):
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this all? why do all paths have to start with lib/?

Copy link
Member

Choose a reason for hiding this comment

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

a wait, xs is a tuple which we do OR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, xs is one entry from e.g. the files list from sanity_check_paths can be a tuple (and if it's just a string, we make it a tuple with a single element higher up).

@boegel
Copy link
Member Author

boegel commented May 7, 2018

@vanzod Reverse (also considering lib/libfoo.a when lib64/libfoo.a is not found) is now also implemented.

In addition, I enabled the lib64 fallback in the sanity check by default; it can be disabled if desired via --disable-lib64-fallback-sanity-check.

@boegel boegel changed the title add support for enabling fallback in sanity check to consider lib64 equivalent for seemingly missing libraries (WIP) add support for enabling fallback in sanity check to consider lib64 equivalent for seemingly missing libraries May 9, 2018
@boegel boegel requested a review from vanzod May 18, 2018 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants