grass.app.runtime: Do not add the same paths again on repeated setup - #7840
Open
Pranav-error wants to merge 2 commits into
Open
grass.app.runtime: Do not add the same paths again on repeated setup#7840Pranav-error wants to merge 2 commits into
Pranav-error wants to merge 2 commits into
Conversation
set_executable_paths(), set_dynamic_library_path() and set_python_path_variable() added their entries unconditionally, so calling the setup repeatedly on one environment, as a pytest run or a Jupyter kernel does, grew PATH, PYTHONPATH and the library path variable without bound. Skip entries that are already present. First call behaviour is unchanged, and paths from another installation are still added in front of the existing ones, so switching installations keeps working. set_dynamic_library_path() also assigned an empty value before appending when the variable was unset, which left a leading separator, and an empty entry in a library path means the current directory. It now assigns the path directly in that case. Fixes OSGeo#7738
github-actions
Bot
added
Python
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.
Fixes #7738.
set_executable_paths(),set_dynamic_library_path()andset_python_path_variable()added their entries unconditionally, so calling the setup repeatedly on one environment grewPATH,PYTHONPATHand the library path variable without bound. That happens in a pytest run and in a Jupyter kernel, wheregs.setup.init()is called many times onos.environ.All three now skip an entry that is already present, which is the fix suggested in the issue. First call behaviour is unchanged, and paths from another installation are still added in front of the existing ones, so switching installations keeps working. I did not route
init()throughensure_runtime_env(), sinceruntime_env_is_active()only checks that GISBASE appears inPATHand would skip a re-setup that is actually needed.set_dynamic_library_path()also assigned""before appending when the variable was unset, leaving a leading separator. An empty entry in a library path means the current directory, so it now assigns the path directly in that case.Tests
Seven tests in
python/grass/app/tests/grass_app_runtime_test.py, covering, for each setter, that a second call changes nothing, and that an existing value is preserved. Two more cover the empty-entry case and that a second installation's paths still come first.Checked against the unfixed module first: the four idempotency and empty-entry tests fail there and pass with the change. The 41 existing tests in that file still pass.
I used an AI assistant while preparing this. I understand the change and can explain it.