Use embedded configuration by default #472
Merged
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
sbc100
added a commit
to emscripten-core/emscripten
that referenced
this pull request
Apr 16, 2020
This change allows emsdk --embdded mode to work out of the box without running emsdk_env.sh. This paves makes it easier to justify using embedded mode by default: emscripten-core/emsdk#472 This is part of a larger plan to avoid using the $HOME directory to store anything at all: #9543
sbc100
force-pushed
the
embedded_by_default
branch
from
ab49230 to
10acbda
Compare
sbc100
added a commit
to emscripten-core/emscripten
that referenced
this pull request
Apr 16, 2020
This change allows emsdk --embdded mode to work out of the box without running emsdk_env.sh. This paves makes it easier to justify using embedded mode by default: emscripten-core/emsdk#472 This is part of a larger plan to avoid using the $HOME directory to store anything at all: #9543
kripken
reviewed
Apr 16, 2020
There was a problem hiding this comment.
In general this sounds good, but I worry about the risk. Where can we mention this to users? The emscripten changelog isn't quite right. Maybe just the mailing list?
Also I'd like to see this after tests pass, to make sure I understand what works and what doesn't.
sbc100
added a commit
to emscripten-core/emscripten
that referenced
this pull request
Apr 17, 2020
This change allows emsdk --embedded mode to work out of the box without running emsdk_env.sh. This paves makes it easier to justify using embedded mode by default: emscripten-core/emsdk#472 This is part of a larger plan to avoid using the $HOME directory to store anything at all: #9543
sbc100
force-pushed
the
embedded_by_default
branch
4 times, most recently
from
cbd49e7 to
f9c2786
Compare
I can't see any reason why the embedded mode isn't always preferable. The emsdk is designed to be used as-is, and per-user customizations don't make sense. I'm hoping if this sicks we can just remove this options completely. This is part of a wider plan to remove the use of the user's HOME directory completely: emscripten-core/emscripten#9543
sbc100
force-pushed
the
embedded_by_default
branch
from
f9c2786 to
5328ded
Compare
|
I'm going to write the mailing list about this change now.. and hopefully is now good to go? |
sbc100
added a commit
to emscripten-core/emscripten
that referenced
this pull request
May 29, 2020
See emscripten-core/emsdk#472 Turns out we were actually depending on the non-embedded config!
sbc100
added a commit
to emscripten-core/emscripten
that referenced
this pull request
May 29, 2020
See emscripten-core/emsdk#472 Turns out we were actually depending on the non-embedded config!
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 10, 2020
Fix emcc failure for wasm32. The wasm32 job is currently failing on CI with the error `ERROR: llc executable not found at /usr/bin/llc`. The issue is that emscripten-core/emsdk#472 has changed how emsdk discovers its configuration. We were relying on the global behavior that would use a configuration from the home directory. However, it looks like emsdk is moving away from that approach. This change adds the necessary env var for emcc to find the correct configuration. There are a few alternate approaches this could take. The `--no-embedded` option could be passed to `emsdk activate` to use the old behavior, but it seems like they want to move away from that. Another option is to source `emsdk_env.sh`, which is how these env vars normally get set. I'm not entirely sure how to do that easily in a Dockerfile, though.
sbc100
added a commit
that referenced
this pull request
Jun 24, 2020
embedded mode as been the default since #472 and I included `--no-embedded` as an option during the interim time, but to simply the code and avoid have two modes of operation I think its safe to now remove the non-embedded mode.
sbc100
added a commit
that referenced
this pull request
Jun 24, 2020
embedded mode as been the default since #472 and I included `--no-embedded` as an option during the interim time, but to simply the code and avoid have two modes of operation I think its safe to now remove the non-embedded mode.
sbc100
added a commit
that referenced
this pull request
Jul 20, 2020
embedded mode as been the default since #472 and I included `--no-embedded` as an option during the interim time, but to simply the code and avoid have two modes of operation I think its safe to now remove the non-embedded mode.
sbc100
added a commit
that referenced
this pull request
Jul 20, 2020
embedded mode as been the default since #472 and I included `--no-embedded` as an option during the interim time, but to simply the code and avoid have two modes of operation I think its safe to now remove the non-embedded mode.
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.
I can't see any reason why the embedded mode isn't always preferable.
The emsdk is designed to be used as-is, and per-user customizations
don't make sense.
I'm hoping if this sicks we can just remove this options completely.
This is part of a wider plan to remove the use of the user's HOME
directory completely:
emscripten-core/emscripten#9543