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

[SUGGESTION] patch sanity_check_paths['files'] to look in lib/ and lib64/ #1405

Conversation

torbjoernk
Copy link

This is a suggestion to fix #1404.
Don't know whether it breaks something elsewhere ...

This is a suggestion to fix easybuilders#1404.
Don't know whether it breaks something elsewhere ...

Signed-off-by: Torbjörn Klatt <t.klatt@fz-juelich.de>
@hpcugentbot
Copy link
Contributor

Automatic reply from Jenkins: Can I test this?

@boegel
Copy link
Member

boegel commented Sep 28, 2015

Jenkins: ok to test

@boegel
Copy link
Member

boegel commented Sep 28, 2015

@torbjoernk: this is an interesting idea... Did @berndmohr put you up to this? ;-)

I had something more elaborate in mind, i.e. add support for additional keys like libs or libraries to be specified in sanity_check_paths, which then enables us to check both in lib and lib64 automagically; this has been an idea for a long time (see #196), but it has never materialised.

Nevertheless, I like this approach, since it's a quick-and-easy fix for something that has annoyed a lot of people (mostly on SuSE and derivatives), and shouldn't do any harm. We already consider both lib and lib64 for $LD_LIBRARY_PATH and $LIBRARY_PATH, for example.

@wpoely86, @rjeschmi, @fgeorgatos, @pescobar, @ocaisa, @JensTimmerman: thoughts?

@wpoely86
Copy link
Member

I feel more for the #196 approach... It cannot be that hard to implement?

@boegel
Copy link
Member

boegel commented Sep 28, 2015

@wpoely86: I don't feel this excludes the need for #196. It's a quick and imho harmless fix to a real problem. Implementing #196 would still require updating a lot of easyconfig files...

@wpoely86
Copy link
Member

#196 can perfectly be done in a backwards compatibly way. Adding a libraries section doesn't invalidate the current files.

@hpcugentbot
Copy link
Contributor

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2099/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel
Copy link
Member

boegel commented Sep 28, 2015

@wpoely86: sure, but introduces a libraries key is only adding support for a fix (which requires reworking lots of easyconfig files), while this trick is an actual fix for the problem at hand (and doesn't rule out working in libraries in the coming future)

@akesandgren
Copy link
Contributor

akesandgren commented Aug 28, 2018

Is this still valid or should it be closed?
If still valid the code needs a slight adjustment, don't use "/", use os.pathsep (or whatever it is called)
And it should go both ways...

Bah... should have read the referenced PR 2477...

So, close? @boegel

@boegel
Copy link
Member

boegel commented Aug 29, 2018

@akesandgren This is no longer relevant, so I'll go ahead and close it.

Fallback to also consider lib64 for lib paths (and vice versa) was implemented in #2477 (and enabled by default, with a switch to disable it) .

This PR did serve as inspiration for that change though, so I'd like to thank @torbjoernk for this...

@boegel boegel closed this Aug 29, 2018
@boegel boegel modified the milestones: 3.x, 3.6.1 Aug 29, 2018
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.

5 participants