[petrasovaa]

Importing grass.lib (ctypes interface to the C libraries) fails with ImportError: Could not load grass_gis... in a Python process that set up a GRASS session with gj.init() or gs.setup.init(), e.g., in a Jupyter notebook. The session setup sets the dynamic library search path variable (LD_LIBRARY_PATH on Linux), but the dynamic linker reads it only at process startup, so in the already running process the GRASS libraries cannot resolve their dependencies on each other (unless the libraries' RUNPATH, i.e. the configured install prefix, happens to be valid).

The library preloading approach is borrowed from the grass-session package (load_libs()), with a few differences:

• Loads only libgrass_* libraries (grass-session loads every *.so in the directory) and also globs *.dylib, so it works on macOS builds where grass-session's .so-only pattern finds nothing.
• Skips Windows entirely: there the ctypes loader already registers directories from PATH via os.add_dll_directory() when grass.lib is imported, which happens after init has extended PATH, so preloading is not needed.
• Failures are silently skipped instead of raising: a library that cannot load (e.g., missing optional system dependency like OpenGL) should not break init for users who never import it, and its grass.lib import fails the same way with or without preloading.
• The retry loop builds a new list each pass and stops when a pass makes no progress, avoiding grass-session's mutation of the list while iterating over it.
• No mode=1 (RTLD_LAZY) argument: ctypes always adds RTLD_NOW on POSIX, so the flag has no effect; the default mode matches what the generated grass.lib loader uses.

[wenzeslaus]

Loading the libraries needs to be opt-in: an init(..., load_libs=False) parameter (default off), so that sessions which only call tools do not pull the whole GRASS C stack and its transitive dependencies (GDAL, PROJ, GL, X11, ...) into the process on every init() (and on create_project() when called outside a session, and grass lock-mapset). The test needs to be a pytest file which is also executed completely standalone, i.e., in a separate CI step on all platforms (Linux, macOS, Windows) with a clean environment, not only as part of the regular pytest run where the installed prefix matches the configure prefix and the test passes even without the preload. Please also deduplicate the library list using Path.resolve() (the glob currently picks up both the symlink and the versioned file, doubling the dlopen calls).

Fable 5 review (details)

Here is an AI-boosted analysis (Claude Fable 5). The findings were checked against a local Autotools build (dist.x86_64-pc-linux-gnu, Linux, glibc) with the PR's preload_dynamic_libraries function copied verbatim into standalone scripts; Windows and macOS claims are from reading the code and documentation only.

1. Alphabetical load order can bind libraries to two different copies of libgrass_gis (reproduced locally). sorted(glob) loads libgrass_arraystats before libgrass_gis. When a same-SONAME libgrass_gis.8.5.so is reachable through the process-startup search path (inherited LD_LIBRARY_PATH from another install, or the libraries' RUNPATH pointing to an existing install), the dynamic linker resolves arraystats' NEEDED entry to that other copy, and the later explicit load of $GISBASE/lib/libgrass_gis.8.5.so maps a second image. Reproduced with two copies of the lib directory: /proc/self/maps shows two libgrass_gis.8.5.so and two libgrass_raster.8.5.so images; grass.lib.gis.G_gisinit() initializes one copy while grass.lib.raster is bound to the other, so Rast_open_old() fails with "System not initialized. Programmer forgot to call G_gisinit()". Both controls (no preload; preload without the second copy on the path) work. So the "RUNPATH happens to be valid" case, which works today, becomes a runtime failure. Options: load in dependency order (datetime, gis, then the rest; a single pass, no retry loop), or preload only when the directory is not already on the search path.

2. The preload runs unconditionally and ignores env. setup_runtime_env calls it for every caller: init() with env=os.environ.copy(), grass lock-mapset, and gs.create_project() through ensure_runtime_env() when called outside a session (the usual create_project() then init() script; inside a session ensure_runtime_env() is a no-op). Measured on the local build: 54 GRASS libraries plus 130 distinct non-GRASS shared objects get mapped into a bare Python process (ldd over all libgrass_*): libgrass_raster/vector/gproj pull GDAL and through it hdf5, netcdf, poppler, libpq, mysqlclient; libgrass_nviz/ogsf/cairodriver pull GL and X11; libgrass_gis pulls PROJ and curl. This cannot be undone by finish(). A process which already has another PROJ/GDAL loaded (e.g., a Jupyter kernel with pyproj or rasterio) gets a second copy where previously only an explicit import grass.lib did that. It also pins the process to the first install: init(A) followed by init(grass_path=B) resolves B's NEEDED entries against A's already-loaded copies. This is the reason for the opt-in requirement above. An alternative or complement is to do the loading lazily in the grass.lib package __init__ (libgrass__init__.py, which since #7605 already registers $GISBASE/lib with the loader), so that only grass.lib users pay for it, once per process; that needs a re-registration hook for the case where grass.lib was imported before init().

3. The updated init() docstring over-claims for the env=os.environ.copy() case. The grass.lib loader reads GISBASE and LD_LIBRARY_PATH from os.environ only (libgrass__init__.py, libraryloader.py). Verified: preloading all libraries by path with GISBASE and LD_LIBRARY_PATH unset in os.environ and then import grass.lib.gis gives ImportError: Could not load grass_gis.8.5, identical to no preload. A user following the docstring's own init(path, env=os.environ.copy()) example gets the error the docstring says is gone. Options: scope the claim to the global-environment case, or make setup_runtime_env pass gisbase/lib to grass.lib.ctypes_loader.add_library_search_dirs() explicitly (no environment involved), which together with the preload makes the import work with any env.

4. The test does not guard the fix on CI. It only removes LD_LIBRARY_PATH/DYLD_LIBRARY_PATH. Since #7605 the top-level library is loaded by absolute path from $GISBASE/lib, and its NEEDED dependencies resolve through the libraries' RUNPATH (readelf -d libgrass_raster.so shows the configure prefix). The CI build scripts configure with --prefix equal to the install directory, so RUNPATH is valid there and the test passes with the preload_dynamic_libraries() call removed. Locally, with the build tree not installed to its prefix, the preload is genuinely needed (import grass.lib.gis fails without it, both in the build tree and in a relocated copy of lib/), which shows what the test needs: a relocated library directory (a copy of $GISBASE/lib under a temporary fake gisbase with an invalid RUNPATH), or an assertion that the libgrass_* images are mapped from $GISBASE/lib (Linux: /proc/self/maps). This is the reason for the standalone CI step above; on macOS the step can move the install prefix after make install and run the test against the moved prefix.

5. The Windows rationale in the docstring is inaccurate. Since Python 3.8, ctypes loads DLLs with the "most secure" LoadLibraryEx flags: load-time dependencies are resolved from the DLL's own directory, system directories, and directories registered with os.add_dll_directory(); PATH is not used. What makes grass.lib work on Windows is libraryloader.py:375-379, which registers every PATH entry with os.add_dll_directory() once, when the loader is first instantiated. So "not needed on Windows" holds only if grass.lib is first imported after init() in the global environment; a process which imported grass.lib before init() never gets the new PATH entries registered. Suggested wording: cite os.add_dll_directory in the loader. A CI experiment to settle it: import grass.lib before init(), then init(), then import grass.lib.gis.

6. The macOS claim is unverified. The build system has no explicit -rpath/install_name handling (include/Make, configure.ac), the Darwin LD_LIBRARY_PATH_VAR is LD_RUN_PATH which dyld never reads, and the test pops DYLD_LIBRARY_PATH which GRASS never sets, so the current macOS CI run exercises nothing new. Preloading by full path plausibly helps for a relocated install via dyld's already-loaded-image lookup, but nothing demonstrates it. The standalone CI step with a moved prefix (point 4) would verify or refute this.

7. All OSErrors are swallowed with no trace. A library that fails to preload (wrong gisbase from the get_install_path() fallback, missing libproj or GL, mismatched libraries) leaves no diagnostic, and the later grass.lib ImportError does not mention the preload, which makes exactly the bug this PR targets harder to diagnose. An option which keeps the silent path for tool-only users: return the list of (path, error) pairs from preload_dynamic_libraries(), ignore the return value in setup_runtime_env, and expose it through a diagnostic API or under GRASS_DEBUG.

8. Glob duplicates and repeated work. lib_path.glob("libgrass_*.so") matches both the unversioned symlink and the versioned file (108 entries for 54 libraries). Measured: 200 CDLL calls in 57 ms per preload versus 100 calls in 51 ms after {p.resolve() for p in ...}; a single dependency-ordered pass would be 54 calls. The time is dominated by the first load of GDAL and friends, so dedup mainly removes redundant work rather than time. The preload is repeated on every init()/create_project() in the same process; a module-level record of already-preloaded install paths would avoid that and could warn when a different install path is requested in a process already bound to another one.

Also considered and found to work as intended: the glob on CMake and distribution layouts (the versioned libgrass_gis.8.5.so still ends in .so, and CMake installs the unversioned namelink, so the pattern matches on both), and the loader's use of RTLD_GLOBAL on macOS (pre-existing behavior, unchanged by this PR).

[wenzeslaus]

Covering the macOS in the CI would be helpful (unclear why removed). With the explicit opt-in, it makes sense to raise exceptions when it fails. Similarly, the combination of custom (non-global) env and load_libs=True does not work as expected since the custom env is not (and cannot be) passed to the C libraries, so raising an exception seems like a good way of communicating that. I will follow up with the suggested fixes.

Fable 5 review (details)

Here is an AI-boosted analysis (Claude Fable 5) of the updated version (head 3739140), verified against fresh local Autotools and CMake builds of the branch on Linux; each earlier finding was re-tested rather than assumed fixed.

Verified fixed:

1. Two copies of libgrass_gis (previously reproduced). With BASE_LIBRARY_NAMES ordering datetime and gis first, the original reproduction (preload one copy while LD_LIBRARY_PATH reaches another) now ends with a single libgrass_gis image, on both build systems. The residual documented in the new comment is real, but broader than the comment suggests. In the same scenario, about 20 non-base libraries (raster, vector, gproj, ogsf, dbmi*, drivers, ...) are still mapped twice, and an LD_DEBUG=bindings trace shows how the copies are used: all gis symbol bindings go to the session copy (the base ordering is fully effective), but for raster only the session copy of raster itself binds to itself -- every other GRASS library, including the session copies of calc, g3d, stats, vector, imagery, ogsf, binds raster symbols to the foreign copy, because the first dependent loaded pulls the foreign copy into the global scope and all later NEEDED lookups by SONAME hit it first; only the explicit full-path dlopen, which is what the grass.lib.raster wrapper uses, gets the session copy. The same pattern holds for vector. So the trigger is narrow -- a second installation with the same SONAME reachable through the process-startup search path; the SONAME encodes only the version (libgrass_gis.8.6.so with Autotools, libgrass_gis.so.8 with CMake), not the installation, so any two installations of the same GRASS version qualify, e.g., a locally compiled 8.6 used for the session while a packaged 8.6 is on an inherited LD_LIBRARY_PATH, or a moved copy used while the original still sits at the prefix recorded in the libraries' RUNPATH -- but once triggered, any workflow which passes a raster or vector handle between grass.lib wrappers crosses two copies -- the comment's "code which passes the state of that library between two different GRASS libraries" is in that scenario essentially all cross-library grass.lib use. Worth recording (comment or commit message): without the preload the same scenario yields a consistent all-foreign stack which works, so for a user with a conflicting installation on the search path, load_libs=True is a regression relative to not opting in; acceptable as an opt-in, and full dependency-order loading would remove it entirely if it ever matters.

2. Test now guards the fix. On both local builds, the new file passes in place and against a relocated prefix, and the importable test fails when the preload is disabled in a copy of the Python package, which it did not with the previous version of the test. The relocated CI step works without GISBASE because get_install_path() derives the installation from the location of the Python package (PYTHONPATH is rewritten to the moved path), verified locally with both layouts; the Autotools launcher itself does not relocate, but nothing in the test needs it.

3. Opt-in. grass lock-mapset and ensure_runtime_env() (used by gs.create_project() outside a session) now take the default load_libs=False path, so tool-only sessions no longer pull the GRASS C stack and its transitive dependencies (measured earlier at 130 non-GRASS shared objects) into the process.

4. Deduplication. The resolve() set halves the dlopen calls (previously measured 200 vs 100 for 54 libraries); with the base libraries ordered first, the retry loop converges in one pass in the common case.

5. Windows docstring and CI. The docstring now correctly describes the os.add_dll_directory registration done by the grass.lib loader. The OSGeo4W step is a real test of that chain: env.bat does not put the library directory on PATH (checked in mswindows/osgeo4w/env.bat.tmpl), so the DLLs are findable only through the PATH set by init() before the first grass.lib import. The xfail for the session-visibility test documents a separate pre-existing limitation and does not weaken the import test.

6. Docstring scope for env. The init() docstring now states that grass.lib works only in a session set up in the global environment, matching the verified behavior.

Remaining, non-blocking:

7. macOS relocated case still unverified, but the step that was removed failed only over a path bug. The step added in 8134d0e hardcoded PYTHONPATH="${HOME}/install-relocated/etc/python", while the Autotools install places the package at ${HOME}/install/grass86/etc/python (prefix plus grass<version>), so pytest failed with ModuleNotFoundError: No module named 'grass' before any test ran -- the test itself was never exercised on macOS. Taking python_path="$(grass --config python_path)" before the move and rewriting the prefix, as the Linux step and the other macOS pytest steps already do, fixes the step. Whether the test then passes on macOS (absolute install names baked from the configure prefix, resolved through dyld's already-loaded-image matching after the preload) is exactly what restoring the step would answer.

8. Failures are returned but never surfaced. preload_dynamic_libraries() returns the (path, error) list, but setup_runtime_env() discards it, so the docstring's "are reported" currently means "are returned to a caller which ignores them". The silent skipping was designed for an unconditional preload; now that the caller explicitly asks with load_libs=True, a failure answers a direct request. Raising on any failure would be too strict: a binary installation run on a machine missing an optional runtime dependency (e.g., libgrass_ogsf/nviz without libGL on a headless system) fails to load while everything the user needs works, and importing that one grass.lib module fails either way. An option which fits both: raise when a base library (datetime, gis) fails -- nothing in grass.lib works without them and it means a broken or mismatched installation -- and keep returning (or debug-logging) the rest.

9. load_libs=True together with env silently loads everything and delivers nothing. Measured with the branch: init(project, env=os.environ.copy(), load_libs=True) in a process without a session in its own environment maps 54 libgrass_* images plus 129 other shared objects (GDAL, PROJ, GL, X11, ...) into the process, irreversibly, and import grass.lib.gis still fails: the grass.lib loader reads GISBASE/LD_LIBRARY_PATH from os.environ, and the C libraries read the session (GISRC) from the process environment, so a session held in an env dict can never be visible to them -- which is presumably why custom-env support was dropped. Since env=os.environ.copy() is the recommended calling pattern elsewhere (AGENTS.md), this combination will happen. An option is to raise ValueError for load_libs=True with an env other than the global environment, before the irreversible load; init() already raises ValueError for inconsistent inputs, and preload_dynamic_libraries() stays available for anyone deliberately loading without a global session. At minimum, the :param load_libs: line should repeat the global-environment restriction stated in the docstring body. Also minor: repeated init(load_libs=True) in one process re-runs the ~54 dlopen calls each time; they are cheap once loaded, so only worth changing if init() is called in a loop.

Appendix: what was executed for this review. All runs are against head 3739140, with two fresh local builds in the same checkout: Autotools (./configure --prefix=<local>, make, make install; gisbase <prefix>/grass86, libraries libgrass_*.8.6.so with RUNPATH set to the prefix) and CMake (cmake --install to a second prefix; gisbase <prefix>/lib/grass86, libraries libgrass_*.so.8.6.0dev with SONAME libgrass_*.so.8). Every run below was done on both installations unless noted.

1. grass_script_setup_load_libs_test.py in place (valid RUNPATH, LD_LIBRARY_PATH unset): 4 passed.
2. The same file against a relocated installation as in the new CI step (whole prefix copied, original renamed away, PYTHONPATH pointing into the copy, no GISBASE exported): 4 passed, confirming that get_install_path() finds the moved installation from the package location alone.
3. Control for 2: same relocated setup with preload_dynamic_libraries() patched to return immediately in a copy of the Python package: test_grass_lib_importable_with_load_libs fails with ImportError: Could not load grass_gis, so the test now guards the fix (the previous version of the test passed with the preload removed).
4. Two-copy scenario (finding 1 of the previous review): preload_dynamic_libraries() run with LD_LIBRARY_PATH pointing at a second copy of the library directory; /proc/self/maps shows libgrass_gis and libgrass_datetime mapped once (previously twice) and about 20 other libraries mapped twice; zero load failures reported.
5. LD_DEBUG=bindings trace of the same scenario: all libgrass_gis symbol bindings go to the session copy; raster and vector symbols are bound to the foreign copy by every library except their own grass.lib wrappers (basis of point 1 above).
6. init(project, env=os.environ.copy(), load_libs=True) in a subprocess with no session in its own environment: 54 libgrass_* images plus 129 other shared objects mapped, import grass.lib.gis still fails (basis of point 9; Autotools build).
7. Static checks: mswindows/osgeo4w/env.bat.tmpl confirmed not to put the library directory on PATH (the OSGeo4W step tests the intended mechanism); the removed macOS step's failure traced to its hardcoded PYTHONPATH missing the grass<version> path component (basis of point 7); CI status of the head commit confirmed green on all platforms.

Not re-run: the end-to-end crash from passing a raster handle across wrappers in scenario 4-5 (inferred from the binding trace plus the reproduction done for the previous review); any macOS or Windows execution (no such machine here; Windows and macOS statements rest on CI results and code reading).

[wenzeslaus]

I added commits addressing my review: Raise when load of "gis" lib fails, when used with env=..., and test on macOS.

Suggested commit message

grass.script: Let init optionally load C libraries for grass.lib (#7733)

The dynamic linker reads the library search path variable only at
process startup, so setting it in init was not enough for grass.lib in
the current process: the GRASS libraries were found only when the
variable was set beforehand, e.g., inside a GRASS session, or through
the search path recorded in the libraries at build time. With
init(..., load_libs=True), the libraries are loaded into the process
by their full paths, which covers also installations moved away from
the prefix they were configured with.

The loading is opt-in because it brings the whole GRASS C stack and
its dependencies (GDAL, PROJ, ...) into the process, which sessions
using only tools do not need. The symlinked files are deduplicated and
libgrass_datetime and libgrass_gis are loaded first, so that all other
libraries share their single copy. A failure to load the core library
raises RuntimeError, an optional library failing to load (e.g.,
without OpenGL) only makes importing the corresponding grass.lib
module fail, and combining load_libs with a custom env raises
ValueError because the C libraries read the session from the process
environment.

The remaining libraries are loaded without resolving their dependency
tree, so when another installation with the same library SONAMEs is
reachable through the process-startup search path, a library other
than the two base ones can still end up loaded twice.

On Windows, no loading is needed: the grass.lib loader registers the
PATH directories with os.add_dll_directory when it is first imported.

The new tests run in the regular test suites and also standalone
against an installation moved away from the prefix it was configured
with, where the loading done by init is the only mechanism left. CI
covers the moved installation on Linux and macOS, and the Windows
OSGeo4W job runs the tests without the library directory on PATH.

[wenzeslaus]

I ended up taking a new approach. First step is #8093. The current behavior is actually better than what the starting point here assumes. Things can be also more automated than the explicit-request (default or not) here suggests.