Skip to content

Commit

Permalink
Avoid exporting EM_CONFIG for modern SDK versions (#1110)
Browse files Browse the repository at this point in the history
Newer versions of emscipten, starting all the way back in 1.39.13, can
automatically locate the `.emscripten` config file that emsdk creates so
there is no need for the explicit EM_CONFIG environment variable.  Its
redundant and adds unnessary noisce/complexity.

Really, adding emcc to the PATH is all the is needed these days.

One nice thing about this change is that it allows folks to run
whichever emcc they want to and have it just work, even if they have
configured emsdk.   Without this change, if I activate emsdk and I run
`some/other/emcc` then emsdk's `EM_CONFIG` will still be present and
override the configuration embedded in `some/other/emcc`.

e.g. in the same shell, with emsdk activated, I can run both these
commands and have them both just work as expected.

```
$ emcc --version
$ /path/to/my/emcc --version
```
  • Loading branch information
sbc100 authored Oct 6, 2022
1 parent b4fd475 commit 3d87d5e
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 15 deletions.
1 change: 0 additions & 1 deletion docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ COPY --from=stage_build /emsdk /emsdk
# This will let use tools offered by this image inside other Docker images
# (sub-stages) or with custom / no entrypoint
ENV EMSDK=/emsdk \
EM_CONFIG=/emsdk/.emscripten \
EMSDK_NODE=/emsdk/node/14.18.2_64bit/bin/node \
PATH="/emsdk:/emsdk/upstream/emscripten:/emsdk/upstream/bin:/emsdk/node/14.18.2_64bit/bin:${PATH}"

Expand Down
8 changes: 6 additions & 2 deletions emsdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -2676,7 +2676,6 @@ def get_env_vars_to_add(tools_to_activate, system, user):

# A core variable EMSDK points to the root of Emscripten SDK directory.
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()
Expand All @@ -2692,6 +2691,9 @@ def get_env_vars_to_add(tools_to_activate, system, user):
# https://github.com/emscripten-core/emscripten/pull/11091
# - Default to embedded cache also started in 1.39.16
# https://github.com/emscripten-core/emscripten/pull/11126
# - Emscripten supports automatically locating the embedded
# config in 1.39.13:
# https://github.com/emscripten-core/emscripten/pull/10935
#
# Since setting EM_CACHE in the environment effects the entire machine
# we want to avoid this except when installing these older emscripten
Expand All @@ -2700,6 +2702,8 @@ def get_env_vars_to_add(tools_to_activate, system, user):
if version < [1, 39, 16]:
em_cache_dir = os.path.join(config['EMSCRIPTEN_ROOT'], 'cache')
env_vars_to_add += [('EM_CACHE', em_cache_dir)]
if version < [1, 39, 13]:
env_vars_to_add += [('EM_CONFIG', os.path.normpath(dot_emscripten_path()))]

envs = tool.activated_environment()
for env in envs:
Expand Down Expand Up @@ -2764,7 +2768,7 @@ def construct_env_with_vars(env_vars_to_add):
'EMSDK_NUM_CORES', 'EMSDK_NOTTY', 'EMSDK_KEEP_DOWNLOADS'])
env_keys_to_add = set(pair[0] for pair in env_vars_to_add)
for key in os.environ:
if key.startswith('EMSDK_') or key.startswith('EM_CACHE'):
if key.startswith('EMSDK_') or key in ('EM_CACHE', 'EM_CONFIG'):
if key not in env_keys_to_add and key not in ignore_keys:
info('Clearing existing environment variable: %s' % key)
env_string += unset_env(key)
Expand Down
4 changes: 2 additions & 2 deletions test/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
WINDOWS = sys.platform.startswith('win')
MACOS = sys.platform == 'darwin'

assert 'EM_CONFIG' in os.environ, "emsdk should be activated before running this script"
emconfig = os.path.abspath('.emscripten')
assert os.path.exists(emconfig)

emconfig = os.environ['EM_CONFIG']
upstream_emcc = os.path.join('upstream', 'emscripten', 'emcc')
fastcomp_emcc = os.path.join('fastcomp', 'emscripten', 'emcc')
emsdk = './emsdk'
Expand Down
7 changes: 0 additions & 7 deletions test/test_activation.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ try {
}

