Repository navigation
Conversation
echoix
reviewed
Oct 12, 2026
Comment on lines
+1203
to
+1204
| # On Windows, the C library has its own copy of the environment, | ||
| # and with fork, it may have read another session already. |
Member
There was a problem hiding this comment.
What are you meaning here? That fork is used on Windows, and does the wrong thing here?
If you relied on this assumption, we must review this PR very attentively.
You must know that the fork syscall doesn't exist at all on Windows. It's a Linux concept. On Windows you create a process, on Linux you fork an existing one, and replace info if needed (approximately that)..
Contributor
Author
There was a problem hiding this comment.
No, fork isn't assumed on Windows, the comment just mixed two separate cases. Fixed it.
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.
Part of #7737.
tgis.init()andtgis.SQLDatabaseInterfaceConnection()now acceptenv, so the temporal framework can be initialized for a session which is not inos.environ, as proposed in the issue:With
env=None,os.environis used as before. This is the first stage from the issue:init()and the messenger and C-library subprocesses it starts. The dataset classes and the remaininggrass.scriptcalls still useos.environand are left for follow-ups.Details (generated with Claude Code)
Changes
init()reads the session (g.gisenv), runst.connect, and readsGRASS_TGIS_RAISE_ON_ERRORandGISBASE(for the SQL templates) fromenv. It also usesenvfor its own database connection and for the database info in version messages.get_sql_template_path(),create_temporal_database()andget_database_info_string()get anenvparameter for this.SQLDatabaseInterfaceConnection(env=...)lists the temporal databases witht.connectinenv, also inadd_mapset(), throughget_available_temporal_mapsets(env=...). The module-levelTools()object is replaced byTools(env=env), because it resolvedt.connectwith thePATHofos.environ.Messenger(env=...)andCLibrariesInterface(env=...)start their subprocess inenv.os.environbefore the first libgis call, because libgis reads the session on first use.GISRCwithG_putenv()and rereads the session file. On Windows, libgis has its own copy of the environment (see the skip reason inlib/gis/tests/lib_gis_env_test.py). With the fork start method, libgis may already have read another session in the parent.envis copied to a dict because the spawn start method pickles the arguments, andos.environcannot be pickled.Behavior changes with
env=Noneinit()creates its ownMessengerinstead of using the shared one fromget_msgr(). On main,init()stops the shared messenger at a session change.get_msgr()then returns the same stopped object, and the next message restarts it with "WARNING: Needed to restart the messenger server". The new messenger uses theraise_on_errorof the currentinit()call.init()also restarts the subprocesses whenGISRCchanges, or when they were started with a differentenv. On main, a new session for the same mapset left the C-library server on the deleted session file of the previous session.available_mapsets()then returned[], with "Incomplete GRASS session" and a server traceback.Verification (macOS, Python 3.14)
Existing tests on main and on this branch give identical results:
python/grass/temporal,python/grass/pygrass,python/grass/jupyter,scripts/v.what.strdsand all temporal tools (407 tests);grass.temporal, all temporal tools, the messenger doctests andv.what.strdswith nc_spm_08 (49 files).The only failures, 33 in
grass_pygrass_grid_test, fail on main as well.Additional scripts, not added to the repo, covered:
init(env=...)with no session inos.environ, and with a different session there; the database is created in the mapset ofenvand nothing else is touched;envandadd_mapset();env;Reverting any changed line makes one of them fail. The Windows-specific line cannot be checked on macOS. The fork start method used on Linux was simulated on macOS, including a parent which had already read a session through libgis.
Not tested: Windows and the PostgreSQL backend.
Not covered yet
Some code still needs the same session in
os.environ:STRDSMetadata, used byopen_old_stds()andtlist_grouped()) and functions called withoutdbif, because they create a connection withoutenv;env.Once these are converted, the
os.environmirroring ingrass_temporal_gui_support_test.pyand indatacatalog/tree.pycan be removed.SQLDatabaseInterfaceConnection(env=...)has to get the same environment asinit(), because the current mapset and the C-library interface come frominit().With the PostgreSQL backend, libpq still reads the
PG*variables fromos.environ.On Windows, only
GISRCis set in the environment copy that libgis uses in the subprocesses.The investigation and the patch were done with the assistance of Claude Code.
Claude Code was used as a coding assistant.