Skip to content

SuperLU handle both lib and lib64 subdirectories#1479

Merged
damianam merged 3 commits intoeasybuilders:developfrom
hajgato:superlu
Aug 24, 2018
Merged

SuperLU handle both lib and lib64 subdirectories#1479
damianam merged 3 commits intoeasybuilders:developfrom
hajgato:superlu

Conversation

@hajgato
Copy link
Copy Markdown
Collaborator

@hajgato hajgato commented Aug 22, 2018

No description provided.

Copy link
Copy Markdown
Member

@damianam damianam left a comment

Choose a reason for hiding this comment

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

I think we can simplify the code a bit. If we assume self.libbits = "lib", check for the path existence, and re-set self.libbits = "lib64" if it doesn't, then we can get rid of a few lines (the loop and the error raising). The failure would be caught in the sanity check anyway, no need to double check it before IMO.

@hajgato
Copy link
Copy Markdown
Collaborator Author

hajgato commented Aug 22, 2018

@damianam Done plus let the automatic fallback to handle lib/lib64 in the sanity check.
@boegel fixed the commit-hell.

@damianam
Copy link
Copy Markdown
Member

@hajgato I wasn't aware of any automatic fallback to check that. @boegel since when do we have that? :-?

@boegel
Copy link
Copy Markdown
Member

boegel commented Aug 23, 2018

@damianam Since EasyBuild v3.6.1, see easybuilders/easybuild-framework#2477

Not sure if it's a good idea to rely on that here though, since we know where stuff is installed...

The fallback can be disabled by configuring EasyBuild with --disable-lib64-fallback-sanity-check, in which case the SuperLU installation would fail during the sanity check if stuff is installed in lib64, while we actually know it's being installed there.

The fallback is sort of intended as a stop gap to avoid having to go hunt for lib hardcoding everywhere and replace it with lib or lib64...

Comment thread easybuild/easyblocks/s/superlu.py Outdated
actual_libpath = os.path.join(self.installdir, "lib", "libsuperlu_%s.%s" % (self.cfg['version'], self.lib_ext))
libbit = "lib"
if not os.path.exists(os.path.join(self.installdir, libbit)):
libbit = "lib64"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hajgato please fix indent here (4 spaces)

Also, what if the lib64 dir doesn't exist either? Don't we want a clean error message then?

Comment thread easybuild/easyblocks/s/superlu.py Outdated
self.libbits = libnames

if not self.libbits:
raise EasyBuildError("No lib or lib64 directory exist in installdir '%s'" % self.installdir)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hajgato Using a for loop is a bit weird here imho, since there are only two options:

self.libbits = 'lib'
if not os.path.exists(os.path.join(self.installdir, self.libbits)):
    self.libbits = 'lib64'

if not os.path.exists(os.path.join(self.installdir, self.libbits)):
    raise EasyBuildError("No lib or lib64 subdirectory exist in %s", self.installdir)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@boegel Done.

Comment thread easybuild/easyblocks/s/superlu.py Outdated
"""
custom_paths = {
'files': ["include/supermatrix.h", "lib/libsuperlu.%s" % self.lib_ext],
'files': ["include/supermatrix.h", "%s/libsuperlu.%s" % (self.libbits, self.lib_ext)],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hajgato Please use os.path.join(self.libbits, 'libsuperlu.%s' % self.lib_ext)

@boegel boegel added this to the 3.7.0 milestone Aug 24, 2018
@boegel
Copy link
Copy Markdown
Member

boegel commented Aug 24, 2018

Good to go for me, still works like a charm with all existing SuperLU easyconfigs.

@damianam Yours to merge?

@damianam damianam merged commit 3d6d9c8 into easybuilders:develop Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants