add support to Python easyblock for conditionally applying extra patch to fix ctypes when EasyBuild is configured to filter $LD_LIBRARY_PATH#3860
Conversation
…iltering LD_LIBRARY_PATH
…e the patch isn't fetched
Co-authored-by: ocaisa <alan.ocais@cecam.org>
|
Code style check still fails: |
|
Ok, sanity check seems to work as intended. Using the patch from easybuilders/easybuild-easyconfigs#23499 I get a successful sanity check, with this in the debugging output: If I use a dummy patch that just inserts some comments, I get: |
|
@boegelbot please test @ jsc-zen3 |
|
@casparvl: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3407980100 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 3 (3 easyconfigs in total) |
|
3.11.5 failed because I was rebuilding that in easybuilders/easybuild-easyconfigs#23499 as well, so lock file existed. Trying again for good measure... @boegelbot please test @ jsc-zen3 |
|
@casparvl: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3408456358 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
…f that works, since ctypes.CDLL relies on it. That way, if the ctypes.CDLL fails, we at least know if the find_library call was successful
|
Strangely enough, I got a failure from the sanity check for 3.10.8: But inspecting the files that should have been patched, it seems the patch applied correctly I therefore installed with Anyway, to help debug, I'll add a |
|
I've tried with the That is unexpected: we would expect it to return the full path - and as mentioned if I skip the sanity check, finalize the build, and then run that command, it returns the full path just fine. @boegel any ideas perhaps why this would be different between 3.10.8 and 3.11.5? :\ Edit: Just to add, running e.g. and then causes the sanity check to pass just fine. |
|
Ok, the reason turned out to be an issue where the patch would just return the libname if that library was present in the current working directory. That happened to be the case for |
|
Test report by @boegel Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 2 (2 easyconfigs in total) edit: Failure is due to the patch from easybuilders/easybuild-easyconfigs#24189 not being used, I'll look into that (probably outdated easyconfigs installation on my end) |
boegel
left a comment
There was a problem hiding this comment.
@casparvl All suggestions tackled in casparvl#5
easybuild/easyblocks/p/python.py
Outdated
| if ( | ||
| 'LD_LIBRARY_PATH' in filtered_env_vars and | ||
| 'LIBRARY_PATH' not in filtered_env_vars and | ||
| patch_ctypes_ld_library_path |
There was a problem hiding this comment.
I'm inclined to simply always run this additional sanity check, regardless of whether or not $LD_LIBRARY_PATH is being filtered.
But that won't work, since the unpatched find_library won't find a hit for libpython3.so (only for python3).
With that in mind, I remove patch_ctypes_ld_library_path from this condition: we want to make sure that if $LD_LIBRARY_PATH is filtered, that we still have a working find_library...
…rectly working + minor changes to comments & log messages
relax condition in Python easyblock to run sanity check on ctypes correctly working + minor changes to comments & log messages
|
@boegelbot please test @ jsc-zen3 |
|
@boegel: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3431402779 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 4 out of 4 (4 easyconfigs in total) |
$LD_LIBRARY_PATH$LD_LIBRARY_PATH
| """ | ||
| Custom fetch step for Python. | ||
|
|
||
| Add patch specified in patch_ctypes_ld_library_path to list of patches if |
There was a problem hiding this comment.
It's better to do this in the constructor, see:
This change introduces extra easyblock-specific configuration items. They allow us to define patches that should be applied conditionally if EasyBuild is configured to filter
LD_LIBRARY_PATH. Since these patches (see Should be used together with: easybuilders/easybuild-easyconfigs#23499) are quite fundamental (changectypesfunctions) we prefer them to only be applied in the scenario where they are really needed.Replaces: #3798
Should be used together with: easybuilders/easybuild-easyconfigs#23499
edit: relevant EESSI support issue: https://gitlab.com/eessi/support/-/issues/154