grass.app.runtime: Do not add the same paths again on repeated setup

[Pranav-error]

Fixes #7738.

set_executable_paths(), set_dynamic_library_path() and set_python_path_variable() added their entries unconditionally, so calling the setup repeatedly on one environment grew PATH, PYTHONPATH and the library path variable without bound. That happens in a pytest run and in a Jupyter kernel, where gs.setup.init() is called many times on os.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() through ensure_runtime_env(), since runtime_env_is_active() only checks that GISBASE appears in PATH and 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.

[nilason]

Looks good to me, I have only a minor suggestion. I'll leave the final approval and merge to a colleague for another pair of eyes on this.

[nilason]

Suggested change

	# Assigning rather than appending to an empty value avoids a leading
	# separator, which would be an empty entry meaning the current directory.

Not worth the comment in code, but please add to commit description.

No need to explain every line of code, I think.

[Pranav-error]

Done — comment removed and the explanation moved into the commit message:

In set_dynamic_library_path() the value is assigned rather than appended when the variable is empty. Appending to an empty value would leave a leading separator, and an empty entry in a path list means the current directory.

Also rebased onto current main while I was in there.

Fair point on not explaining every line. That one was load-bearing enough that losing it would invite someone to "simplify" the assignment back into an append, which is why I had written it down — but the commit message is the better place for it, since that is where someone doing the simplifying would look.

[echoix]

That reads weird to me. Is there missing parentheses for me to understand well?

I read it like this:

        existing + os.pathsep + (library_path if existing else library_path)

Which is a useless if.

If it resolves to

        (existing + os.pathsep + library_path) if existing else library_path

Then it makes a bit more sense

[echoix]

Is the deduplication keeping the paths in the same order, keeping the left-most instance? And is it only affecting what we are adding, not pre-existing paths set by the user or software outside of us?

My comment might mean contradictory instructions though. But not keeping a path in front because it it found later on in the list might not be equivalent, if a binary of the same name exists in one of these other locations

[Pranav-error]

Both are worth answering precisely rather than from memory, so I went back to the code for each.

Line 357 (set_dynamic_library_path): the parens aren't missing — the code is already correct, though I agree it reads ambiguously. Python's conditional expression (A if cond else B) binds looser than +, so this:

env[variable_name] = (
    existing + os.pathsep + library_path if existing else library_path
)

parses as (existing + os.pathsep + library_path) if existing else library_path, not existing + os.pathsep + (library_path if existing else library_path). That's the intended behaviour — append with a separator when there's an existing value, otherwise just the bare path. Happy to add the explicit parens you suggested so it doesn't need a precedence check to read.

Line 291 (set_executable_paths dedup):

existing = env.get("PATH") or ""
present = existing.split(os.pathsep) if existing else []
paths = [path for path in paths if path not in present]

Yes to both parts of your question. paths starts as a deque built left-to-right by append_left_addon_paths / append_left_main_executable_paths — only the new entries, in that original order. The comprehension filters that deque against present without reordering it, so the left-most/original order among the new entries is preserved. It doesn't dedupe the new entries against each other, only against what's already in PATH — though two identical new entries isn't a case that can actually happen given how the deque is built.

And it's scoped to only what's being added: existing (the pre-existing PATH value) is never split, reordered, or deduped — it's appended back whole and rejoined at the end. So a duplicate that was already in the user's PATH before this function ran stays exactly as it was; only what GRASS itself is about to add gets checked against it.

Will push the explicit parens from the first point as a follow-up commit.