Skip to content

Conversation

@eoudejans
Copy link
Collaborator

@eoudejans eoudejans commented May 27, 2025

Python Subprocess.Popen env variable sets the process environment block. This PR makes sure that placeholders are also resolved based on the environment block, next to known or unknown registry keys. Without this PR the env parameter cannot be used to communicate with geodms, in light of the regression testing rework.

Simple test code to test out interpreting the environment block from c++:

geodms:

container test
{
	parameter<string> test := Expand(.,"%SourceDataDir%"), StorageName="%projDir%/data/test.txt", StorageType="str", StorageReadOnly="False";
}

python:

import subprocess
import os

cmd_parts = ['cmd','/k', 'title', 'test','&&', 'call', 'C:/dev/geodms/geodms_v17/bin/Release/x64/GeoDmsGuiQt.exe','/S1','/S2', '/S3',\
             'C:/Users/Cicada/Desktop/test_subprocess_env/cfg/test_subprocess_env.dms','test']

custom_env = os.environ.copy()
custom_env['SourceDataDir'] = 'I changed the sourcedatadir using the environment block'

parent_process_open_handle = subprocess.Popen(
    cmd_parts,
    cwd=None, env=custom_env, 
    creationflags=subprocess.CREATE_NEW_CONSOLE
)

Output in "%projDir%/data/test.txt" should be equal to custom_env['SourceDataDir'].

…. See https://learn.microsoft.com/en-us/windows/win32/procthread/environment-variables for more information. Required for instance to properly deal with Python Subprocess.Popen env parameter which sets the environment block.
@MaartenHilferink
Copy link
Collaborator

Note that GetConvertedRegConfigSetting, called at line 408 -> 420 calls GetRegConfigSetting(..., key), which calls PlatformInfo::GetEnvString(Överridable", key), which calls GetEnv("GEODMS_Overridable_"+key); but for SourceDataDir, this is now shielded off by the earlier call (line 403) to GetPlaceholderValue(configDir, "SourceDataDir"), which returns GetSourceDataDir().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants