Fix environment variables to be consistently set when using --system or --permanent (#515)
Also switch PostMessage to SendMessageWithTimeout and only send one if
environment variables actually changed.
Also, enable and fix --permanent unit tests.
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 4a2b452..6a5e16f 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -91,12 +91,7 @@
name: --permanent test
shell: powershell.exe
command: |
- try {
- scripts/test_permanent.ps1
- } catch {
- Write-Host "`n Error Message: [$($_.Exception.Message)"] -ForegroundColor Red
- Write-Host "`n Line Number: "$_.InvocationInfo.ScriptLineNumber -ForegroundColor Red
- }
+ scripts/test_permanent.ps1
build-docker-image:
executor: bionic
diff --git a/emsdk.py b/emsdk.py
index ee5452d..5032244 100644
--- a/emsdk.py
+++ b/emsdk.py
@@ -299,6 +299,7 @@
def win_set_environment_variable_direct(key, value, system=True):
+ folder = None
try:
if system:
# Read globally from ALL USERS section.
@@ -308,23 +309,22 @@
folder = winreg.OpenKeyEx(winreg.HKEY_CURRENT_USER, 'Environment', 0, winreg.KEY_ALL_ACCESS)
winreg.SetValueEx(folder, key, 0, winreg.REG_EXPAND_SZ, value)
debug_print('Set key=' + key + ' with value ' + value + ' in registry.')
+ return True
except Exception as e:
# 'Access is denied.'
- if e.args[0] == 5:
+ if e.args[3] == 5:
exit_with_error('Error! Failed to set the environment variable \'' + key + '\'! Setting environment variables permanently requires administrator access. Please rerun this command with administrative privileges. This can be done for example by holding down the Ctrl and Shift keys while opening a command prompt in start menu.')
errlog('Failed to write environment variable ' + key + ':')
errlog(str(e))
- folder.Close()
- return None
-
- folder.Close()
- HWND_BROADCAST = ctypes.wintypes.HWND(0xFFFF) # win32con.HWND_BROADCAST == 65535
- WM_SETTINGCHANGE = 0x001A # win32con.WM_SETTINGCHANGE == 26
- ctypes.windll.user32.SendMessageA(HWND_BROADCAST, WM_SETTINGCHANGE, 0, 'Environment')
+ return False
+ finally:
+ if folder is not None:
+ folder.Close()
-def win_get_environment_variable(key, system=True):
+def win_get_environment_variable(key, system=True, fallback=True):
try:
+ folder = None
try:
if system:
# Read globally from ALL USERS section.
@@ -334,22 +334,24 @@
folder = winreg.OpenKey(winreg.HKEY_CURRENT_USER, 'Environment')
value = str(winreg.QueryValueEx(folder, key)[0])
except Exception:
- # PyWin32 is not available - read via os.environ. This has the drawback
+ # If reading registry fails for some reason - read via os.environ. This has the drawback
# that expansion items such as %PROGRAMFILES% will have been expanded, so
# need to be precise not to set these back to system registry, or
# expansion items would be lost.
- return os.environ[key]
+ if fallback:
+ return os.environ[key]
+ return None
+ finally:
+ if folder is not None:
+ folder.Close()
+
except Exception as e:
+ # this catch is if both the registry key threw an exception and the key is not in os.environ
if e.args[0] != 2:
# 'The system cannot find the file specified.'
errlog('Failed to read environment variable ' + key + ':')
errlog(str(e))
- try:
- folder.Close()
- except Exception:
- pass
return None
- folder.Close()
return value
@@ -367,11 +369,11 @@
def win_set_environment_variable(key, value, system=True):
debug_print('set ' + str(key) + '=' + str(value) + ', in system=' + str(system), file=sys.stderr)
- previous_value = win_get_environment_variable(key, system)
+ previous_value = win_get_environment_variable(key, system, fallback=False)
if previous_value == value:
debug_print(' no need to set, since same value already exists.')
# No need to elevate UAC for nothing to set the same value, skip.
- return
+ return False
if not value:
try:
@@ -382,13 +384,12 @@
debug_print(str(cmd))
value = subprocess.call(cmd, stdout=subprocess.PIPE)
except Exception:
- return
- return
+ return False
+ return True
try:
- if system:
- win_set_environment_variable_direct(key, value, system)
- return
+ if win_set_environment_variable_direct(key, value, system):
+ return True
# Escape % signs so that we don't expand references to environment variables.
value = value.replace('%', '^%')
if len(value) >= 1024:
@@ -398,15 +399,53 @@
retcode = subprocess.call(cmd, stdout=subprocess.PIPE)
if retcode != 0:
errlog('ERROR! Failed to set environment variable ' + key + '=' + value + '. You may need to set it manually.')
+ else:
+ return True
except Exception as e:
errlog('ERROR! Failed to set environment variable ' + key + '=' + value + ':')
errlog(str(e))
errlog('You may need to set it manually.')
+ return False
+
+
+def win_set_environment_variables(env_vars_to_add, system):
+ if not env_vars_to_add:
+ return
+
+ changed = False
+
+ for key, value in env_vars_to_add:
+ if win_set_environment_variable(key, value, system):
+ if not changed:
+ changed = True
+ print('Setting global environment variables:')
+
+ print(key + ' = ' + value)
+
+ if not changed:
+ print('Global environment variables up to date')
+ return
+
+ # if changes were made then we need to notify other processes
+ try:
+ HWND_BROADCAST = ctypes.wintypes.HWND(0xFFFF) # win32con.HWND_BROADCAST == 65535
+ WM_SETTINGCHANGE = 0x001A # win32con.WM_SETTINGCHANGE == 26
+ SMTO_BLOCK = 0x0001 # win32con.SMTO_BLOCK == 1
+ ctypes.windll.user32.SendMessageTimeoutA(
+ HWND_BROADCAST, # hWnd: notify everyone
+ WM_SETTINGCHANGE, # Msg: registry changed
+ 0, # wParam: Must be 0 when setting changed is sent by users
+ 'Environment', # lParam: Specifically environment variables changed
+ SMTO_BLOCK, # fuFlags: Wait for message to be sent or timeout
+ 100) # uTimeout: 100ms
+ except Exception as e:
+ errlog('SendMessageTimeout failed with error: ' + str(e))
+
def win_delete_environment_variable(key, system=True):
debug_print('win_delete_environment_variable(key=' + key + ', system=' + str(system) + ')')
- win_set_environment_variable(key, None, system)
+ return win_set_environment_variable(key, None, system)
# Returns the absolute pathname to the given path inside the Emscripten SDK.
@@ -1770,14 +1809,6 @@
return False
return True
- def win_activate_env_vars(self, system):
- if WINDOWS:
- envs = self.activated_environment()
- for env in envs:
- key, value = parse_key_value(env)
-
- win_set_environment_variable(key, value, system)
-
# If this tool can be installed on this system, this function returns True.
# Otherwise, this function returns a string that describes the reason why this
# tool is not available.
@@ -2416,20 +2447,14 @@
# calling shell environment. On other platform `source emsdk_env.sh` is
# required.
if WINDOWS:
- env_string = construct_env(tools_to_activate)
+ # always set local environment variables since permanently activating will only set the registry settings and
+ # will not affect the current session
+ env_vars_to_add = get_env_vars_to_add(tools_to_activate)
+ env_string = construct_env_with_vars(env_vars_to_add)
write_set_env_script(env_string)
- # Apply environment variables to global all users section.
- if WINDOWS and permanently_activate:
- # Individual env. vars
- for tool in tools_to_activate:
- tool.win_activate_env_vars(system=system)
-
- # PATH variable
- newpath, added_items = adjusted_path(tools_to_activate, system=system)
- # Are there any actual changes?
- if newpath != os.environ['PATH']:
- win_set_environment_variable('PATH', newpath, system=system)
+ if permanently_activate:
+ win_set_environment_variables(env_vars_to_add, system)
return tools_to_activate
@@ -2504,22 +2529,14 @@
return (separator.join(whole_path), new_emsdk_tools)
-def construct_env(tools_to_activate):
- env_string = ''
+def get_env_vars_to_add(tools_to_activate):
+ env_vars_to_add = []
+
newpath, added_path = adjusted_path(tools_to_activate)
# Don't bother setting the path if there are no changes.
if os.environ['PATH'] != newpath:
- if POWERSHELL:
- env_string += '$env:PATH="' + newpath + '"\n'
- elif CMD:
- env_string += 'SET PATH=' + newpath + '\n'
- elif CSH:
- env_string += 'setenv PATH "' + newpath + '";\n'
- elif BASH:
- env_string += 'export PATH="' + newpath + '";\n'
- else:
- assert False
+ env_vars_to_add += [('PATH', newpath)]
if added_path:
errlog('Adding directories to PATH:')
@@ -2528,11 +2545,8 @@
errlog('')
# A core variable EMSDK points to the root of Emscripten SDK directory.
- env_vars = [('EMSDK', to_unix_path(emsdk_path()))]
-
- em_config_path = os.path.normpath(dot_emscripten_path())
- if to_unix_path(os.environ.get('EM_CONFIG', '')) != to_unix_path(em_config_path):
- env_vars += [('EM_CONFIG', em_config_path)]
+ env_vars_to_add += [('EMSDK', to_unix_path(emsdk_path()))]
+ env_vars_to_add += [('EM_CONFIG', os.path.normpath(dot_emscripten_path()))]
for tool in tools_to_activate:
config = tool.activated_config()
@@ -2540,27 +2554,40 @@
# For older emscripten versions that don't use this default we export
# EM_CACHE.
em_cache_dir = os.path.join(config['EMSCRIPTEN_ROOT'], 'cache')
- env_vars += [('EM_CACHE', em_cache_dir)]
+ env_vars_to_add += [('EM_CACHE', em_cache_dir)]
envs = tool.activated_environment()
for env in envs:
key, value = parse_key_value(env)
value = to_native_path(tool.expand_vars(value))
- env_vars += [(key, value)]
+ env_vars_to_add += [(key, value)]
- if env_vars:
+ return env_vars_to_add
+
+
+def construct_env(tools_to_activate):
+ return construct_env_with_vars(get_env_vars_to_add(tools_to_activate))
+
+
+def construct_env_with_vars(env_vars_to_add):
+ env_string = ''
+ if env_vars_to_add:
errlog('Setting environment variables:')
- for key, value in env_vars:
- if POWERSHELL:
- env_string += '$env:' + key + '="' + value + '"\n'
- elif CMD:
- env_string += 'SET ' + key + '=' + value + '\n'
- elif CSH:
- env_string += 'setenv ' + key + ' "' + value + '";\n'
- elif BASH:
- env_string += 'export ' + key + '="' + value + '";\n'
- else:
- assert False
- if 'EMSDK_PYTHON' in env_vars:
+
+ for key, value in env_vars_to_add:
+ # Don't set env vars which are already set to the correct value.
+ if key not in os.environ or to_unix_path(os.environ[key]) != to_unix_path(value):
+ errlog(key + ' = ' + value)
+ if POWERSHELL:
+ env_string += '$env:' + key + '="' + value + '"\n'
+ elif CMD:
+ env_string += 'SET ' + key + '=' + value + '\n'
+ elif CSH:
+ env_string += 'setenv ' + key + ' "' + value + '"\n'
+ elif BASH:
+ env_string += 'export ' + key + '="' + value + '"\n'
+ else:
+ assert False
+ if 'EMSDK_PYTHON' in env_vars_to_add:
# When using our bundled python we never want the user's
# PYTHONHOME or PYTHONPATH
# See https://github.com/emscripten-core/emsdk/issues/598
@@ -2579,7 +2606,6 @@
else:
assert False
- errlog(key + ' = ' + value)
return env_string
diff --git a/scripts/test_permanent.ps1 b/scripts/test_permanent.ps1
index 01ec1f1..4c9da19 100644
--- a/scripts/test_permanent.ps1
+++ b/scripts/test_permanent.ps1
@@ -33,24 +33,24 @@
$path_split = $PATH_USER.Split(';')
-$EMSDK_Path_USER = $path_split | Where-Object { $_ -match 'emsdk' }
+$EMSDK_Path_USER = $path_split | Where-Object { $_ -like "$repo_root*" }
if (!$EMSDK_Path_USER) {
throw "No path is added!"
}
-$EMSDK_NODE_Path_USER = $path_split | Where-Object { $_ -match 'emsdk\\node' }
+$EMSDK_NODE_Path_USER = $path_split | Where-Object { $_ -like "$repo_root\node*" }
if (!$EMSDK_NODE_Path_USER) {
- throw "emsdk\node is not added to path."
+ throw "$repo_root\\node is not added to path."
}
-$EMSDK_PYTHON_Path_USER = $path_split | Where-Object { $_ -match 'emsdk\\python' }
+$EMSDK_PYTHON_Path_USER = $path_split | Where-Object { $_ -like "$repo_root\python*" }
if (!$EMSDK_PYTHON_Path_USER) {
- throw "emsdk\python is not added to path."
+ throw "$repo_root\\python is not added to path."
}
-$EMSDK_JAVA_Path_USER = $path_split | Where-Object { $_ -match 'emsdk\\java' }
+$EMSDK_JAVA_Path_USER = $path_split | Where-Object { $_ -like "$repo_root\java*" }
if (!$EMSDK_JAVA_Path_USER) {
- throw "emsdk\java is not added to path."
+ throw "$repo_root\\java is not added to path."
}
-$EMSDK_UPSTREAM_Path_USER = $path_split | Where-Object { $_ -match 'emsdk\\upstream\\emscripten' }
+$EMSDK_UPSTREAM_Path_USER = $path_split | Where-Object { $_ -like "$repo_root\upstream\emscripten*" }
if (!$EMSDK_UPSTREAM_Path_USER) {
- throw "emsdk\upstream\emscripten is not added to path."
+ throw "$repo_root\\upstream\emscripten is not added to path."
}
