grass.app: Separate env modification from path retrieval (#6469)

The RuntimePaths object would also modify the environment while a variable is being retrieved. This is unexpected side effect of an attribute access. It was also the way how the GISBASE environment variable was set in the main grass executable which is a very opaque way of setting it. The environment modification is now removed, and the caller code must set GISBASE explicitly.

The main grass executable code now first gets the compile-time determined runtime path from RuntimePaths. Then it passes that to get_install_path which will evaluate if it is an existing (valid) path, and will find a fallback if it is not. Previously, the main grass executable fully relied on the compile-time determined path. Now, it can find a path itself the same way the Python API (grass.script.setup.init) does that, but the executable uses the compile-time determined path as a starting point.

If the search for a install path fallback is successful in the main executable, it will hide issues with the path in the installation. To compensate for that, the get_install_path function is now tested for consistency with the compile-time determined runtime install path coming from a RuntimePaths object in pytest-based tests. This will allow a GRASS process to run even with a broken installation (some broken installations to be exect), but running the test will show that there is an issue. To further facilitate debugging, the get_install_path returns the originally provided path if none of the tested ways was successful.

The get_install_path function now guarantees that the paths it returns exists except when none exists, then it returns whatever was the path provided as a parameter. This makes it possible to pass an invalid compile-time determined path, but fall back to path determined in some other way, while behaving consistently with other cases (some were already using existence as a test). The function now also always returns a string assuming the primary usage is to set an environment variable.

The path for the main executable is now set in the same way as for the init function. Both are in the library, although still at two different places and with a slight difference of what is used as the starting point (compile-time determined runtime path and user-provided path or, usually, None).

Author: wenzeslaus

lib/init/grass.py

@@ -2169,6 +2169,7 @@ def main() -> None:
     find_grass_python_package()
 
     from grass.app.runtime import RuntimePaths
+    from grass.script.setup import get_install_path
 
     global \
         CMD_NAME, \
@@ -2183,7 +2184,7 @@ def main() -> None:
     GRASS_VERSION = runtime_paths.version
     GRASS_VERSION_MAJOR = runtime_paths.version_major
     GRASS_VERSION_GIT = runtime_paths.grass_version_git
-    GISBASE = runtime_paths.gisbase
+    GISBASE = get_install_path(runtime_paths.gisbase)
     CONFIG_PROJSHARE = runtime_paths.config_projshare
 
     grass_config_dir = create_grass_config_dir()

python/grass/app/runtime.py

@@ -78,12 +78,9 @@ def __get_dir(self, env_var):
         If the environmental variable not yet set, it is retrived and
         set from resource_paths."""
         if env_var in self.env and len(self.env[env_var]) > 0:
-            res = os.path.normpath(self.env[env_var])
-        else:
-            path = getattr(res_paths, env_var)
-            res = os.path.normpath(os.path.join(res_paths.GRASS_PREFIX, path))
-            self.env[env_var] = res
-        return res
+            return os.path.normpath(self.env[env_var])
+        path = getattr(res_paths, env_var)
+        return os.path.normpath(os.path.join(res_paths.GRASS_PREFIX, path))
 
 
 def get_grass_config_dir(*, env):
@@ -187,6 +184,9 @@ def set_executable_paths(install_path, grass_config_dir, env):
 
 def set_paths(install_path, grass_config_dir):
     """Set variables with executable paths, library paths, and other paths"""
+    # Set main prefix.
+    # See also grass.script.setup.setup_runtime_env.
+    os.environ["GISBASE"] = install_path
     set_executable_paths(
         install_path=install_path, grass_config_dir=grass_config_dir, env=os.environ
     )

python/grass/app/tests/grass_app_runtime.py

@@ -0,0 +1,83 @@
+"""Tests of runtime setup, mostly focused on paths
+
+The install path (aka GISBASE) tests are assuming, or focusing on a non-FHS
+installation. Their potential to fail is with broken builds and installs.
+"""
+
+from pathlib import Path
+
+import pytest
+
+from grass.app.runtime import RuntimePaths
+from grass.script.setup import get_install_path
+
+
+def return_as_is(x):
+    """Return the parameter exactly as received"""
+    return x
+
+
+def test_install_path_consistent():
+    """Differently sourced install paths should be the same.
+
+    Dynamically determined install path and compile-time install path should be
+    the same in a healthy installation.
+
+    This is a non-FHS oriented test.
+    """
+    assert get_install_path() == RuntimePaths().gisbase
+
+
+@pytest.mark.parametrize("path_type", [str, Path, return_as_is])
+def test_install_path_used_as_result(path_type):
+    """Passing a valid compile-time install path should return the same path.
+
+    If the path is not recognized as install path or there is a problem with the
+    dynamic determination of the install path, the test will fail.
+
+    This is a non-FHS oriented test.
+    """
+    path = RuntimePaths().gisbase
+    assert get_install_path(path_type(path)) == path
+
+
+def test_consistent_install_path_returned():
+    """Two subsequent calls should return the same result.
+
+    The environment should not be modified by the call, so the result should
+    always be the same.
+
+    This is a non-FHS oriented test.
+    """
+    assert get_install_path() == get_install_path()
+
+
+@pytest.mark.parametrize("path_type", [str, Path, return_as_is])
+def test_feeding_output_as_input_again(path_type):
+    """Passing result of the get_install_path back to it should give the same result.
+
+    When a dynamically path gets returned by the function, the same path should be
+    the one returned again when called with that path (sort of like calling the same
+    function twice because we can't tell here if it is a newly computed path or the
+    provided path if they are the same).
+
+    We use this to test different path types.
+
+    This is a non-FHS oriented test.
+    """
+    path = get_install_path()
+    assert path == get_install_path(path_type(path))
+
+
+@pytest.mark.parametrize("path_type", [str, Path, return_as_is])
+def test_passing_non_existent_path(path_type):
+    """Passing result of the get_install_path back to it should give the same result.
+
+    When a dynamically path gets returned by the function, the same path should be
+    the one returned again when called with that path (sort of like calling the same
+    function twice because we can't tell here if it is a newly computed path or the
+    provided path if they are the same).
+
+    This is a non-FHS oriented test.
+    """
+    assert get_install_path(path_type("/does/not/exist")) == get_install_path()

python/grass/script/setup.py

@@ -80,6 +80,8 @@
 # is known, this would allow moving things from there, here
 # then this could even do locking
 
+from __future__ import annotations
+
 from pathlib import Path
 import datetime
 import os
@@ -109,7 +111,7 @@ def set_gui_path():
         sys.path.insert(0, gui_path)
 
 
-def get_install_path(path=None):
+def get_install_path(path: str | Path | None = None) -> str:
     """Get path to GRASS installation usable for setup of environmental variables.
 
     The function tries to determine path tp GRASS installation so that the
@@ -145,27 +147,31 @@ def ask_executable(arg):
             [arg, "--config", "path"], text=True, check=True, capture_output=True
         ).stdout.strip()
 
