Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: [Emscripten] Pin Dockerfile to 2.0.10 to prevent patch failure #8628

Merged

Conversation

embeddedt
Copy link
Contributor

@embeddedt embeddedt commented Jan 31, 2021

Motivation / Problem

The provided emsdk patch for adding LZMA support to Emscripten is out of date and fails with the following error:

Step 3/3 : RUN cd /emsdk/upstream/emscripten && patch -p1 < /emsdk-liblzma.patch
 ---> Running in fc5f4dc7d2fc
patching file embuilder.py
Hunk #1 succeeded at 63 (offset 3 lines).
Hunk #2 FAILED at 198.
1 out of 2 hunks FAILED -- saving rejects to file embuilder.py.rej
patching file src/settings.js
Hunk #1 succeeded at 1212 (offset 15 lines).
patching file tools/ports/liblzma.py
The command '/bin/sh -c cd /emsdk/upstream/emscripten && patch -p1 < /emsdk-liblzma.patch' returned a non-zero code: 1

Description

This PR fixes that by pinning the Emscripten version to 2.0.10, similarly to what is being done for CI.

Limitations

It would be preferable to upstream LZMA support instead of relying on patching, which is IMO a fragile process. However, that is out of the scope of this PR.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

embeddedt added a commit to embeddedt/OpenTTD-modded that referenced this pull request Jan 31, 2021
@TrueBrain TrueBrain merged commit 76a279e into OpenTTD:master Jan 31, 2021
@TrueBrain
Copy link
Member

Really silly of us .. in the GitHub workflow we do pin the version .. oops :D Tnx for the Pull Request!

@embeddedt embeddedt deleted the vanilla_emscripten_dockerfile_fix branch January 31, 2021 14:52
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.

None yet

2 participants