Skip installing optional npm deps, but manually install Closure Compiler native package #630
Conversation
…opriate Closure Compiler native package Follow on to emscripten-core#625 Fixes emscripten-core/emscripten#12406
|
Having this much logic in the post-install phase of emsdk makes me a little sad. But at the same time it does save a significant amount of space so we need to fix it somehow. |
| cwd=directory, stderr=subprocess.STDOUT, env=env, | ||
| universal_newlines=True) | ||
| except subprocess.CalledProcessError as e: | ||
| print('Error running %s:\n%s' % (e.cmd, e.output)) | ||
| return False | ||
|
|
||
| # Manually install the appropriate native Closure Compiler package |
There was a problem hiding this comment.
Could you leave a longer comment here about what this is needed?
Can you mention the upstream closure bug that bloats the native package: google/closure-compiler-npm#186
And also mention that its only need because we use npm ci (which seems to install all optional dependencies) as opposed to npm install which does the right thing and installs just the current platform one :(
I'm hoping we might be able to revert this if they fix either or both of these issues.
There was a problem hiding this comment.
How's this look?
Add detailed comment, referring to npm/cli#558 and google/closure-compiler-npm#186
|
Unfortunately, google-closure-compiler-linux is a platform specific package, for x86: this change breaks any from-source built platform that isn't x86: emsdk build: npm ERR! A complete log of this run can be found in: Installation failed! Package Test: npm ERR! A complete log of this run can be found in: |
…opriate Closure Compiler native package Follow on to emscripten-core#625, emscripten-core#630, emscripten-core#632, emscripten-core#633 Fixes emscripten-core/emscripten#12406
Follow on to #625
Fixes emscripten-core/emscripten#12406