From 75509c46497f3b7ddde971a6cbd22635b08d9251 Mon Sep 17 00:00:00 2001 From: Rok Mandeljc Date: Thu, 12 Sep 2024 13:21:51 +0200 Subject: [PATCH 1/2] hooks: update gribapi hook for compatibility with eccodes v2.37.0 Starting with `eccodes` v2.37.0, upstream provides binary wheels with bundled copy of the `eccodes` shared library. Adjust the hook for `gribapi` (part of `eccodes` dist) to account for that. Also handle the issue with circular imports by importing `eccodes` before `gribapi.bindings` when trying to obtain the path to the shared library. --- .github/workflows/pr-test.yml | 5 +- .../stdhooks/hook-gribapi.py | 51 +++++++++++++++++-- news/799.update.rst | 3 ++ requirements-test-libraries.txt | 3 +- tests/test_libraries.py | 32 +++++++++--- 5 files changed, 79 insertions(+), 15 deletions(-) create mode 100644 news/799.update.rst diff --git a/.github/workflows/pr-test.yml b/.github/workflows/pr-test.yml index a99b8adae..3b2b3cf37 100644 --- a/.github/workflows/pr-test.yml +++ b/.github/workflows/pr-test.yml @@ -111,8 +111,6 @@ jobs: # These are dependencies of gmsh sudo apt-get install -y libglu1 libgl1 libxrender1 libxcursor1 libxft2 \ libxinerama1 libgomp1 - # This one is required by eccodes - sudo apt-get install -y libeccodes0 - name: Install brew dependencies if: startsWith(matrix.os, 'macos') @@ -125,7 +123,8 @@ jobs: brew install libdiscid # Install lsl library for pylsl brew install labstreaminglayer/tap/lsl - # This one is required by eccodes + # This one is required by eccodes (binary wheels with bundled eccodes + # library are provided only for macOS 13+). brew install eccodes - name: Install dependencies diff --git a/_pyinstaller_hooks_contrib/stdhooks/hook-gribapi.py b/_pyinstaller_hooks_contrib/stdhooks/hook-gribapi.py index c1ad320d1..45d3f0ae2 100644 --- a/_pyinstaller_hooks_contrib/stdhooks/hook-gribapi.py +++ b/_pyinstaller_hooks_contrib/stdhooks/hook-gribapi.py @@ -11,16 +11,39 @@ # ------------------------------------------------------------------ import os +import pathlib -from PyInstaller.utils.hooks import collect_data_files, get_module_attribute, logger +from PyInstaller import isolated +from PyInstaller.utils.hooks import collect_data_files, logger # Collect the headers (eccodes.h, gribapi.h) that are bundled with the package. datas = collect_data_files('gribapi') -# Collect the eccodes shared library +# Collect the eccodes shared library. Starting with eccodes 2.37.0, binary wheels with bundled shared library are +# provided for linux and macOS. + + +# NOTE: custom isolated function is used here instead of `get_module_attribute('gribapi.bindings', 'library_path')` +# hook utility function because with eccodes 2.37.0, `eccodes` needs to be imported before `gribapi` to avoid circular +# imports... Also, this way, we can obtain the root directory of eccodes package at the same time. +@isolated.decorate +def get_eccodes_library_path(): + import eccodes + import gribapi.bindings + + return ( + # Path to eccodes shared library used by the gribapi bindings. + str(gribapi.bindings.library_path), + # Path to eccodes package (implicitly assumed to be next to the gribapi package, since they are part of the + # same eccodes dist). + str(eccodes.__path__[0]), + ) + + binaries = [] + try: - library_path = get_module_attribute('gribapi.bindings', 'library_path') + library_path, package_path = get_eccodes_library_path() except Exception: logger.warning("hook-gribapi: failed to query gribapi.bindings.library_path!", exc_info=True) library_path = None @@ -38,5 +61,23 @@ logger.warning("hook-gribapi: could not determine path to eccodes shared library!") if library_path: - logger.debug("hook-gribapi: collecting eccodes shared library: %r", library_path) - binaries.append((library_path, '.')) + # If we are collecting eccodes shared library that is bundled with eccodes >= 2.37.0 binary wheel, attempt to + # preserve its parent directory layout. This ensures that the library is found at run-time, but implicitly requires + # PyInstaller 6.x, whose binary dependency analysis (that might also pick up this shared library) also preserves the + # parent directory layout of discovered shared libraries. With PyInstaller 5.x, this will result in duplication + # because binary dependency analysis collects into top-level application directory, but that copy will not be + # discovered at run-time, so duplication is unavoidable. + library_parent_path = pathlib.PurePath(library_path).parent + package_parent_path = pathlib.PurePath(package_path).parent + + if package_parent_path in library_parent_path.parents: + # Should end up being `eccodes.libs` on Linux, and `eccodes/.dylib` on macOS). + dest_dir = str(library_parent_path.relative_to(package_parent_path)) + else: + # External copy; collect into top-level application directory. + dest_dir = '.' + + logger.info( + "hook-gribapi: collecting eccodes shared library %r to destination directory %r", library_path, dest_dir + ) + binaries.append((library_path, dest_dir)) diff --git a/news/799.update.rst b/news/799.update.rst new file mode 100644 index 000000000..78cdfd25a --- /dev/null +++ b/news/799.update.rst @@ -0,0 +1,3 @@ +Update ``gribapi`` hook for compatibility with ``eccodes`` v2.37.0, +to account for possibility of bundles ``eccodes`` shared library, which +is provided by newly-introduced binary wheels for Linux and macOS 13+. diff --git a/requirements-test-libraries.txt b/requirements-test-libraries.txt index 2606c03f5..e8f50ff69 100644 --- a/requirements-test-libraries.txt +++ b/requirements-test-libraries.txt @@ -226,7 +226,8 @@ pysaml2==7.3.0; python_version < '3.9' # ------------------- Platform (OS) specifics # eccodes package requires the eccodes shared library provided by the environment (linux distribution, homebrew, or Anaconda). -eccodes==1.7.1; sys_platform == 'darwin' or sys_platform == 'linux' +# Starting with v2.37.0, binary wheels with bundled shared library are provided for linux and macOS 13+. +eccodes==2.37.0; sys_platform == 'darwin' or sys_platform == 'linux' # dbus-fast has pre-built wheels only for Linux; and D-Bus is available only there, anyway. dbus-fast==2.24.2; sys_platform == 'linux' diff --git a/tests/test_libraries.py b/tests/test_libraries.py index ff01beb23..c3270a49f 100644 --- a/tests/test_libraries.py +++ b/tests/test_libraries.py @@ -2034,11 +2034,15 @@ def test_schwifty(pyi_builder): """) -@importorskip('gribapi') +@importorskip('eccodes') def test_eccodes_gribapi(pyi_builder): pyi_builder.test_source(""" import sys import os + import pathlib + + # With eccodes 2.37.0, eccodes needs to be imported before gribapi to avoid circular imports. + import eccodes # Basic import test import gribapi @@ -2046,12 +2050,28 @@ def test_eccodes_gribapi(pyi_builder): # Ensure that the eccodes shared library is bundled with the frozen application. import gribapi.bindings - lib_filename = os.path.join( - sys._MEIPASS, - os.path.basename(gribapi.bindings.library_path), - ) + print(f"gribapi.bindings.library_path={gribapi.bindings.library_path}") - assert os.path.isfile(lib_filename), f"Shared library {lib_filename!s} not found!" + library_path = gribapi.bindings.library_path + if os.path.basename(library_path) == library_path: + # Only library basename is given - assume this is a system-wide copy that was collected + # into top-level application directory and loaded via `findlibs.find()`/`ctypes`. + expected_library_file = os.path.join( + sys._MEIPASS, + library_path, + ) + if not os.path.isfile(expected_library_file): + raise RuntimeError(f"Shared library {expected_library_file!s} not found!") + else: + # Absolute path; check that it is rooted in top-level application directory. This covers all valid locations + # as per https://github.com/ecmwf/eccodes-python/blob/2.37.0/gribapi/bindings.py#L61-L64, + # - sys._MEIPASS/eccodes + # - sys._MEIPASS/eccodes.libs + # - sys._MEIPASS/eccodes/.dylibs + if pathlib.PurePath(sys._MEIPASS) not in pathlib.PurePath(library_path).parents: + raise RuntimeError( + f"Shared library path {library_path} is not rooted in top-level application directory!" + ) """) From eb27b5ba27e468c657223d90227aa64be6a21c02 Mon Sep 17 00:00:00 2001 From: Rok Mandeljc Date: Thu, 12 Sep 2024 16:13:02 +0200 Subject: [PATCH 2/2] hooks: add run-time hook for findlibs Add run-time hook for `findlibs` that overrides `findlibs.find()` function with custom implementation that gives precedence to searching `sys._MEIPASS` and then immediately falling back to `ctypes.util.find_library`. The original implementation searches a set of hard-coded paths before falling back to `ctypes.util.find_library`; those path include Homebrew directory on macOS, so if that happens to be installed on the run-time system, the original `findlibs.find()` implementation ends up returning system-wide copy of the library (if it is available) instead of the bundled one. Which, especially on macOS, can lead to problems due to (lack of proper) signature on the system-wide copy of the library when application itself is signed with developer identity. that searches a set of hard-coded paths before --- _pyinstaller_hooks_contrib/rthooks.dat | 1 + .../rthooks/pyi_rth_findlibs.py | 52 +++++++++++++++++++ news/799.new.rst | 7 +++ tests/test_libraries.py | 3 ++ 4 files changed, 63 insertions(+) create mode 100644 _pyinstaller_hooks_contrib/rthooks/pyi_rth_findlibs.py create mode 100644 news/799.new.rst diff --git a/_pyinstaller_hooks_contrib/rthooks.dat b/_pyinstaller_hooks_contrib/rthooks.dat index 538e94909..f868f56f7 100644 --- a/_pyinstaller_hooks_contrib/rthooks.dat +++ b/_pyinstaller_hooks_contrib/rthooks.dat @@ -1,6 +1,7 @@ { 'cryptography': ['pyi_rth_cryptography_openssl.py'], 'enchant': ['pyi_rth_enchant.py'], + 'findlibs': ['pyi_rth_findlibs.py'], 'ffpyplayer': ['pyi_rth_ffpyplayer.py'], 'osgeo': ['pyi_rth_osgeo.py'], 'traitlets': ['pyi_rth_traitlets.py'], diff --git a/_pyinstaller_hooks_contrib/rthooks/pyi_rth_findlibs.py b/_pyinstaller_hooks_contrib/rthooks/pyi_rth_findlibs.py new file mode 100644 index 000000000..43d3db11d --- /dev/null +++ b/_pyinstaller_hooks_contrib/rthooks/pyi_rth_findlibs.py @@ -0,0 +1,52 @@ +#----------------------------------------------------------------------------- +# Copyright (c) 2024, PyInstaller Development Team. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# +# The full license is in the file COPYING.txt, distributed with this software. +# +# SPDX-License-Identifier: Apache-2.0 +#----------------------------------------------------------------------------- + +# Override the findlibs.find() function to give precedence to sys._MEIPASS, followed by `ctypes.util.find_library`, +# and only then the hard-coded paths from the original implementation. The main aim here is to avoid loading libraries +# from Homebrew environment on macOS when it happens to be present at run-time and we have a bundled copy collected from +# the build system. This happens because we (try not to) modify `DYLD_LIBRARY_PATH`, and the original `findlibs.find()` +# implementation gives precedence to environment variables and several fixed/hard-coded locations, and uses +# `ctypes.util.find_library` as the final fallback... +def _pyi_rthook(): + import sys + import os + import ctypes.util + + import findlibs + + _orig_find = getattr(findlibs, 'find', None) + + def _pyi_find(lib_name, pkg_name=None): + pkg_name = pkg_name or lib_name + extension = findlibs.EXTENSIONS.get(sys.platform, ".so") + + # First check sys._MEIPASS + fullname = os.path.join(sys._MEIPASS, "lib{}{}".format(lib_name, extension)) + if os.path.isfile(fullname): + return fullname + + # Fall back to `ctypes.util.find_library` (to give it precedence over hard-coded paths from original + # implementation). + lib = ctypes.util.find_library(lib_name) + if lib is not None: + return lib + + # Finally, fall back to original implementation + if _orig_find is not None: + return _orig_find(lib_name, pkg_name) + + return None + + findlibs.find = _pyi_find + + +_pyi_rthook() +del _pyi_rthook diff --git a/news/799.new.rst b/news/799.new.rst new file mode 100644 index 000000000..73fd3b0df --- /dev/null +++ b/news/799.new.rst @@ -0,0 +1,7 @@ +Add run-time hook for ``findlibs`` that overrides the ``findlibs.find`` +function with custom implementation in order to ensure that the top-level +application directory is searched first. This prevents a system-wide +copy of the library being found and loaded instead of the bundled copy +when the system-wide copy happens to be available in one of fixed +locations that is scanned by the original implementation of ``findlibs.find`` +(for example, Homebrew directory on macOS). diff --git a/tests/test_libraries.py b/tests/test_libraries.py index c3270a49f..3233a986e 100644 --- a/tests/test_libraries.py +++ b/tests/test_libraries.py @@ -2068,6 +2068,9 @@ def test_eccodes_gribapi(pyi_builder): # - sys._MEIPASS/eccodes # - sys._MEIPASS/eccodes.libs # - sys._MEIPASS/eccodes/.dylibs + # as well as sys._MEIPASS itself (in case system-wide copy was collected into top-level application + # directory but is reported with full path instead of just basename due to our override of `findlibs.find()` + # via run-time hook). if pathlib.PurePath(sys._MEIPASS) not in pathlib.PurePath(library_path).parents: raise RuntimeError( f"Shared library path {library_path} is not rooted in top-level application directory!"