build: refactor grass.py in preparation for FHS #5933

nilason · 2025-06-21T15:15:56Z

(This is extracted from #5630 to simplify review.)

The transitional period from the traditional GRASS installation structure to a Filesystem Hierarchy Standard (FHS) complying structure (enabled by CMake build) requires an alternative way to find resources. Currently all resources are located in relation to GISBASE, that is no longer the case with a FHS installation.

In addition, and instead of GISBASE, the following environment variables will be needed (not included in this update):

GRASS_SHAREDIR
GRASS_LOCALEDIR
GRASS_PYDIR
GRASS_GUIWXDIR
GRASS_GUISCRIPTDIR
GRASS_GUIRESDIR
GRASS_FONTSDIR
GRASS_ETCDIR

Setting up all these variables would bloat the grass.py file, therefore this update suggests to move this to the grass.app Python module.

Summary of changes:

grass.py: to simplify Conda adoption, the variable GRASS_PREFIX is added, which is essentially equal to installation prefix, but easily changeable if needed.
grass.py need to find the Python module to be able to set necessary variables, a new variable GRASS_PYDIR is configured into the file to do just this.
python/grass/app/resource_paths.py is a configurable file (like grass.py)
update copy_python_files_in_subdir.cmake to be able to exclude files

python/grass/app/Makefile

python/grass/app/resource_paths.py

wenzeslaus · 2025-06-24T19:26:37Z

python/grass/app/resource_paths.py

+    if "GISBASE" in os.environ and len(os.getenv("GISBASE")) > 0:
+        GISBASE = os.path.normpath(os.environ["GISBASE"])


This is the current behavior. I'm little unsure if the setup code should respect or overwrite the current GISBASE.

lib/init/grass.py

nilason · 2025-06-25T10:48:09Z

How about to move the content of my added "resource_paths.py" file to the existing "runtime.py" and move all other configuration variables like GRASS_VERSION = "@GRASS_VERSION_NUMBER@". What must remain is GRASS_PYDIR to be able to find the Python module. That way all configuration info will be available through use of Python module only.
Perhaps rework runtime.py to create a (singleton) class named Runtime or something similar?

from grass.app import Runtime
startup_location = os.path.join(Runtime().share_dir, "demolocation")
color_dir = Runtime().color_dir

wenzeslaus · 2025-06-25T13:53:06Z

How about to move the content of my added "resource_paths.py" file to the existing "runtime.py" and move all other configuration variables like GRASS_VERSION = "@GRASS_VERSION_NUMBER@".

My idea for runtime.py was to provide interface and functionality for all things runtime, so yes, the runtime.py should hide what is in the background and should make it easy to use. On the other hand, maybe there is a value in having all the generated stuff at one place, so interfacing through runtime.py, but having the @NAME@ pieces in resource_paths.py.

# resource_paths.py
GRASS_VERSION_NUMBER = "@GRASS_VERSION_NUMBER@"
GRASS_COLOR_DIR = "@GRASS_COLOR_DIR@"

# runtime.py
# Actually deal with paths and partial paths, e.g.,
def get_color_dir():
    return os.path.normpath(os.path.join(get_prefix(), "@GRASS_COLOR_DIR@")

What must remain is GRASS_PYDIR to be able to find the Python module.

I'm not sure what you refer to. Remain where? In grass.py? Yes. My question in the review was about having the environmental variable.

That way all configuration info will be available through use of Python module only.

+1

Perhaps rework runtime.py to create a (singleton) class named Runtime or something similar?
from grass.app import Runtime
startup_location = os.path.join(Runtime().share_dir, "demolocation")
color_dir = Runtime().color_dir

Singleton may not be necessary, at least not on the level where a user needs to see it. I recently used module-level __getattr__ which can make it seem as simple variables in a module.

# runtime.py
def __getattr__(name):
    if name == "GRASS_VERSION":
        return "@GRASS_VERSION_NUMBER@"
    if name == "GRASS_COLOR_DIR":
        # Runs only when actually asked for
        return os.path.normpath(os.path.join(GRASS_PREFIX, "@GRASS_COLOR_DIR@")

Or, if we want snake case names on module level and no logic is need for some:

# runtime.py
version = "@GRASS_VERSION_NUMBER@"
def __getattr__(name):
    if name == "color_dir":
        # Runs only when actually asked for
        return os.path.normpath(os.path.join(prefix, "@GRASS_COLOR_DIR@")

Either way:

from grass.app.runtime import version
import grass.app.runtime as runtime
runtime.color_dir

However, a function returning an object which may or may not be singleton is possible too.

# runtime.py
class RuntimePaths:
    @property
    def color_dir(self):
        if self._color_dir is None:
            self._color_dir = os.path.normpath(os.path.join(self.prefix, "@GRASS_COLOR_DIR@")
        return self._color_dir

_runtime_paths = RuntimePaths()
def runtime_paths():
    return _runtime_paths

User code:

from grass.app import runtime_paths
startup_location = os.path.join(runtime_paths().share_dir, "demolocation")
color_dir = runtime_paths().color_dir

Or even through an attribute rather than a function call:

_runtime_paths = None
def __getattr__(name):
    if name == "runtime_paths":
        if _runtime_paths is None:
            _runtime_paths = RuntimePaths()
        return _runtime_paths

from grass.app import runtime_paths
startup_location = os.path.join(runtime_paths.share_dir, "demolocation")
color_dir = runtime_paths.color_dir

nilason · 2025-06-25T15:46:25Z

Thanks for the input! I'll see what I can come up with.

nilason · 2025-06-26T13:00:35Z

Currently GISBASE is initialised in grass.py in this way:

if "GISBASE" in os.environ and len(os.getenv("GISBASE")) > 0:
    GISBASE = os.path.normpath(os.environ["GISBASE"])
else:
    GISBASE = os.path.normpath("@GISBASE_INSTALL_PATH@")
    os.environ["GISBASE"] = GISBASE

The new path env. variables should also be set up in similar way, if we are to keep current behaviour. E.g., if the env. variable is set, then return the value of that, or set it with a configured value. The functions in runtime.py and setup.py also reflects the factor of environment. To achieve that using a simple class may perhaps be an approach:

# runtime.py
class RuntimePaths:
    def __init__(self, env=os.environ):
        self.env = env

    def __getattr__(self, name):
        if name == "grass_version":
            return runtime_paths.GRASS_VERSION_NUMBER


# user code
from runtime import RuntimePaths

gisbase = RuntimePaths(env=my_env).gisbase
# or:
runtime_paths = RuntimePaths()  # using default "os.environ"
version = runtime_paths.grass_version
share_dir = runtime_paths.share_dir

nilason · 2025-06-27T10:05:28Z

Just pushed update with the alternative approach I mentioned before. It is not completely ready yet, especially the cmake build needs some more work. But you can see if this could be an acceptable and/or promising solution.

wenzeslaus

Good idea, when the environment is involved, even this should accept env parameter.

wenzeslaus · 2025-06-30T13:57:34Z

python/grass/app/runtime.py

+version = grass.app.resource_paths.GRASS_VERSION
+version_major = grass.app.resource_paths.GRASS_VERSION_MAJOR
+version_minor = grass.app.resource_paths.GRASS_VERSION_MINOR
+ld_library_path_var = grass.app.resource_paths.LD_LIBRARY_PATH_VAR
+grass_exe_name = grass.app.resource_paths.GRASS_EXE_NAME
+grass_version_git = grass.app.resource_paths.GRASS_VERSION_GIT


These are defined as "global constants" because they are defined during the compilation as opposed to the other ones which may be influenced by environment variables, or?

Yes, at least this is the current behaviour. These are "hardcoded", get-only variables. The "RuntimePaths" also sets environment variables (all derived from the use of GISBASE).

An alternative approach would be to combine the two, but rename the class to e.g., "RuntimeConfigs".

runtime_config = RuntimeConfig() GISBASE = runtime_config.gisbase GRASS_VERSION_GIT = runtime_config.grass_version_git

As in "runtime and config"? That's not obvious to me from the name. Single place sounds good though, regardless of the name.

Integrated all variables as properties of RuntimePaths.

RuntimeSettings could be an alternative name...

runtime_settings = RuntimeSettings() GISBASE = runtime_settings.gisbase GRASS_VERSION_GIT = runtime_settings.grass_version_git

python/grass/app/runtime.py

lib/init/CMakeLists.txt

(This is extracted from OSGeo#5630 to simplify review.) The transitional period from the traditional GRASS installation structure to a Filesystem Hierarchy Standard (FHS) complying structure (enabled by CMake build) requires an alternative way to find resources. Currently all resources are located in relation to GISBASE, that is no longer the case with a FHS installation. In addition, and instead of GISBASE, the following environment variables will be needed (not included in this update): - GRASS_SHAREDIR - GRASS_LOCALEDIR - GRASS_PYDIR - GRASS_GUIWXDIR - GRASS_GUISCRIPTDIR - GRASS_GUIRESDIR - GRASS_FONTSDIR - GRASS_ETCDIR Setting up all these variables would bloat the grass.py file, therefore this update suggests to move this to the grass.app Python module. Summary of changes: - grass.py: to simplify Conda adoption, the variable GRASS_PREFIX is added, which is essentially equal to installation prefix, but easily changeable if needed. - grass.py need to find the Python module to be able to set necessary variables, a new variable GRASS_PYDIR is configured into the file to do just this. - python/grass/app/resource_paths.py is a configurable file (like grass.py) - update copy_python_files_in_subdir.cmake to be able to exclude files

nilason · 2025-07-03T20:00:03Z

Any more suggestions, objections. #5630 builds upon this and that one is starting to shape up, so it would be good to move on.

echoix · 2025-07-04T16:33:51Z

Are some changes in the mswindows/ folder needed, regarding the changes of @var@ substitutions? There are some instances in that folder.

wenzeslaus

I like the Python part and the CMake changes look reasonable. We can merge this as is. I have just some minor comments which you can ignore.

wenzeslaus · 2025-07-06T04:06:26Z

lib/init/grass.py

+
+    # The "@...@" variables are being substituted during build process
+
+    if "GRASS_PYDIR" in os.environ and len(os.getenv("GRASS_PYDIR")) > 0:


I'm not sure when len(os.getenv("GRASS_PYDIR")) > 0 is different from os.getenv("GRASS_PYDIR"), but if it is the same, Ruff or something will tell us.

wenzeslaus · 2025-07-06T04:10:20Z

python/grass/app/runtime.py

@@ -17,12 +17,73 @@
 import subprocess
 import sys

+from . import resource_paths as res_paths


I think this should work without an alias.

Suggested change

from . import resource_paths as res_paths

from . import resource_paths

nilason added this to the 8.5.0 milestone Jun 21, 2025

github-actions bot added Python Related code is in Python libraries CMake labels Jun 21, 2025

echoix reviewed Jun 21, 2025

View reviewed changes

python/grass/app/Makefile Outdated Show resolved Hide resolved

python/grass/app/resource_paths.py Show resolved Hide resolved

wenzeslaus reviewed Jun 24, 2025

View reviewed changes

nilason marked this pull request as draft June 25, 2025 15:49

nilason force-pushed the move_gisbase branch from 10be97c to 8e67d0a Compare June 30, 2025 11:48

wenzeslaus reviewed Jun 30, 2025

View reviewed changes

nilason commented Jun 30, 2025

View reviewed changes

lib/init/CMakeLists.txt Outdated Show resolved Hide resolved

nilason marked this pull request as ready for review June 30, 2025 18:37

nilason force-pushed the move_gisbase branch from 9036506 to 06c715f Compare July 2, 2025 08:44

wenzeslaus approved these changes Jul 6, 2025

View reviewed changes

		if "GISBASE" in os.environ and len(os.getenv("GISBASE")) > 0:
		GISBASE = os.path.normpath(os.environ["GISBASE"])


		# The "@...@" variables are being substituted during build process

		if "GRASS_PYDIR" in os.environ and len(os.getenv("GRASS_PYDIR")) > 0:

	from . import resource_paths as res_paths
	from . import resource_paths

Uh oh!

build: refactor grass.py in preparation for FHS #5933

Are you sure you want to change the base?

build: refactor grass.py in preparation for FHS #5933

Uh oh!

Conversation

nilason commented Jun 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nilason commented Jun 25, 2025

Uh oh!

wenzeslaus commented Jun 25, 2025

Uh oh!

nilason commented Jun 25, 2025

Uh oh!

nilason commented Jun 26, 2025

Uh oh!

nilason commented Jun 27, 2025

Uh oh!

wenzeslaus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nilason Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nilason commented Jul 3, 2025

Uh oh!

echoix commented Jul 4, 2025

Uh oh!

wenzeslaus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nilason Jul 1, 2025 •

edited

Loading