cleanup in Mesa easyblock + add custom sanity check step #17
Conversation
…ck step in Mesa easyblock
easybuild/easyblocks/m/mesa.py
Outdated
| } | ||
| # determine list of values to pass to swr-arches configuration option | ||
| cpu_features = get_cpu_features() | ||
| self.swr_arches = sorted([feat_to_swrarch[key] for key in feat_to_swrarch if key in cpu_features]) |
There was a problem hiding this comment.
Why not "swrarch for feat, swrarch in feat_to_swrarch.items()..." as it was? More descriptive (and faster)
|
|
||
| return super(EB_Mesa, self).configure_step() | ||
|
|
||
| def install_step(self): |
There was a problem hiding this comment.
Where is this coming from? internal header doesn't sound like something a user would need. Also why is it not installed if it is meant to?
The description sounds like it belongs to a library not installed. Wouldn't that be bad if the header exist but the library does not?
There was a problem hiding this comment.
see easybuilders/easybuild-easyconfigs#9129
It seems like this is no longer needed for the most recent Mesa versions (hence the os.path.exists check)
boegel
changed the title
easybuild/easyblocks/m/mesa.py
Outdated
| gl_inc_files = ['glext.h', 'gl_mangle.h', 'glx.h', 'osmesa.h', 'gl.h', 'glxext.h', 'glx_mangle.h'] | ||
| gles_inc_files = [('GLES', 'gl.h'), ('GLES2', 'gl2.h'), ('GLES3', 'gl3.h')] |
There was a problem hiding this comment.
These sanity checks will fail with Mesa v20. There's been some changes in v20 and now most of these headers are no longer installed if building with libglvnd (as we do in EB). You can check the changes between v19 and v20 in include/meson.build. The list of headers I used in easybuilders/easybuild-easyconfigs#10175 reflects what is installed in v20.
There was a problem hiding this comment.
It's good now 👍
With Mesa v20 using -Dgallium-drivers='swrast,swr' -Dswr-arches=avx,avx2,skx, the sanity checks for
{'files': ['lib/libOSMesa.so', 'include/EGL/eglmesaext.h', 'include/EGL/eglextchromium.h', 'include/GL/osmesa.h', 'include/GL/internal/dri_interface.h', 'lib/libswrAVX.so', 'lib/libswrAVX2.so', 'lib/libswrSKX.so'], 'dirs': ['include/GL/internal']}
|
|
||
| shlib_ext = get_shared_lib_ext() | ||
|
|
||
| if LooseVersion(self.version) >= LooseVersion('20.0'): |
There was a problem hiding this comment.
Just thinking: If this changes so considerably between versions, wouldn't this be better done in the EC?
There was a problem hiding this comment.
Time will tell.
Going forward we can always easily overrule the easyblock by including sanity_check_paths in the easyconfig file.
There was a problem hiding this comment.
Ah they take precedence. Didn't know that, than 👍
@edmondac Some additional changes for easybuilders#1892 (it got a little bit out of hand, but it all makes sense imho...)