Skip to content

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

Merged
wenzeslaus merged 2 commits intoOSGeo:mainfrom
wenzeslaus:handle-gisbase-more-carefully
Oct 10, 2025
Merged

grass.app: Separate env modification from path retrieval#6469
wenzeslaus merged 2 commits intoOSGeo:mainfrom
wenzeslaus:handle-gisbase-more-carefully

Conversation

@wenzeslaus
Copy link
Member

@wenzeslaus wenzeslaus commented Oct 8, 2025

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).

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 comming 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).
@github-actions github-actions bot added Python Related code is in Python libraries tests Related to Test Suite labels Oct 8, 2025
Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

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

Simple typos when reading.

Otherwise, the PR description is great and didn't find anything that bothered me in the changes.

wenzeslaus added a commit to nilason/grass that referenced this pull request Oct 9, 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.
Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
@wenzeslaus wenzeslaus enabled auto-merge (squash) October 9, 2025 23:09
@wenzeslaus
Copy link
Member Author

Thanks. I would merge this as is and solve further alignment with #5630 in there or in a separate PR.

@wenzeslaus wenzeslaus merged commit a92a3a0 into OSGeo:main Oct 10, 2025
27 of 28 checks passed
@wenzeslaus wenzeslaus deleted the handle-gisbase-more-carefully branch October 10, 2025 01:12
@github-actions github-actions bot added this to the 8.5.0 milestone Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libraries Python Related code is in Python tests Related to Test Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants