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
enhance PETSc easyblock for version 3.9 #1421
Conversation
easybuild/easyblocks/p/petsc.py
Outdated
if self.cfg['sourceinstall']: | ||
prefix1 = self.petsc_subdir | ||
prefix2 = os.path.join(self.petsc_subdir, self.petsc_arch) | ||
|
||
if LooseVersion(self.version) >= LooseVersion("3.9"): | ||
prefix3 = prefix1 + 'lib/petsc/' |
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.
@RvDijk please use os.path.join
:
prefix3 = os.path.join(prefix1, 'lib', 'petsc')
easybuild/easyblocks/p/petsc.py
Outdated
@@ -325,9 +330,12 @@ def sanity_check_step(self): | |||
else: | |||
libext = 'a' | |||
|
|||
if LooseVersion(self.version) >= LooseVersion("3.9"): | |||
prefix3 = prefix1 + 'lib/petsc/' |
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.
Hmm, it's pretty bad we have this logic in two different places, we should probably clean that up...
Are you up for looking into that @RvDijk?
easybuild/easyblocks/p/petsc.py
Outdated
os.path.join(prefix2, 'include')] | ||
'files': [os.path.join(self.prefix2, 'lib', 'libpetsc.%s' % libext)], | ||
'dirs': [os.path.join(self.prefix3, 'bin'), os.path.join(self.prefix1, 'include'), | ||
os.path.join(self.prefix2, 'include')] | ||
} | ||
if LooseVersion(self.version) < LooseVersion('3.6'): | ||
custom_paths['dirs'].append(os.path.join(prefix2, 'conf')) |
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.
@RvDijk You missed a bit:
File "/tmp/eb-G_WOVv/included-easyblocks/easybuild/easyblocks/petsc.py", line 334, in sanity_check_step
custom_paths['dirs'].append(os.path.join(prefix2, 'lib', 'petsc', 'conf'))
NameError: global name 'prefix2' is not defined
Also, I'm wondering if we can come up with better names than prefix1
, prefix2
, prefix3
...
How about:
self.prefix1
->self.prefix_inc
self.prefix2
->self.prefix_lib
self.prefix3
->self.prefix_bin
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.
Those seem a lot more descriptive 👍
easybuild/easyblocks/p/petsc.py
Outdated
self.prefix2 = os.path.join(self.petsc_subdir, self.petsc_arch) | ||
|
||
if LooseVersion(self.version) >= LooseVersion("3.9"): | ||
self.prefix3 = os.path.join(prefix1, 'lib', 'petsc') |
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.
@RvDijk You forgot to change prefix1
to self.prefix1
here?
Please re-test the updated easyblock with easybuilders/easybuild-easyconfigs#6308?
self.prefix_lib = os.path.join(self.petsc_subdir, self.petsc_arch) | ||
|
||
if LooseVersion(self.version) >= LooseVersion("3.9"): | ||
self.prefix_bin = os.path.join(self.prefix_inc, 'lib', 'petsc') |
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.
@RvDijk This doesn't work currently, because the paths in sanity_check_step
method are constructed using self.prefix_*
before make_module_req_guess
is called.
You should move the code that updates self.prefix_*
into __init__
above, right below where you initialize these to ''
.
tested with both existing |
Ali's fix to the PETSc Sanity Check (commit:f09782c4) conflict with the upstream changes in easybuild 3.6.1 (easybuilders#1421) This commit reverts the Sanity Check to the stock EB version (EB 3.6.1 / 3.6.2)
The scripts in $PETSC_DIR/bin are now in $PETSC_DIR/lib/petsc/bin
https://www.mcs.anl.gov/petsc/documentation/changes/39.html