Skip to content

Conversation

@juj
Copy link
Collaborator

@juj juj commented Apr 29, 2021

Remove explicit closure install step, the first install command does already seem to install google-closure-compiler-windows at least.

See #802 , #793, #636.

This would help from Linux and macOS testing to see if that works for people. CC also @curiousdannii

…already seem to install google-closure-compiler-windows at least.
@sbc100
Copy link
Collaborator

sbc100 commented Apr 29, 2021

I this works that is awesome!

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm assuming its been verified to work

@sbc100 sbc100 merged commit 619a289 into master May 5, 2021
@sbc100 sbc100 deleted the remove_explicit_closure_install branch May 5, 2021 01:02
sbc100 pushed a commit that referenced this pull request May 5, 2021
…already seem to install google-closure-compiler-windows at least. (#803)
radekdoulik referenced this pull request in dotnet/emsdk May 20, 2021
…already seem to install google-closure-compiler-windows at least. (#803)
@sbc100
Copy link
Collaborator

sbc100 commented Aug 20, 2021

This doesn't seem to work on linux.

$ npm list
/usr/local/google/home/sbc/dev/wasm/emscripten
├── [email protected]
├─┬ [email protected]
│ ├─┬ [email protected]
│ │ ├─┬ [email protected]
│ │ │ └─┬ [email protected]
│ │ │   └── [email protected]
│ │ ├── [email protected]
│ │ └─┬ [email protected]
│ │   └── [email protected]
│ ├── [email protected]
│ ├── UNMET OPTIONAL DEPENDENCY [email protected]
│ ├── UNMET OPTIONAL DEPENDENCY [email protected]
│ ├── UNMET OPTIONAL DEPENDENCY [email protected]
│ ├── [email protected]
│ ├─┬ [email protected]

juj added a commit that referenced this pull request Mar 4, 2022
… 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.
juj added a commit that referenced this pull request Apr 13, 2022
… 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.
juj added a commit that referenced this pull request Apr 13, 2022
* 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.

* Add note to bug

* Improve google-closure-compiler-java uninstall.

* Read Closure version from Emscripten repository

* Skip native google-closure-compiler install when it is not present in the emscripten branch in package.json

* Print error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants