From c341e544a748e3368de61f58a5d62e81a06053a9 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 15 Jun 2021 22:19:47 -0700 Subject: [PATCH] Error out on attempt to activate a missing tools (#838) Previously if a tool (any part of an SDK) was not installed we would issue a warning and continue to active without returning non-zero. This meant doing `emsdk install 2.0.0 && emsdk activate latest` would appear to be work aside from the warning messages about latest not being installed. This is especially annoying since we dropped support for side by side SDK installations. The following sequence is no longer valid and we want to make that clear by erroring out: ``` $ emsdk install 2.0.1 $ emsdk install 2.0.2 $ emsdk activate 2.0.1 ``` Since 2.0.2 replaces 2.0.1 on the filesystem the active here could fail hard rather than just warning. --- emsdk.py | 17 +++-------------- test/test.py | 8 ++++++-- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/emsdk.py b/emsdk.py index a1d15d9..e130078 100644 --- a/emsdk.py +++ b/emsdk.py @@ -2497,19 +2497,6 @@ def can_simultaneously_activate(tool1, tool2): return tool1.id != tool2.id -def remove_nonexisting_tools(tool_list, log_errors=True): - i = 0 - while i < len(tool_list): - tool = tool_list[i] - if not tool.is_installed(): - if log_errors: - errlog("Warning: The SDK/tool '" + str(tool) + "' cannot be activated since it is not installed! Skipping this tool...") - tool_list.pop(i) - continue - i += 1 - return tool_list - - # Expands dependencies for each tool, and removes ones that don't exist. def process_tool_list(tools_to_activate, log_errors=True): i = 0 @@ -2520,7 +2507,9 @@ def process_tool_list(tools_to_activate, log_errors=True): tools_to_activate = tools_to_activate[:i] + deps + tools_to_activate[i:] i += len(deps) + 1 - tools_to_activate = remove_nonexisting_tools(tools_to_activate, log_errors=log_errors) + for tool in tools_to_activate: + if not tool.is_installed(): + exit_with_error("error: tool is not installed and therefore cannot be activated: '%s'" % tool) # Remove conflicting tools i = 0 diff --git a/test/test.py b/test/test.py index 7b02189..8428149 100755 --- a/test/test.py +++ b/test/test.py @@ -64,8 +64,8 @@ def failing_call_with_output(cmd, expected): if WINDOWS: print('warning: skipping part of failing_call_with_output() due to error codes not being propagated (see #592)') else: - assert proc.returncode, 'call must have failed: ' + str([stdout, "\n========\n", stderr]) - assert expected in stdout or expected in stderr, 'call did not have the right output' + assert proc.returncode, 'call must have failed: ' + str([stdout, '\n========\n', stderr]) + assert expected in stdout or expected in stderr, 'call did not have the right output: ' + str([stdout, '\n========\n', stderr]) def hack_emsdk(marker, replacement): @@ -262,6 +262,10 @@ int main() { # Test that its possible to install emscripten as tool instead of SDK checked_call_with_output(emsdk + ' install releases-upstream-77b065ace39e6ab21446e13f92897f956c80476a', unexpected='Installing SDK') + def test_activate_missing(self): + run_emsdk('install latest') + failing_call_with_output(emsdk + ' activate 2.0.1', expected="error: tool is not installed and therefore cannot be activated: 'releases-upstream-13e29bd55185e3c12802bc090b4507901856b2ba-64bit'") + if __name__ == '__main__': unittest.main(verbosity=2)