Opened on 03/21/2017 at 09:20:06 AM
Closed on 03/27/2017 at 08:04:43 AM
#5020 closed change (fixed)
[emscripten] Improve compile script
Reported by: | trev | Assignee: | trev |
---|---|---|---|
Priority: | P4 | Milestone: | |
Module: | Core | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #4122 | Platform: | Unknown / Cross platform |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
https://codereview.adblockplus.org/29390650/ |
Description (last modified by trev)
Background
The script currently used to compile Emscripten code was meant as a temporary solution and has a number of shortcomings that are easy to fix.
What to change
- Make the script PEP-8 compliant.
- Use os.path.join() consistently.
- Don't use current directory, resolve paths relative to script directory instead.
- Add command line option to set Emscripten path and only use ../emscripten as fallback.
- Add --debug command line flag to disable code minification.
- Add --tracing command line flag to enable memory tracing.
- Implement a Windows-compatible solution to retrieve Emscripten environment.
Attachments (0)
Change History (6)
comment:3 Changed on 03/21/2017 at 11:24:22 AM by trev
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:5 Changed on 03/27/2017 at 08:03:56 AM by abpbot
Some commits referencing this issue have landed:
- Issue 5020 - [emscripten Part 1: Make compile script PEP8-compliant]
- Issue 5020 - [emscripten Part 2: Fix usage of paths in compile script]
- Issue 5020 - [emscripten Part 3: Add command line options to compile script]
- Issue 5020 - [emscripten Part 4: Make compile script Windows-compatible]
comment:6 Changed on 03/27/2017 at 08:04:43 AM by trev
- Resolution set to fixed
- Status changed from reviewing to closed
Note: See
TracTickets for help on using
tickets.
Added last part, the one making the script compatible with Windows. It makes the process of retrieving Emscripten configuration somewhat more robust by reading .emscripten configuration file directly instead of relying on stable emsdk output. In the process I found out that Emscripten on Windows isn't exactly a well-supported solution: