Skip to content

Fix emscripten CI after recent emsdk change#11293

Merged
sbc100 merged 1 commit intomasterfrom
fix_ci_after_emsdk_change
May 29, 2020
Merged

Fix emscripten CI after recent emsdk change#11293
sbc100 merged 1 commit intomasterfrom
fix_ci_after_emsdk_change

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented May 29, 2020

See emscripten-core/emsdk#472

Turns out we were actually depending on the non-embedded
config!

@sbc100
Fix emscripten CI after recent emsdk change
5bfdad9 
See emscripten-core/emsdk#472

Turns out we were actually depending on the non-embedded
config!
@sbc100 sbc100 requested a review from kripken May 29, 2020 17:52
kripken
kripken reviewed May 29, 2020
View reviewed changes
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Ah, this worries me that more users will break then...

I assume the plan is to switch CI to embedded, but it would take more work than makes sense to do right now to unbreak? (how much work is that?)

@sbc100
Copy link
Collaborator Author

sbc100 commented May 29, 2020

I agree there could other unexpected fallout. That is why I will leave the --no-embedded option in there for a while until we can figure out all the use cases.

Our use case is pretty weird here. We use emsdk here but never call ./emsdk_env.sh which means none of the emsdk environment variables get set... and then we expect to be able to use our own version of emscripten it. But I think we can figure out way to switch to the embedded config here anyway.

@sbc100
Copy link
Collaborator Author

sbc100 commented May 29, 2020

Yes.. exactly .. unbreak first.. then look at making the switch.

@sbc100 sbc100 merged commit 119c0d1 into master May 29, 2020
@sbc100 sbc100 deleted the fix_ci_after_emsdk_change branch May 29, 2020 18:20
@madmiraal madmiraal mentioned this pull request May 30, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kripken kripken kripken approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sbc100 @kripken