$EMSDK = [System.Environment]::GetEnvironmentVariable("EMSDK", $env_type)
$EM_CONFIG = [System.Environment]::GetEnvironmentVariable("EM_CONFIG", $env_type)
$EMSDK_NODE = [System.Environment]::GetEnvironmentVariable("EMSDK_NODE", $env_type)
$EMSDK_PYTHON = [System.Environment]::GetEnvironmentVariable("EMSDK_PYTHON", $env_type)
$JAVA_HOME = [System.Environment]::GetEnvironmentVariable("JAVA_HOME", $env_type)
Expand All @@ -37,9 +36,6 @@ try {
if (!$EMSDK) {
throw "EMSDK is not set for the user"
}
if (!$EM_CONFIG) {
throw "EM_CONFIG is not set for the user"
}
if (!$EMSDK_NODE) {
throw "EMSDK_NODE is not set for the user"
}
Expand Down Expand Up @@ -83,22 +79,19 @@ finally {

# Recover pre activation env variables
[Environment]::SetEnvironmentVariable("EMSDK", $null, "User")
[Environment]::SetEnvironmentVariable("EM_CONFIG", $null, "User")
[Environment]::SetEnvironmentVariable("EMSDK_NODE", $null, "User")
[Environment]::SetEnvironmentVariable("EMSDK_PYTHON", $null, "User")
[Environment]::SetEnvironmentVariable("JAVA_HOME", $null, "User")

try {
[Environment]::SetEnvironmentVariable("EMSDK", $null, "Machine")
[Environment]::SetEnvironmentVariable("EM_CONFIG", $null, "Machine")
[Environment]::SetEnvironmentVariable("EMSDK_NODE", $null, "Machine")
[Environment]::SetEnvironmentVariable("EMSDK_PYTHON", $null, "Machine")
[Environment]::SetEnvironmentVariable("JAVA_HOME", $null, "Machine")
} catch {}


[Environment]::SetEnvironmentVariable("EMSDK", $null, "Process")
[Environment]::SetEnvironmentVariable("EM_CONFIG", $null, "Process")
[Environment]::SetEnvironmentVariable("EMSDK_NODE", $null, "Process")
[Environment]::SetEnvironmentVariable("EMSDK_PYTHON", $null, "Process")
[Environment]::SetEnvironmentVariable("JAVA_HOME", $null, "Process")
Expand Down
3 changes: 0 additions & 3 deletions test/test_path_preservation.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -123,22 +123,19 @@ finally {

# Recover pre activation env variables
[Environment]::SetEnvironmentVariable("EMSDK", $null, "User")
[Environment]::SetEnvironmentVariable("EM_CONFIG", $null, "User")
[Environment]::SetEnvironmentVariable("EMSDK_NODE", $null, "User")
[Environment]::SetEnvironmentVariable("EMSDK_PYTHON", $null, "User")
[Environment]::SetEnvironmentVariable("JAVA_HOME", $null, "User")

try {
[Environment]::SetEnvironmentVariable("EMSDK", $null, "Machine")
[Environment]::SetEnvironmentVariable("EM_CONFIG", $null, "Machine")
[Environment]::SetEnvironmentVariable("EMSDK_NODE", $null, "Machine")
[Environment]::SetEnvironmentVariable("EMSDK_PYTHON", $null, "Machine")
[Environment]::SetEnvironmentVariable("JAVA_HOME", $null, "Machine")
} catch {}


[Environment]::SetEnvironmentVariable("EMSDK", $null, "Process")
[Environment]::SetEnvironmentVariable("EM_CONFIG", $null, "Process")
[Environment]::SetEnvironmentVariable("EMSDK_NODE", $null, "Process")
[Environment]::SetEnvironmentVariable("EMSDK_PYTHON", $null, "Process")
[Environment]::SetEnvironmentVariable("JAVA_HOME", $null, "Process")
Expand Down

0 comments on commit 3d87d5e

Please sign in to comment.