-
Notifications
You must be signed in to change notification settings - Fork 313
add support to Python easyblock for conditionally applying extra patch to fix ctypes when EasyBuild is configured to filter $LD_LIBRARY_PATH
#3860
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
Changes from 24 commits
e1425a4
1e03933
197fb18
55298c1
d8af2d2
125c23f
b1f3fcd
c2bf826
8b17a9a
d32dc4c
993f991
9fd0b9e
0c6d7ab
4ec46bb
817c51c
67b35ec
4510e29
3b64e2b
c622e77
247c394
cbab116
13471fd
c9f47a3
51cb613
dd442a3
15fdc3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -302,6 +302,13 @@ def extra_options(): | |
| 'ulimit_unlimited': [False, "Ensure stack size limit is set to '%s' during build" % UNLIMITED, CUSTOM], | ||
| 'use_lto': [None, "Build with Link Time Optimization (>= v3.7.0, potentially unstable on some toolchains). " | ||
| "If None: auto-detect based on toolchain compiler (version)", CUSTOM], | ||
| 'patch_ctypes_ld_library_path': [None, | ||
| "The ctypes module strongly relies on LD_LIBRARY_PATH to find " | ||
| "libraries. This allows specifying a patch that will only be " | ||
| "applied if EasyBuild is configured to filter LD_LIBRARY_PATH, in " | ||
| "order to make sure ctypes can still find libraries without it. " | ||
| "Please make sure to add the checksum for this patch to 'checksums'.", | ||
| CUSTOM], | ||
| } | ||
| return ConfigureMake.extra_options(extra_vars) | ||
|
|
||
|
|
@@ -345,6 +352,62 @@ def _get_pip_ext_version(self): | |
| return ext[1] | ||
| return None | ||
|
|
||
| def fetch_step(self, *args, **kwargs): | ||
| """ | ||
| Custom fetch step for Python: add patches from patches_custom_ctypes to 'patches' if | ||
| EasyBuild is configured to filter LD_LIBRARY_PATH (and is configured not to filter LIBRARY_PATH). | ||
| This needs to be done in (or before) the fetch step to ensure that those patches are also fetched. | ||
| """ | ||
| # If we filter out LD_LIBRARY_PATH (not unusual when using rpath), ctypes is not able to dynamically load | ||
| # libraries installed with EasyBuild (see https://github.com/EESSI/software-layer/issues/192). | ||
| # If EasyBuild is configured to filter LD_LIBRARY_PATH any patches listed in `patches_custom_ctypes` | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| # are added to the list of patches. Also, we add the checksums_filter_ld_library_path to the checksums list in | ||
| # that case. | ||
| # This mechanism e.g. makes sure we can patch ctypes, which normally strongly relies on LD_LIBRARY_PATH to find | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| # libraries. But, we want to do the patching conditionally on EasyBuild configuration (i.e. which env vars | ||
| # are filtered), hence this setup based on the custom config option 'patches_custom_ctypes' | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| filtered_env_vars = build_option('filter_env_vars') or [] | ||
| patch_ctypes_ld_library_path = self.cfg.get('patch_ctypes_ld_library_path') | ||
| if ( | ||
| 'LD_LIBRARY_PATH' in filtered_env_vars and | ||
| 'LIBRARY_PATH' not in filtered_env_vars and | ||
| patch_ctypes_ld_library_path | ||
| ): | ||
| # Some sanity checking so we can raise an early and clear error if needed | ||
| # We expect a (one) checksum for the patch_ctypes_ld_library_path | ||
| checksums = self.cfg['checksums'] | ||
| sources = self.cfg['sources'] | ||
| patches = self.cfg.get('patches') | ||
| len_patches = len(patches) if patches else 0 | ||
| if len_patches + len(sources) + 1 == len(checksums): | ||
| msg = "EasyBuild was configured to filter LD_LIBRARY_PATH (and not to filter LIBRARY_PATH). " | ||
| msg += "The ctypes module relies heavily on LD_LIBRARY_PATH for locating its libraries. " | ||
| msg += "The following patches will be applied to make sure ctypes.CDLL, ctypes.cdll.LoadLibrary " | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| msg += f"and ctypes.util.find_library will still work correctly: {patch_ctypes_ld_library_path}." | ||
| self.log.info(msg) | ||
| self.log.info(f"Original list of patches: {self.cfg['patches']}") | ||
| self.log.info(f"Patch to be added: {patch_ctypes_ld_library_path}") | ||
| self.cfg.update('patches', [patch_ctypes_ld_library_path]) | ||
| self.log.info(f"Updated list of patches: {self.cfg['patches']}") | ||
| else: | ||
| msg = "The length of 'checksums' (%s) was not equal to the total amount of sources (%s) + patches (%s)" | ||
| msg += ". Did you forget to add a checksum for patch_ctypes_ld_library_path?." | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| raise EasyBuildError(msg, len(checksums), len(sources), len(len_patches + 1)) | ||
| # If LD_LIBRARY_PATH is filtered, but no patch is specified, warn the user that his may not work | ||
| elif ( | ||
| 'LD_LIBRARY_PATH' in filtered_env_vars and | ||
| 'LIBRARY_PATH' not in filtered_env_vars and | ||
| not patch_ctypes_ld_library_path | ||
| ): | ||
| msg = "EasyBuild was configured to filter LD_LIBRARY_PATH (and not to filter LIBRARY_PATH). " | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| msg += "However, no patch for ctypes was specified through 'patch_ctypes_ld_library_path' in the " | ||
| msg += "EasyConfig. Note that ctypes.util.find_library, ctypes.CDLL and ctypes.cdll.LoadLibrary heavily " | ||
| msg += "rely on LD_LIBRARY_PATH. Without the patch, a setup without LD_LIBRARY_PATH will likely not work " | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| msg += "correctly." | ||
| self.log.warning(msg) | ||
|
|
||
| super().fetch_step(*args, **kwargs) | ||
|
|
||
| def patch_step(self, *args, **kwargs): | ||
| """ | ||
| Custom patch step for Python: | ||
|
|
@@ -357,33 +420,6 @@ def patch_step(self, *args, **kwargs): | |
| # Ignore user site dir. -E ignores PYTHONNOUSERSITE, so we have to add -s | ||
| apply_regex_substitutions('configure', [(r"(PYTHON_FOR_BUILD=.*-E)'", r"\1 -s'")]) | ||
|
|
||
| # If we filter out LD_LIBRARY_PATH (not unusual when using rpath), ctypes is not able to dynamically load | ||
| # libraries installed with EasyBuild (see https://github.com/EESSI/software-layer/issues/192). | ||
| # ctypes is using GCC (and therefore LIBRARY_PATH) to figure out the full location but then only returns the | ||
| # soname, instead let's return the full path in this particular scenario | ||
| filtered_env_vars = build_option('filter_env_vars') or [] | ||
| if 'LD_LIBRARY_PATH' in filtered_env_vars and 'LIBRARY_PATH' not in filtered_env_vars: | ||
| ctypes_util_py = os.path.join("Lib", "ctypes", "util.py") | ||
| orig_gcc_so_name = None | ||
| # Let's do this incrementally since we are going back in time | ||
| if LooseVersion(self.version) >= "3.9.1": | ||
| # From 3.9.1 to at least v3.12.4 there is only one match for this line | ||
| orig_gcc_so_name = "_get_soname(_findLib_gcc(name)) or _get_soname(_findLib_ld(name))" | ||
| if orig_gcc_so_name: | ||
| orig_gcc_so_name_regex = r'(\s*)' + re.escape(orig_gcc_so_name) + r'(\s*)' | ||
| # _get_soname() takes the full path as an argument and uses objdump to get the SONAME field from | ||
| # the shared object file. The presence or absence of the SONAME field in the ELF header of a shared | ||
| # library is influenced by how the library is compiled and linked. For manually built libraries we | ||
| # may be lacking this field, this approach also solves that problem. | ||
| updated_gcc_so_name = ( | ||
| "_findLib_gcc(name) or _findLib_ld(name)" | ||
| ) | ||
| apply_regex_substitutions( | ||
| ctypes_util_py, | ||
| [(orig_gcc_so_name_regex, r'\1' + updated_gcc_so_name + r'\2')], | ||
| on_missing_match=ERROR | ||
| ) | ||
|
|
||
| # if we're installing Python with an alternate sysroot, | ||
| # we need to patch setup.py which includes hardcoded paths like /usr/include and /lib64; | ||
| # this fixes problems like not being able to build the _ssl module ("Could not build the ssl module") | ||
|
|
@@ -685,6 +721,45 @@ def install_step(self): | |
| symlink(target_lib_dynload, lib_dynload) | ||
| change_dir(cwd) | ||
|
|
||
| def _sanity_check_ctypes_ld_library_path_patch(self): | ||
| """Check that the patch for ctypes that should be applied when filtering LD_LIBRARY_PATH works""" | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| # Try find_library first, since ctypes.CDLL relies on that to work correctly | ||
| cmd = "python -c 'from ctypes import util; print(util.find_library(\"libpython3.so\"))'" | ||
| res = run_shell_cmd(cmd) | ||
| out = res.output.strip() | ||
| escaped_python_root = re.escape(self.installdir) | ||
| pattern = rf"^{escaped_python_root}.*libpython3\.so$" | ||
| match = re.match(pattern, out) | ||
| self.log.debug(f"Matching regular expression pattern {pattern} to string {out}") | ||
| if match: | ||
| msg = "Call to ctypes.util.find_library('libpython3.so') succesfully found libpython3.so under the prefix " | ||
| msg += "of the current python installation. indicating that the patch that fixes ctypes when EasyBuild is " | ||
| msg += "configured to filter LD_LIBRARY_PATH was applied succesfully." | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| self.log.info(msg) | ||
| else: | ||
| msg = "Finding the library libpython3.so using ctypes.util.find_library('libpython3.so') failed. " | ||
| msg += "Ctypes requires a patch when EasyBuild is configured to filter LD_LIBRARY_PATH. " | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| msg += "Please check if you specified a patch through patch_ctypes_ld_library_path and check " | ||
| msg += "the logs to see if it applied correctly." | ||
| raise EasyBuildError(msg) | ||
| # Now that we know find_library was patched correctly, check if ctypes.CDLL is also patched correctly | ||
| cmd = "python -c 'import ctypes; print(ctypes.CDLL(\"libpython3.so\"))'" | ||
| res = run_shell_cmd(cmd) | ||
| out = res.output.strip() | ||
| pattern = rf"^<CDLL '{escaped_python_root}.*libpython3\.so', handle [a-f0-9]+ at 0x[a-f0-9]+>$" | ||
| match = re.match(pattern, out) | ||
| self.log.debug(f"Matching regular expression pattern {pattern} to string {out}") | ||
| if match: | ||
| msg = "Call to ctypes.CDLL('libpython3.so') succesfully opened libpython3.so, indicating that the patch " | ||
| msg += "that fixes ctypes when EasyBuild is configured to filter LD_LIBRARY_PATH was applied succesfully." | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| self.log.info(msg) | ||
| else: | ||
| msg = "Opening of libpython3.so using ctypes.CDLL('libpython3.so') failed. " | ||
| msg += "Ctypes requires a patch when EasyBuild is configured to filter LD_LIBRARY_PATH. " | ||
|
boegel marked this conversation as resolved.
Outdated
|
||
| msg += "Please check if you specified a patch through patch_ctypes_ld_library_path and check " | ||
| msg += "the logs to see if it applied correctly." | ||
| raise EasyBuildError(msg) | ||
|
|
||
| def _sanity_check_ebpythonprefixes(self): | ||
| """Check that EBPYTHONPREFIXES works""" | ||
| temp_prefix = tempfile.mkdtemp(suffix='-tmp-prefix') | ||
|
|
@@ -749,6 +824,18 @@ def sanity_check_step(self): | |
| if self.cfg.get('ebpythonprefixes'): | ||
| self._sanity_check_ebpythonprefixes() | ||
|
|
||
| # If the conditions for applying the patch specified through patch_ctypes_ld_library_path are met, check that | ||
| # the patch applied correctly (and fixed the issue). Note that the condition should be identical to the one | ||
| # used to determine if the patch_ctypes_ld_library_path patch should be applied | ||
| filtered_env_vars = build_option('filter_env_vars') or [] | ||
| patch_ctypes_ld_library_path = self.cfg.get('patch_ctypes_ld_library_path') | ||
| if ( | ||
| 'LD_LIBRARY_PATH' in filtered_env_vars and | ||
| 'LIBRARY_PATH' not in filtered_env_vars and | ||
| patch_ctypes_ld_library_path | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we remove the condition that a patch is applied? In other words: should we allow a user to build with filtered
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm inclined to simply always run this additional sanity check, regardless of whether or not But that won't work, since the unpatched With that in mind, I remove |
||
| ): | ||
| self._sanity_check_ctypes_ld_library_path_patch() | ||
|
|
||
| pyver = 'python' + self.pyshortver | ||
| custom_paths = { | ||
| 'files': [ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.