From 4258c792ffb9fd528f7836b269fcfe6099a56d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Fri, 4 Mar 2022 15:01:29 +0200 Subject: [PATCH 1/6] Fix native Closure Compiler to work. Reverts my earlier PR #803 which was a great mistake, as it caused Emscripten to silently fall back to Java version of the Closure Compiler. After this PR, Closure Compiler will work on user platforms that do not have Java installed. Also forcibly remove Java version of Closure Compiler on systems where installing the native version succeeds, in order to save on code size. --- emsdk.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/emsdk.py b/emsdk.py index 90b4c5561a..ceb60f2fcb 100644 --- a/emsdk.py +++ b/emsdk.py @@ -1411,6 +1411,47 @@ def emscripten_npm_install(tool, directory): errlog('Error running %s:\n%s' % (e.cmd, e.output)) return False + # Manually install the appropriate native Closure Compiler package + # This is currently needed because npm ci will install the packages + # for Closure for all platforms, adding 180MB to the download size + # There are two problems here: + # 1. npm ci does not consider the platform of optional dependencies + # https://github.com/npm/cli/issues/558 + # 2. A bug with the native compiler has bloated the packages from + # 30MB to almost 300MB + # https://github.com/google/closure-compiler-npm/issues/186 + # If either of these bugs are fixed then we can remove this exception + closure_compiler_native = '' + if LINUX and ARCH in ('x86', 'x86_64'): + closure_compiler_native = 'google-closure-compiler-linux' + if MACOS and ARCH in ('x86', 'x86_64'): + closure_compiler_native = 'google-closure-compiler-osx' + if WINDOWS and ARCH == 'x86_64': + closure_compiler_native = 'google-closure-compiler-windows' + + if closure_compiler_native: + closure_compiler_native += '@20220104.0.0' + print('Running post-install step: npm install', closure_compiler_native) + try: + subprocess.check_output( + [npm, 'install', '--production', '--no-optional', closure_compiler_native], + cwd=directory, stderr=subprocess.STDOUT, env=env, + universal_newlines=True) + + # Now that we succeeded to install the native version, delete the Java version + # since that takes up ~12.5MB of installation space that is no longer needed + # as we have a native version of the compiler instead. + # This process is currently a little bit hacky, see https://github.com/google/closure-compiler/issues/3926 + remove_tree(os.path.join(directory, 'node_modules', 'google-closure-compiler-java')) + + # Remove import of Java Closure Compiler module. + compiler_filename = os.path.join(directory, 'node_modules', 'google-closure-compiler', 'lib', 'node', 'closure-compiler.js') + compiler_js = open(compiler_filename, 'r').read().replace("require('google-closure-compiler-java')", "''/*require('google-closure-compiler-java') XXX Removed by Emsdk*/") + open(compiler_filename, 'w').write(compiler_js) + except subprocess.CalledProcessError as e: + errlog('Error running %s:\n%s' % (e.cmd, e.output)) + return False + print('Done running: npm ci') return True From 4efc7a48e78f2c073e616c69dd4ad16eca6d5ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Fri, 4 Mar 2022 15:03:19 +0200 Subject: [PATCH 2/6] Add note to bug --- emsdk.py | 1 + 1 file changed, 1 insertion(+) diff --git a/emsdk.py b/emsdk.py index ceb60f2fcb..8d70c7b9f3 100644 --- a/emsdk.py +++ b/emsdk.py @@ -1421,6 +1421,7 @@ def emscripten_npm_install(tool, directory): # 30MB to almost 300MB # https://github.com/google/closure-compiler-npm/issues/186 # If either of these bugs are fixed then we can remove this exception + # See also https://github.com/google/closure-compiler/issues/3925 closure_compiler_native = '' if LINUX and ARCH in ('x86', 'x86_64'): closure_compiler_native = 'google-closure-compiler-linux' From c5f4628c6b766017f7b12bef24bdbe461b52d9d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Fri, 4 Mar 2022 15:35:49 +0200 Subject: [PATCH 3/6] Improve google-closure-compiler-java uninstall. --- emsdk.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/emsdk.py b/emsdk.py index 8d70c7b9f3..7adf2ac5e2 100644 --- a/emsdk.py +++ b/emsdk.py @@ -1439,16 +1439,22 @@ def emscripten_npm_install(tool, directory): cwd=directory, stderr=subprocess.STDOUT, env=env, universal_newlines=True) - # Now that we succeeded to install the native version, delete the Java version - # since that takes up ~12.5MB of installation space that is no longer needed - # as we have a native version of the compiler instead. - # This process is currently a little bit hacky, see https://github.com/google/closure-compiler/issues/3926 - remove_tree(os.path.join(directory, 'node_modules', 'google-closure-compiler-java')) - # Remove import of Java Closure Compiler module. compiler_filename = os.path.join(directory, 'node_modules', 'google-closure-compiler', 'lib', 'node', 'closure-compiler.js') - compiler_js = open(compiler_filename, 'r').read().replace("require('google-closure-compiler-java')", "''/*require('google-closure-compiler-java') XXX Removed by Emsdk*/") - open(compiler_filename, 'w').write(compiler_js) + if os.path.isfile(compiler_filename): + old_js = open(compiler_filename, 'r').read() + new_js = old_js.replace("require('google-closure-compiler-java')", "''/*require('google-closure-compiler-java') XXX Removed by Emsdk*/") + if old_js == new_js: + raise Exception('Failed to patch google-closure-compiler-java dependency away!') + open(compiler_filename, 'w').write(new_js) + + # Now that we succeeded to install the native version and patch away the Java dependency, delete the Java version + # since that takes up ~12.5MB of installation space that is no longer needed. + # This process is currently a little bit hacky, see https://github.com/google/closure-compiler/issues/3926 + remove_tree(os.path.join(directory, 'node_modules', 'google-closure-compiler-java')) + print('Removed google-closure-compiler-java dependency.') + else: + errlog('Failed to patch away google-closure-compiler Java dependency. ' + compiler_filename + ' does not exist.') except subprocess.CalledProcessError as e: errlog('Error running %s:\n%s' % (e.cmd, e.output)) return False From 09cc9cb97397c961b71d185b81f75b078b12fdc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Wed, 13 Apr 2022 11:00:49 +0300 Subject: [PATCH 4/6] Read Closure version from Emscripten repository --- emsdk.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/emsdk.py b/emsdk.py index 7adf2ac5e2..0c27d8a188 100644 --- a/emsdk.py +++ b/emsdk.py @@ -1402,6 +1402,8 @@ def emscripten_npm_install(tool, directory): env = os.environ.copy() env["PATH"] = node_path + os.pathsep + env["PATH"] print('Running post-install step: npm ci ...') + # Do a --no-optional install to avoid bloating disk size: + # https://github.com/emscripten-core/emscripten/issues/12406 try: subprocess.check_output( [npm, 'ci', '--production', '--no-optional'], @@ -1412,7 +1414,7 @@ def emscripten_npm_install(tool, directory): return False # Manually install the appropriate native Closure Compiler package - # This is currently needed because npm ci will install the packages + # This is currently needed because npm ci would install the packages # for Closure for all platforms, adding 180MB to the download size # There are two problems here: # 1. npm ci does not consider the platform of optional dependencies @@ -1431,7 +1433,10 @@ def emscripten_npm_install(tool, directory): closure_compiler_native = 'google-closure-compiler-windows' if closure_compiler_native: - closure_compiler_native += '@20220104.0.0' + # Check which version of native Closure Compiler we want to install via npm. + # (npm install command has this requirement that we must explicitly tell the pinned version) + closure_version = json.load(open(os.path.join(directory, 'package.json')))['dependencies']['google-closure-compiler'] + closure_compiler_native += '@' + closure_version print('Running post-install step: npm install', closure_compiler_native) try: subprocess.check_output( @@ -1439,7 +1444,8 @@ def emscripten_npm_install(tool, directory): cwd=directory, stderr=subprocess.STDOUT, env=env, universal_newlines=True) - # Remove import of Java Closure Compiler module. + # Installation of native Closure compiler was successful, so remove import of Java Closure Compiler module to avoid + # a Java dependency. compiler_filename = os.path.join(directory, 'node_modules', 'google-closure-compiler', 'lib', 'node', 'closure-compiler.js') if os.path.isfile(compiler_filename): old_js = open(compiler_filename, 'r').read() From 1d861adda2113249f6aa85b72ed1ef3e19fce6d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Wed, 13 Apr 2022 14:15:14 +0300 Subject: [PATCH 5/6] Skip native google-closure-compiler install when it is not present in the emscripten branch in package.json --- emsdk.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/emsdk.py b/emsdk.py index 0c27d8a188..3a5702054a 100644 --- a/emsdk.py +++ b/emsdk.py @@ -1435,7 +1435,14 @@ def emscripten_npm_install(tool, directory): if closure_compiler_native: # Check which version of native Closure Compiler we want to install via npm. # (npm install command has this requirement that we must explicitly tell the pinned version) - closure_version = json.load(open(os.path.join(directory, 'package.json')))['dependencies']['google-closure-compiler'] + try: + closure_version = json.load(open(os.path.join(directory, 'package.json')))['dependencies']['google-closure-compiler'] + except KeyError as e: + # The target version of Emscripten does not (did not) have a package.json that would contain google-closure-compiler. (fastcomp) + # Skip manual native google-closure-compiler installation there. + print('Emscripten version does not have a npm package.json with google-closure-compiler dependency, skipping native google-closure-compiler install step') + return True + closure_compiler_native += '@' + closure_version print('Running post-install step: npm install', closure_compiler_native) try: From b01f8ed3eb5b6b7c51d0eed43968c6a1cfd8ef9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jukka=20Jyl=C3=A4nki?= Date: Wed, 13 Apr 2022 14:18:30 +0300 Subject: [PATCH 6/6] Print error --- emsdk.py | 1 + 1 file changed, 1 insertion(+) diff --git a/emsdk.py b/emsdk.py index 3a5702054a..e83b2c80bb 100644 --- a/emsdk.py +++ b/emsdk.py @@ -1440,6 +1440,7 @@ def emscripten_npm_install(tool, directory): except KeyError as e: # The target version of Emscripten does not (did not) have a package.json that would contain google-closure-compiler. (fastcomp) # Skip manual native google-closure-compiler installation there. + print(str(e)) print('Emscripten version does not have a npm package.json with google-closure-compiler dependency, skipping native google-closure-compiler install step') return True