Fix LLVM easyblock for no-runtime builds (like Clang 20.1.8)#4083
Fix LLVM easyblock for no-runtime builds (like Clang 20.1.8)#4083Micket wants to merge 2 commits intoeasybuilders:developfrom
Conversation
|
Were you able to trigger this error somehow? Guarding the check for an empty |
|
Yes, it carshes hard at the end of my 3 hour clang build 😢 |
|
Yeah, disabling I'd keep your fix for sure, guarding these things is always a good idea I think. to also check if CC: @Crivella |
|
Test report by @Thyre Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (total: 1 secs) (1 easyconfigs in total) |
|
Wait, we don't need a more complicated check. If there are any files in Let me try to come up with a draft. |
|
I think this might be sufficient enough: diff --git a/easybuild/easyblocks/l/llvm.py b/easybuild/easyblocks/l/llvm.py
index d52cc8945..258b76e98 100644
--- a/easybuild/easyblocks/l/llvm.py
+++ b/easybuild/easyblocks/l/llvm.py
@@ -1613,9 +1613,6 @@ class EB_LLVM(CMakeMake):
def sanity_check_step(self, custom_paths=None, custom_commands=None, *args, **kwargs):
"""Perform sanity checks on the installed LLVM."""
- lib_dir_runtime = None
- if self.cfg['build_runtimes']:
- lib_dir_runtime = self.get_runtime_lib_path(self.installdir)
shlib_ext = '.' + get_shared_lib_ext()
if not hasattr(self, 'nvptx_target_cond'):
@@ -1808,6 +1805,10 @@ class EB_LLVM(CMakeMake):
custom_commands += ["python -c 'import clang'"]
custom_commands += ["python -c 'import mlir'"]
+ lib_dir_runtime = None
+ if self.cfg['build_runtimes'] or check_librt_files:
+ lib_dir_runtime = self.get_runtime_lib_path(self.installdir)
+
check_files.extend(os.path.join('bin', x) for x in check_bin_files)
check_files.extend(os.path.join('lib', x) for x in check_lib_files)
check_files.extend(os.path.join(lib_dir_runtime, x) for x in check_librt_files) |
|
But even then you need to protect if |
It's certainly better to miss a check than to hard crash. |
|
Feel free to submit test reports. I'll upload for Clang. |
boegel
changed the title
|
Test report by @Micket Overview of tested easyconfigs (in order)
Build succeeded for 0 out of 1 (total: 2 hours 48 mins 25 secs) (1 easyconfigs in total) |
Failing the RPATH check looks weird. Do you have the exact EasyConfig to test with? |
| custom_commands += ["python -c 'import mlir'"] | ||
|
|
||
| lib_dir_runtime = None | ||
| if self.cfg['build_runtimes'] or check_librt_files: |
There was a problem hiding this comment.
Umh this is probably the safest check. In theory we should not get any runtime files if both runtimes and openmp are off, but we only switched to using openmp as a runtime from 20 so checking for check_librt_files is probably better than checking for build_openmp
| check_files.extend(os.path.join('bin', x) for x in check_bin_files) | ||
| check_files.extend(os.path.join('lib', x) for x in check_lib_files) | ||
| check_files.extend(os.path.join(lib_dir_runtime, x) for x in check_librt_files) | ||
| if lib_dir_runtime is not None: |
There was a problem hiding this comment.
This guard should not be needed as get_runtime_lib_path will hard-fail with the default of fail_ok=False.
In general i think that if we expect runtime files to be there but we cannot locate them that should be a fail for the sanity check, not an ignore
There was a problem hiding this comment.
No this check is the only thing absolutely must have.
The code allows lib_dir_runtime to be None, and you must never do os.path.join(None, 'some_str'). It will hard crash EB itself with a TypeError.
There was a problem hiding this comment.
If check_librt_files is not empty then lib_dir_runtime should always be defined (this is enforced by adding or check_librt_files as a condition for setting check_librt_files which will fail if not found).
Otherwise we have some file that we are expecting to find but cant find and that is a sanity check failure
What i agree on is that we should give a better more clear failure than a stack trace of using os.path.join() with a None
if check_librt_files is empty that code should not fail even if lib_dir_runtime is None
>>> import os
>>> [os.path.join(None, _) for _ in []]
[]
>>>
It was an old versionbumped Clang-18.1.8-GCCcore-13.3.0.eb But i don't actually need it, since i just need libclang from llvm. But, still, at least fixing so that it doesn't crash eb is my main motivation. Not sure about the rpath'ing here. |
|
We're missing the rpath entry for the runtime library folder. That's why your build failed: So there's very likely some oversight in the EasyBlock still. |
|
Weird the code should be compiled with either rpaths or |
The EasyConfig @Micket uses skips the test suite. So that's why it passes. |
4 participants
(created using
eb --new-pr)lib_runtime_diris assignedNonevalue, but this sanity check doesn't control for that. This crashes easybuild hard asos.path.join(None, 'xxx')errors withTypeError.I don't know the whole logic here, i just protect that use of this variable.