Skip to content

build: prepare for FHS, where GISBASE is not the absolute truth#5630

Open
nilason wants to merge 8 commits intoOSGeo:mainfrom
nilason:fix_cmake_share_dir
Open

build: prepare for FHS, where GISBASE is not the absolute truth#5630
nilason wants to merge 8 commits intoOSGeo:mainfrom
nilason:fix_cmake_share_dir

Conversation

@nilason
Copy link
Contributor

@nilason nilason commented May 9, 2025

GISBASE is used as a means to retrieve paths to various resources in source code. With FHS (#3661) this is no longer possible. This adds a number of new environment variables to address this.

Currently this is addressed with following workaround:

grass/CMakeLists.txt

Lines 176 to 194 in 5e998d3

if(WITH_FHS)
file(CREATE_LINK ${OUTDIR}/${GRASS_INSTALL_DEMODIR}
${RUNTIME_GISBASE}/demolocation SYMBOLIC)
file(CREATE_LINK ${OUTDIR}/${GRASS_INSTALL_FONTSDIR} ${RUNTIME_GISBASE}/fonts
SYMBOLIC)
file(CREATE_LINK ${OUTDIR}/${GRASS_INSTALL_ETCDIR}/colors
${RUNTIME_GISBASE}/etc/colors SYMBOLIC)
file(CREATE_LINK ${OUTDIR}/${GRASS_INSTALL_ETCDIR}/colors.desc
${RUNTIME_GISBASE}/etc/colors.desc SYMBOLIC)
file(CREATE_LINK ${OUTDIR}/${GRASS_INSTALL_ETCDIR}/element_list
${RUNTIME_GISBASE}/etc/element_list SYMBOLIC)
file(CREATE_LINK ${OUTDIR}/${GRASS_INSTALL_ETCDIR}/renamed_options
${RUNTIME_GISBASE}/etc/renamed_options SYMBOLIC)
file(CREATE_LINK ${OUTDIR}/${GRASS_INSTALL_ETCDIR}/VERSIONNUMBER
${RUNTIME_GISBASE}/etc/VERSIONNUMBER SYMBOLIC)
file(MAKE_DIRECTORY "${GISBASE}/gui/wxpython")
file(CREATE_LINK ${OUTDIR}/${GRASS_INSTALL_GUIDIR}/wxpython/xml
${RUNTIME_GISBASE}/gui/wxpython/xml SYMBOLIC)
endif()

This is work in progress.

@nilason nilason added this to the 8.5.0 milestone May 9, 2025
@nilason nilason self-assigned this May 9, 2025
@github-actions github-actions bot added GUI wxGUI related vector Related to vector data processing raster Related to raster data processing Python Related code is in Python C Related code is in C translation Message translation related database Related to database management libraries module docs general display CMake labels May 9, 2025
@nilason nilason force-pushed the fix_cmake_share_dir branch from 5acbd05 to 518072c Compare May 19, 2025 14:44
@nilason nilason force-pushed the fix_cmake_share_dir branch 2 times, most recently from 91955b8 to d852aef Compare May 21, 2025 16:11
nilason added a commit to nilason/grass that referenced this pull request Jun 21, 2025
(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 added a commit to nilason/grass that referenced this pull request Jun 30, 2025
(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 added a commit to nilason/grass that referenced this pull request Jul 2, 2025
(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 added a commit to nilason/grass that referenced this pull request Jul 2, 2025
(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 nilason force-pushed the fix_cmake_share_dir branch 6 times, most recently from 1ebd308 to dd7c201 Compare July 8, 2025 08:07
@nilason nilason modified the milestones: 8.5.0, Future Aug 9, 2025
@echoix echoix added the conflicts/needs rebase Rebase to or merge with the latest base branch is needed label Oct 7, 2025
…e by a single dict object at the class level. Use dynamic attributes to avoid repeating the access code. Separate env modification from path retrieval as in OSGeo#6469.
@wenzeslaus
Copy link
Member

I updated the code to align with #6469 in terms of setting the variable explicitly, while keeping the idea that this may be used to set the variables.

The conflict which was already there in setup.py will be better to fix only with #6469 merged to also resolve some of the GISBASE setting and fallback inconsistencies between this and #6469. (for some of it, I can also update #6469 to reduce the changes here).

I have some small tests lined up which will test at least some the variables in their specific context, but most variables here are either already tested (because they are crucial) or they are used in context which is hard-to-test automatically (typically GUI).

@nilason I didn't merge main here to update, because you were doing rebase (not merge) on this branch, but it seems merge would be easier now with the multiple commits, no?

wenzeslaus added a commit to wenzeslaus/grass that referenced this pull request Oct 9, 2025
To test that color tables are built and that r.colors can access them, this adds a simple smoke test which fails if a color table file is not found.

This supports testing of changes in paths in OSGeo#5630.
@github-actions github-actions bot added the tests Related to Test Suite label Oct 9, 2025
wenzeslaus added a commit that referenced this pull request Oct 9, 2025
To test that color tables are built and that r.colors can access them, this adds a simple smoke test which fails if a color table file is not found.

This supports testing of changes in paths in #5630.
wenzeslaus added a commit to wenzeslaus/grass that referenced this pull request Oct 10, 2025
In anticiaption of OSGeo#5630 which adds multiple variables besides GISBASE, this modifies RuntimePaths to handle variables from a list (dict to be exact). Now, only one variable is handled that way and that is GISBASE. The GRASS_PREFIX variable is handled in a different way because the compile-time values of the other variables are without a prefix and the RuntimePaths is prefixing them when the variable is needed.

The RuntimePaths object can now set environment variables when explicitly asked to do so. This hides the info about all the variables handled by the object inside the class code, so the caller does not need to know about a change from one GISBASE to multiple GISBASE-like variables. Both the main grass executable and the Python init function now use RuntimePaths to set up the GISBASE (and other GISBASE-like variables in the future).

While both usages of RuntimePaths are similar, they are not completely the same. Python init takes optional, caller-provided gisbase which is used as a prefix (assuming prefix and gisbase are, in practice, the same), while the main executable always uses the RuntimePaths default. Both test if the gisbase from RuntimePaths exists, and if not, they go to get_install_path to get a fallback one.

To allow for the fallback to take effect, RuntimePaths can take prefix in the constructor and will use it instead of the compile-time determined GRASS_PREFIX.
@wenzeslaus
Copy link
Member

#6482 will bring the main closer to this one, so it will be easier to merge, i.e., no need to attempt merging with main now. (#6482 needs changes to tests I'm working on, but don't have ready yet.)

wenzeslaus added a commit that referenced this pull request Oct 18, 2025
In anticipation of #5630 which adds multiple variables besides GISBASE, this modifies RuntimePaths to handle variables from a list (dict to be exact). Now, only one variable is handled that way and that is GISBASE. The GRASS_PREFIX variable is handled in a different way because the compile-time values of the other variables are without a prefix and the RuntimePaths is prefixing them when the variable is needed.

The RuntimePaths object can now set environment variables when explicitly asked to do so. This hides the info about all the variables handled by the object inside the class code, so the caller does not need to know about a change from one GISBASE to multiple GISBASE-like variables. Both the main grass executable and the Python init function now use RuntimePaths to set up the GISBASE (and other GISBASE-like variables in the future).

While both usages of RuntimePaths are similar, they are not completely the same. Python init takes optional, caller-provided gisbase which is used as a prefix (assuming prefix and gisbase are, in practice, the same), while the main executable always uses the RuntimePaths default. Both test if the gisbase from RuntimePaths exists, and if not, they go to get_install_path to get a fallback one.

To allow for the fallback to take effect, RuntimePaths can take prefix in the constructor and will use it instead of the compile-time determined GRASS_PREFIX. Additionally, allow mixing of gisbase and prefix-only on the input which is needed for real installations, at least those without FHS. If the path is full GISBASE, the unique GISBASE part is removed, remove it to get the prefix only. The corresponding test passes with non-trivial GISBASE. Also, the consistency tests fail when the compile-time prefix path is non-sense.

For simple, and more readable code, avoid import alias for resource_paths and use the module name directly.

The actual init code needs to unfortunately deal with GISBASE directly. RuntimePaths itself always assumes that the paths are right, so any searches and fallbacks need to happen in the caller code. When we provide a corrected gisbase as a prefix, a broken build will supply wrong gisbase to the prefix breaking gisbase again, so we need to manually fix it with a subsequent call. This is not nice, and we need to have it at two different places now, but it is not a overly complicated code.

Generally, the tests are trying to check that resource paths are substituted during build and fail otherwise, i.e., the opposite of what the code is trying to do which is work even if the installation is broken in some way.
string(APPEND _c_flags " -I${CMAKE_SOURCE_DIR}/msvc")
elseif(APPLE AND CMAKE_OSX_SYSROOT)
string(APPEND _c_flags " --sysroot ${CMAKE_OSX_SYSROOT}")
message("_c_flags")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not seem to be related here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an early debugging left-over, which shouldn't have remained at all, and doesn't deserve a PR in its own right.

@wenzeslaus
Copy link
Member

I merged the main branch into this branch. Some of the new tests fail here on Windows. It seems to be related to creation of a project with g.proj, but I don't know beyond that.

@wenzeslaus
Copy link
Member

On Windows, g.proj is failing with a return code that may indicate a stack overflow:

grass.exceptions.ScriptError: g.proj failed with return code 3221225725, but no error message was produced

@echoix
Copy link
Member

echoix commented Oct 18, 2025

On Windows, g.proj is failing with a return code that may indicate a stack overflow:

grass.exceptions.ScriptError: g.proj failed with return code 3221225725, but no error message was produced

Is this a new failure? I think I remember seeing this kind of error quite often in the logs.

@wenzeslaus
Copy link
Member

This specific message is new, see the latest commit, but I don't see why it would surface here and not elsewhere.

@echoix
Copy link
Member

echoix commented Oct 18, 2025

This specific message is new, see the latest commit, but I don't see why it would surface here and not elsewhere.

I had in mind only the big number as return code

@nilason
Copy link
Contributor Author

nilason commented Oct 20, 2025

Thanks for updating to main! Still occupied the next couple of days, but will check out the failures as soon as possible.

nilason added a commit that referenced this pull request Jan 26, 2026
Recent additions to the CMake legacy build, have rendered the FHS build
broken. These additions include foremost GUI menu generation and
documentation indexing. Proper fix for this would require changes in code,
addressed with #5630.

For the time being, with this PR, those failing parts will be skipped if building
with FHS compliance.

This does not affect legacy build with CMake.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C CMake conflicts/needs rebase Rebase to or merge with the latest base branch is needed database Related to database management display docs general GUI wxGUI related libraries module Python Related code is in Python raster Related to raster data processing tests Related to Test Suite translation Message translation related vector Related to vector data processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants