Implement FHS layout support in CMake and separate grassgui package#10
Open
wenzeslaus wants to merge 17 commits into
Open
Implement FHS layout support in CMake and separate grassgui package#10wenzeslaus wants to merge 17 commits into
wenzeslaus wants to merge 17 commits into
Conversation
Local working document for the fhs-implementation branch; not intended for a PR. Written with Claude Code.
Squash of OSGeo#5630 by Nicklas Larsson and contributors, taken as the baseline for local FHS work. Introduces per-resource environment variables, C accessors in lib/gis/resource_dirs.c, and RuntimePaths and resource_paths.py coverage of the new variables in both build systems. Local progress-tracking commit, not for a PR.
The custom command copying headers to the output tree had no DEPENDS, so an already staged header was never refreshed when its source changed. Incremental builds then compiled against stale headers; with the new resource_dirs.c declarations missing, G_locale_dir() was implicitly declared as returning int and the truncated pointer crashed G_init_locale(). Found and fixed with Claude Code.
G_locale_dir() calls G_fatal_error() when GRASS_LOCALEDIR is not set, and G_fatal_error() translates its message, which re-enters G_init_locale() and recurses until the stack overflows. Read the variable directly and skip bindtextdomain() when it is not set, matching the pre-existing behavior for GISBASE. Likely the cause of the g.proj stack overflow (0xC00000FD) seen on Windows CI in the FHS preparation PR. Found and fixed with Claude Code.
Make GRASS_INSTALL_PYDIR prefix-relative by default (overridable, must stay relative) so the staged runtime tree and the installed tree stay inside their roots; previously the absolute Python3_SITEARCH leaked into build/output and cmake --install would write outside the prefix. Point the demolocation GISDBASE at GRASS_INSTALL_MISCDIR, which is the same path as before in the legacy layout and the actual demolocation parent under FHS. With resource paths now coming from environment variables (PR OSGeo#5630), drop the configure-time symlink workaround and re-enable the wxGUI menudata, man index, and translation status targets under FHS. Rewrite build-tree font paths in the installed fontcap to the install prefix (affects both layouts). Verified: fresh WITH_FHS=ON and legacy builds complete; staged FHS tree has no absolute-path leak; session from the FHS build tree runs C tools. Python tools in a session still fail to import grass under FHS (runtime PYTHONPATH is legacy-shaped) - addressed in Phase 2. Local progress-tracking commit, not for a PR. Written with Claude Code.
Add GRASS_PYDIR and GRASS_MANDIR to the build-time resource paths and RuntimePaths so sessions export them like the other resource variables. Use them for the session PYTHONPATH and MANPATH (with the legacy layout as fallback) instead of hardcoding etc/python and docs/man under the install path. Report the resolved Python package path from grass --config python_path. Convert g.manual to the resource variables for man, HTML, and MkDocs paths; it was missed in the FHS preparation PR. Verified against both the FHS install and the legacy build tree: grass --config python_path, Python tools in a session (t.list), grass man g.region, and pytest for grass.app and grass.script (228 passed) and raster/r.slope.aspect. Local progress-tracking commit, not for a PR. Written with Claude Code.
Run the existing build-install-test job as a matrix over WITH_FHS OFF and ON. The test scripts go through the installed grass command and sessions, so they exercise the FHS layout end to end, including the gunittest suite against the installed tree. Local progress-tracking commit, not for a PR. Written with Claude Code.
Add a CMake subsection to INSTALL.md covering the basic build, the two layouts, GRASS_INSTALL_PYDIR, and the resource environment variable contract. Note the FHS library location for test setup in AGENTS.md. Record branch progress in the local plan document. Local progress-tracking commit, not for a PR. Written with Claude Code.
When a GRASS_* resource variable is not set, fall back to the legacy layout location relative to GISBASE so that environments created from GISBASE alone keep working with legacy installations. Fail fatally only when the variable is not set and the fallback directory does not exist, which is the case for an FHS installation without a properly created session. Verified with a bare-GISBASE environment: legacy build lists color rules through the fallback; FHS build reports the incomplete session error cleanly. Local progress-tracking commit, not for a PR. Written with Claude Code.
Split the GUI between code and resources per FHS: Python code stays in the Python package (site-packages/grass/gui), while icons, images, and xml (menu definitions and DTDs) install to share/grass/gui. The directory structure below the resource root is the same in both layouts, so the legacy layout is unchanged. Introduce GRASS_GUIXMLDIR for the wxGUI menu xml directory, exported like the other resource variables and substituted by both build systems, and use it in the GUI instead of joining the code directory with xml. Verified: fresh FHS build and incremental legacy build complete; xml, icons, and images are absent from the FHS Python package and present under share/grass/gui; menudata generation, interface parsing, and the grass.app and grass.script tests pass against the FHS install; legacy paths are unchanged. Local progress-tracking commit, not for a PR. Written with Claude Code.
Export GRASS_LIBRARY_DIR from the GRASS CMake package and use it for the addon install RPATH in build_addon.cmake instead of GISBASE/lib, which does not contain the libraries in the FHS layout. Add the GRASS_LIBDIR resource path (both build systems) and use it for the session dynamic library path with the legacy location as fallback. Verified against the FHS install: g.extension installs both a Python addon (d.region.grid) and a C addon (r.area); the C addon links, loads, and runs, with RPATH pointing to the real library directory. Addons install into GRASS_ADDON_BASE with the legacy user-directory layout regardless of the core layout. Local progress-tracking commit, not for a PR. Written with Claude Code.
Configure grass.pc from the actual install directories and install it into the pkgconfig directory under the prefix libdir for both layouts. The Autotools build configures its own grass.pc at the top level but never installs it; the CMake template mirrors its content. Verified by compiling and running a program against the FHS install using pkg-config --cflags --libs. Record the resolved design decisions in the plan document. Local progress-tracking commit, not for a PR. Written with Claude Code.
Rewrite the wxGUI's flat sys.path imports (from core import ...) to package imports (from grassgui.core import ...) across 190 files and add a package init, making the GUI a proper top-level Python package separate from the grass library package. Under FHS, the package installs as site-packages/grassgui and site-packages/grass/gui is gone. In the legacy layout, files stay in the Autotools-compatible location (gui/wxpython under GISBASE) and the same init file installed into etc/python/grassgui acts as a stand-in that redirects the package content to the wxGUI directory, so both build systems keep working. grass.script.setup.set_gui_path() is kept unchanged as a compatibility path for GUI addons that still use flat imports. Also fix another staged-copy staleness bug: the g.gui stamp command in build_gui_in_subdir.cmake had no DEPENDS on the Python files, so source edits were never re-staged in incremental builds. Verified: fresh FHS and incremental legacy builds complete including menudata and menustrings generation; import grassgui and grassgui.core work in sessions on both layouts (real package on FHS, redirect on legacy); g.gui.animation --help runs on the FHS install; grass.app and grass.script tests pass (231). The bulk import rewrite was done mechanically by a script written with Claude Code. Local progress-tracking commit, not for a PR.
…session Convert the two GUI imports outside the wxGUI itself (d.rast.edit and wxpyimgview) to grassgui imports and drop their set_gui_path() calls. Give the legacy stand-in package a fallback that locates the wxGUI directory relative to its own location, so grassgui resolves in environments without session variables, such as the pylint CI job which only sets PYTHONPATH from --config python_path. Verified: both layouts rebuild; import grassgui resolves grassgui.core.globalvar in a bare environment with only PYTHONPATH set; d.rast.edit runs on both layouts. Local progress-tracking commit, not for a PR. Written with Claude Code.
Add a package __main__ which runs the wxGUI main, staged and installed by both build systems, and a gui subcommand in the experimental CLI which creates a session (temporary project by default, --project to use an existing one) and runs the GUI as a blocking process so the session lives as long as the GUI. Register the subcommand in the startup dispatch list. Verified with a real display: g.gui -f launches the GUI on all three builds (FHS and legacy CMake, Autotools), python -m grassgui and grass gui launch it on the FHS install; processes ran until killed with no import errors. The Autotools build compiles from scratch with no errors and passes the session, shim import, tool, man, and bare-GISBASE fallback checks. Local progress-tracking commit, not for a PR. Written with Claude Code.
| """ | ||
|
|
||
| import os as _os | ||
|
|
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| import pathlib | |
| # case; when the content is next to this file (FHS layout), do nothing. | ||
| # The relative fallback covers environments without session variables | ||
| # (the stand-in is at <gisbase>/etc/python/grassgui/__init__.py). | ||
| if not _os.path.isdir(_os.path.join(_os.path.dirname(__file__), "core")): |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| if not _os.path.isdir(_os.path.join(_os.path.dirname(__file__), "core")): | |
| if not pathlib.Path(_os.path.join(_os.path.dirname(__file__), "core")).is_dir(): |
| # (the stand-in is at <gisbase>/etc/python/grassgui/__init__.py). | ||
| if not _os.path.isdir(_os.path.join(_os.path.dirname(__file__), "core")): | ||
| _guiwx = _os.environ.get("GRASS_GUIWXDIR") | ||
| if not _guiwx or not _os.path.isdir(_guiwx): |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| if not _guiwx or not _os.path.isdir(_guiwx): | |
| if not _guiwx or not pathlib.Path(_guiwx).is_dir(): |
| ) | ||
| ) | ||
| _guiwx = _os.path.join(_base, "gui", "wxpython") | ||
| if _os.path.isdir(_guiwx): |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| if _os.path.isdir(_guiwx): | |
| if pathlib.Path(_guiwx).is_dir(): |
| from grassgui.core.gcmd import RunCommand, GWarning | ||
|
|
||
| from grassgui.animation.mapwindow import AnimationWindow | ||
| from grassgui.animation.provider import BitmapProvider, BitmapPool, MapFilesPool, CleanUp |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| from grassgui.animation.provider import BitmapProvider, BitmapPool, MapFilesPool, CleanUp | |
| from grassgui.animation.provider import ( | |
| BitmapProvider, | |
| BitmapPool, | |
| MapFilesPool, | |
| CleanUp, | |
| ) |
| from grassgui.core.gcmd import GError, GMessage, GWarning, RunCommand | ||
| from grassgui.gui_core.forms import CmdPanel | ||
| from grassgui.gui_core.gselect import GdalSelect | ||
| from grassgui.gui_core.widgets import GListCtrl, GNotebook, LayersList, LayersListValidator |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| from grassgui.gui_core.widgets import GListCtrl, GNotebook, LayersList, LayersListValidator | |
| from grassgui.gui_core.widgets import ( | |
| GListCtrl, | |
| GNotebook, | |
| LayersList, | |
| LayersListValidator, | |
| ) |
| from psmap.menudata import PsMapMenuData | ||
| from psmap.toolbars import PsMapToolbar | ||
| from psmap.utils import ( | ||
| from grassgui.psmap.instructions import InitMap, Instruction, NewId, SetResolution, PageSetup |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| from grassgui.psmap.instructions import InitMap, Instruction, NewId, SetResolution, PageSetup | |
| from grassgui.psmap.instructions import ( | |
| InitMap, | |
| Instruction, | |
| NewId, | |
| SetResolution, | |
| PageSetup, | |
| ) |
| from grassgui.gui_core.gselect import ColumnSelect | ||
| from grassgui.core.units import Units | ||
| from grassgui.core.settings import UserSettings | ||
| from grassgui.gui_core.wrap import Button, CheckBox, FloatSpin, SpinCtrl, StaticBox, StaticText |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| from grassgui.gui_core.wrap import Button, CheckBox, FloatSpin, SpinCtrl, StaticBox, StaticText | |
| from grassgui.gui_core.wrap import ( | |
| Button, | |
| CheckBox, | |
| FloatSpin, | |
| SpinCtrl, | |
| StaticBox, | |
| StaticText, | |
| ) |
Comment on lines
+193
to
+195
| _WXPYTHON_BASE = os.environ.get("GRASS_GUIWXDIR") or gpath( | ||
| "gui", "wxpython" | ||
| ) |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| _WXPYTHON_BASE = os.environ.get("GRASS_GUIWXDIR") or gpath( | |
| "gui", "wxpython" | |
| ) | |
| _WXPYTHON_BASE = os.environ.get("GRASS_GUIWXDIR") or gpath("gui", "wxpython") |
Comment on lines
+286
to
+288
| subparser = subparsers.add_parser( | ||
| "gui", help="launch the graphical user interface" | ||
| ) |
There was a problem hiding this comment.
[ruff] reported by reviewdog 🐶
Suggested change
| subparser = subparsers.add_parser( | |
| "gui", help="launch the graphical user interface" | |
| ) | |
| subparser = subparsers.add_parser("gui", help="launch the graphical user interface") |
Add pytest steps (parallel and needs_solo_run passes, mirroring the pytest workflow without coverage) to the CMake workflow so both the legacy and the FHS matrix legs run pytest against the installed tree; previously only the gunittest suite ran there and pytest was Autotools-only. Add grass --config lib_path reporting the runtime library directory so test setups do not hardcode the legacy GISBASE/lib location, which is wrong under FHS; used by the new steps for LD_LIBRARY_PATH. Verified: --config lib_path returns the prefix libdir on the FHS install and dist/lib on Autotools; the CI environment recipe passes a pytest subset against the FHS install (232 passed). Local progress-tracking commit, not for a PR. Written with Claude Code.
The lock program in g.mapset and the seg and ram programs in the r.watershed command line construction were looked up with G_etc_dir(), which is the arch-independent data directory and differs from the binaries directory in the FHS layout. Use G_etcbin_dir() as the front-end of r.watershed and d.mon already do. This broke every gunittest creating a mapset on the FHS CI leg (lock file of mapset cannot be checked). Also look up the r.in.wms support modules through GRASS_ETCBINDIR instead of deriving the path from the script location. Verified on the FHS install: g.mapset -c switches mapsets and the previously failing test setUp proceeds past mapset creation. Local progress-tracking commit, not for a PR. Written with Claude Code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Working branch for completing the Filesystem Hierarchy Standard support in the CMake build (
WITH_FHS=ON) while keeping the legacy Autotools-compatible layout as the default, plus the conversion of the wxGUI into a separate importablegrassguiPython package. The plan and progress log are inFHS_PLAN.md.What is in the branch
G_init_locale()recursed without bound whenGRASS_LOCALEDIRwas unset; likely the Windowsg.proj0xC00000FD failure on PR build: prepare for FHS, where GISBASE is not the absolute truth OSGeo/grass#5630.GRASS_INSTALL_PYDIR, removal of the configure-time symlink workaround, re-enabled GUI menudata/man index/translation status targets, fontcap path rewriting at install time, demolocationGISDBASEfix.GRASS_PYDIR,GRASS_MANDIR,GRASS_LIBDIR,GRASS_GUIXMLDIRresource paths in both build systems; sessionPYTHONPATH,MANPATH, and dynamic library path derived from them with legacy fallbacks;g.manualconverted;grass --config python_pathreports the resolved path.share/grass/guiunder FHS; code became the importable top-levelgrassguipackage (flat imports rewritten across ~190 files) with a redirecting stand-in package for the legacy layout;python -m grassguiand agrass guisubcommand launch the GUI.g.extensionworks against an FHS install for Python and C addons (addon RPATH from the exportedGRASS_LIBRARY_DIR); CMake builds now installgrass.pc; the CMake CI workflow is aWITH_FHSOFF/ON matrix; INSTALL.md documents the CMake build and both layouts.Verification
FHS CMake, legacy CMake, and Autotools builds all complete; session smoke tests,
grass man, addon installation, pkg-config compile/link, and pytest forgrass.app/grass.script/r.slope.aspect pass against the installed FHS tree; the GUI launches on a real display from all three builds (g.gui -f,python -m grassgui,grass gui).Written with Claude Code: the changes were developed, tested, and committed in an AI-assisted session, including the mechanical import rewrite done by a script.
🤖 Generated with Claude Code
https://claude.ai/code/session_017wDYMKHYcXzqVpfRe1Qarb