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
drop useless definition of NLSPATH in IntelBase, fix ipp library paths #1442
Conversation
else: | ||
nlspath = os.path.join('idb', 'intel64', 'locale', '%l_%t', '%N') | ||
txt += self.module_generator.prepend_paths('NLSPATH', nlspath) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree
easybuild/easyblocks/i/ipp.py
Outdated
@@ -109,12 +109,12 @@ def make_module_req_guess(self): | |||
guesses = super(EB_ipp, self).make_module_req_guess() | |||
|
|||
if LooseVersion(self.version) >= LooseVersion('9.0'): | |||
lib_path = os.path.join('lib', self.arch) | |||
lib_path = [os.path.join('ipp/lib', self.arch), os.path.join('lib', self.arch)] | |||
include_path = 'ipp/include' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're at it. Use os.path.join all the way, i.e.
lib_path = [os.path.join('ipp', 'lib', self.arch), os.path.join('lib', self.arch)]
include_path = os.path.join('ipp', 'include')
easybuild/easyblocks/i/ipp.py
Outdated
'files': ['ipp/lib/intel64/libipp%s' % y for x in ipp_libs for y in ['%s.a' % x, '%s.%s' % (x, shlib_ext)]], | ||
'files': [ | ||
os.path.join('ipp', 'lib', 'intel64', 'libipp%s') % y for x in ipp_libs | ||
for y in ['%s.a' % x, '%s.%s' % (x, shlib_ext)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continuation line unaligned for hanging indent
easybuild/easyblocks/i/ipp.py
Outdated
os.path.join('ipp', 'bin'), | ||
os.path.join('ipp', 'include'), | ||
os.path.join('ipp', 'tools', 'intel64') | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would write this slightly different. Something like
dirs = []
ipp_dir_list = ['bin', 'include', os.path.join('tools', 'intel64')]
if LooseVersion ...
add the version specific ones to dirs and ipp_dir_list
dirs.extend(os.path.join('ipp', d)) for d in ipp_dir_list
(At least i think it is extend one should use, but i usually never get that right)
easybuild/easyblocks/i/ipp.py
Outdated
@@ -82,21 +83,26 @@ def sanity_check_step(self): | |||
"""Custom sanity check paths for IPP.""" | |||
shlib_ext = get_shared_lib_ext() | |||
|
|||
dirs = [ os.path.join('ipp', x) for x in ['bin', 'include', os.path.join('tools', 'intel64')]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace after '['
Agreement reached about removing NLSPATH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
NLSPATH
with idb has been an useless variable for a long time. At least since 2015. More importantly, theipp
easyblock has never set correctly the library paths, as the ipp libraries are inipp/lib/intel64
, not inlib/intel64
(which also exists and has some extra libraries)