grass.app: Do env var setup in RuntimePaths#6482
Conversation
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.
…for real installations. 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.
…testing (fails on Windows otherwise)
|
While this works on Linux even with the incomplete conda package, it does not work on macOS in CI because at least one variable is not substituted and path to clean_temp is then |
|
The failing test is one of the weird ones. I originally wrote that test to run through multiprocessing to avoid polluting the main environment while avoiding monkey patching or otherwise fixing the environment or running the a Python subprocess with a code as a string. At that time, the function would always access the global environment which is not the case anymore. Since it is the only test failing, a possible solution is to go through the tests and rewrite them without multiprocessing hacks, although possibly using a full, standard process to check setting of the global environment in a very isolated way. Notably, I added specific tests to check for variable values which were not substituted (in this PR) and checks for consistency between in-source-code (compile-time-determined) paths and dynamic fallbacks. All these tests pass in macOS CI while I can make them fail by breaking my installation locally. These will pass when other tests leak the environment which we know is happening, so that's likely what is happening here. There are other tests which pass including first part of the failing test which does run a GRASS tool. Nevertheless, |
…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.
|
For the record, this PR still keeps the functionality added in #6391, #6393, and #6469 - that is the intention that even broken installation still works if the code can figure out where the other pieces are (command finding the Python package or Python code finding the installation). As expected, running tests which test for path validity fails: The most important result here is the failure "test_install_path_consistent" which tells us that something is wrong with the paths, although most of the code can run through fallbacks. (Tested with HuidaeCho/grass-conda@d6c353e as in #5067.) |
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.