+    # Directory was provided as a parameter.
+    if path and os.path.isdir(path):
+        return os.fspath(path)
+
     # Executable was provided as parameter.
     if path and shutil.which(path):
         # The path was provided by the user and it is an executable
         # (on path or provided with full path), so raise exception on failure.
-        return ask_executable(path)
-
-    # Presumably directory was provided.
-    if path:
-        return path
+        path_from_executable = ask_executable(path)
+        if os.path.isdir(path_from_executable):
+            return path_from_executable
 
-    # GISBASE is already set.
+    # GISBASE is set from the outside or already set.
     env_gisbase = os.environ.get("GISBASE")
-    if env_gisbase:
+    if env_gisbase and os.path.isdir(env_gisbase):
         return env_gisbase
 
     # Executable provided in environment (name is from grass-session).
     # The variable is supported (here), documented, but not widely promoted
     # at this point (to be re-evaluated).
     grass_bin = os.environ.get("GRASSBIN")
     if grass_bin and shutil.which(grass_bin):
-        return ask_executable(grass_bin)
+        path_from_executable = ask_executable(grass_bin)
+        if os.path.isdir(path_from_executable):
+            return path_from_executable
 
     # Derive the path from path to this file (Python module).
     # This is the standard way when there is no user-provided settings.
@@ -176,17 +182,21 @@ def ask_executable(arg):
     bin_path = install_path / "bin"
     lib_path = install_path / "lib"
     if bin_path.is_dir() and lib_path.is_dir():
-        return install_path
+        return os.fspath(install_path)
 
     # As a last resort, try running grass command if it exists.
     # This is less likely give the right result than the relative path on systems
     # with multiple installations (where an explicit setup is likely required).
     # However, it allows for non-standard installations with standard command.
     grass_bin = "grass"
     if grass_bin and shutil.which(grass_bin):
-        return ask_executable(grass_bin)
+        path_from_executable = ask_executable(grass_bin)
+        if os.path.isdir(path_from_executable):
+            return path_from_executable
 
-    return None
+    # We fallback to whatever was provided. This may help trace the issue
+    # in broken installations.
+    return os.fspath(path) if path else path
 
 
 def setup_runtime_env(gisbase=None, *, env=None):
@@ -202,11 +212,7 @@ def setup_runtime_env(gisbase=None, *, env=None):
     If _gisbase_ is not provided, a heuristic is used to find the path to GRASS
     installation (see the :func:`get_install_path` function for details).
     """
-    if not gisbase:
-        gisbase = get_install_path()
-
-    # Accept Path objects.
-    gisbase = os.fspath(gisbase)
+    gisbase = get_install_path(gisbase)
 
     # If environment is not provided, use the global one.
     if not env:
@@ -221,15 +227,17 @@ def setup_runtime_env(gisbase=None, *, env=None):
         RuntimePaths,
     )
 
-    # Set GISBASE
+    runtime_paths = RuntimePaths(env=env)
+    # Set main prefix.
+    # See also grass.app.runtime.set_paths.
     env["GISBASE"] = gisbase
     set_executable_paths(
         install_path=gisbase,
         grass_config_dir=get_grass_config_dir(env=env),
         env=env,
     )
     set_dynamic_library_path(
-        variable_name=RuntimePaths().ld_library_path_var, install_path=gisbase, env=env
+        variable_name=runtime_paths.ld_library_path_var, install_path=gisbase, env=env
     )
     set_python_path_variable(install_path=gisbase, env=env)
     set_path_to_python_executable(env=env)