Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Oct 7, 2022

Since emscripten-core/emsdk#1110 landed emsdk no longer set EM_CONFIG so we need to do that (in order to make our local version of emscripten use the config from emsdk).

@sbc100 sbc100 requested review from dschuff and kripken October 7, 2022 01:42
@sbc100 sbc100 force-pushed the em_config branch 2 times, most recently from 7c1489a to f8e26bb Compare October 7, 2022 01:50
Since emscripten-core/emsdk#1110 landed emsdk
no longer set `EM_CONFIG` so we need to do that (in order to make our
local version of emscripten use the config from emsdk).
@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 7, 2022

Landing TBR to get the CI tests passing again.

@sbc100 sbc100 merged commit 5a0adcd into main Oct 7, 2022
@sbc100 sbc100 deleted the em_config branch October 7, 2022 02:40
@dschuff
Copy link
Member

dschuff commented Oct 7, 2022

Fine for getting CI tests passing. But I thought setting EM_CONFIG shouldn't be necessary anymore when using emsdk? Is this going to affect our users?

@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 7, 2022

No, I don't think it should effect users who want to use emsdk. I this case we are trying to use emsdk with our own out-of-tree version of emscripten. It this case it seems reasonable to set EM_CONFIG to point to the emsdk config file.

@dschuff
Copy link
Member

dschuff commented Oct 7, 2022

Makes sense, thanks.

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