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 specifying multiple alternatives for sanity check paths #753

Merged
merged 4 commits into from
Nov 9, 2013

Conversation

boegel
Copy link
Member

@boegel boegel commented Nov 9, 2013

This adds support for specifying sanity check paths like:

sanity_check_paths = {
    'files': ["bin/foo", ("lib/libfoo.a", "lib64/libfoo.a")],
    'dirs': [],
}

If libfoo.a is found in either lib or lib64, the sanity check would be considered OK.

Having this included would help resolve the problem with GCC lib dirs reported in easybuilders/easybuild-easyblocks#283, and is also required for software packages like OTF (see easybuilders/easybuild-easyconfigs#505), where the lib directories are different depending on the OS (e.g., lib on Scientific Linux, lib64 on OpenSuSE, etc.)

We already provide flexibility like this in make_module_req_guess for setting $LD_LIBRARY_PATH, so we should also provide it for sanity_check_paths.

@boegel
Copy link
Member Author

boegel commented Nov 9, 2013

@berndmohr, @stdweird, @fgeorgatos: comments?

@fgeorgatos
Copy link
Collaborator

visual review OK from this side

did you consider collapsing the two code blocks in something common? (there may be arguments against it though)

…hance toy easyblock to make default sanity check paths work
…stop hardcoding 'files' and 'dirs' here and there
@boegel
Copy link
Member Author

boegel commented Nov 9, 2013

@fgeorgatos: I pondered it briefly, but it seemed too much trouble. But, after looking at it again, I agree the two blocks are too similar to ignore it.
I've also refactored it a bit to avoid hardcoding of the files and libs keys as much as possible, and enhanced the (toy build) unit tests to check for the various use cases (default sanity check paths, paths which use this alternative paths business).

boegel added a commit that referenced this pull request Nov 9, 2013
add support for specifying multiple alternatives for sanity check paths
@boegel boegel merged commit 2e32ecb into easybuilders:develop Nov 9, 2013
@boegel boegel deleted the sanity_check_path_alternatives branch November 9, 2013 13:36
@berndmohr
Copy link

Well, this fix allows to write EB files with sanity checks for the various distros, BUT it requires extra care by the EB writers and you have to go through ALL EB files and make the fixes accordingly.

I think the real solution is to find out what is causing this (is it really libtool idotic stuff that causes it?) and then come up with a solution, because only then you really know what to fix.

The more flexible sanity checks might be still useful, e.g. to specify like things "('bin/x86_64/tool', 'bin/ia32/tool')"
and similar items.

Note that the lib/lib64 issue is not really fixed by providing more flexible sanity checks:

  1. LD_LIBRARY_PATH is not automatically set with lib64 in the module files, right? (works only with lib?)

  2. Other software (easyconfigs) using EB files using it, need now to find out whether to use "-L${EBROOTTOOL}/lib" or "-L${EBROOTTOOL}/lib64" -- that is you moved the issue to another level

  3. there might be even more stuff

@boegel
Copy link
Member Author

boegel commented Nov 9, 2013

@berndmohr: You're right, this is only a workaround, not a genuine fix of the problem. But for now, it'll do.

One way to properly fix this after figuring out what causes it, or how to figure out what will be used, is the extend the possible keys in sanity_check_paths to libs, and then have people only specify library names instead of paths. EB can then take care of getting the lib or lib64 part correct.

The $LD_LIBRARY_PATH thing is covered, EB checks for both lib and lib64, and will update $LD_LIBRARY_PATH with existing paths.
You're right about 2) (and probably 3) though, we've just moved the problem further down the line...

@boegel
Copy link
Member Author

boegel commented Nov 10, 2013

@berndmohr: Your item 2) of above can now easily be fixed in easyblocks (not in easyconfigs (yet?), it's a lot more difficult to make that possible in a clean way), see #754.
That PR provides get_software_libdir next to get_software_root, to figure out whether the dependency libdir is lib or lib64.

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.

3 participants