Remove JAVA config setting #15055
Closed
Remove JAVA config setting #15055
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A couple of reasons for this simplification. 1. We don't use java for anything anymore in the normal run of things. For closure-compiler we run the native version and only fall back to the Java version when there is no native version that the current architecture (i.e. on non-x86 platforms). 2. In the case we do want to run the java version it makes sense to simpy expect to be installed in the PATH.
sbc100
added a commit
to emscripten-core/emsdk
that referenced
this pull request
Sep 16, 2021
Its possible we were not add this to directory to PATH to avoid message with the user's already installed JAVA. However this seems inconsistent given: 1. We do set JAVA_HOME, which is already interfering with any java installation the user might have. 2. We do add node to that PATH, overriding any node installation the user might have. This change makes removal of JAVA config setting from emscripten feasible since it doesn't have to handle this strange half-configured java installation: emscripten-core/emscripten#15055
sbc100
added a commit
to emscripten-core/emsdk
that referenced
this pull request
Sep 16, 2021
Its possible we were not adding this to directory to `PATH` to avoid message with the user's already installed java. However this seems inconsistent given: 1. We do set `JAVA_HOME`, which is already interfering with any java installation the user might have. 2. We do add node to that PATH, overriding any node installation the user might have. This change makes removal of `JAVA` config setting from emscripten feasible since it doesn't have to handle this strange half-configured java installation: emscripten-core/emscripten#15055 As a followup we could consider not install java at all anymore since it doesn't have any usage in modern emscripten (at least not on windows where the native version of closure-compiler should always be available).
|
Closing in favor of #15056 for now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A couple of reasons for this simplification.
For closure-compiler we run the native version and only fall back
to the Java version when there is no native version that the current
architecture (i.e. on non-x86 platforms).
simply expect it to be installed in the PATH.