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

HTML5: Fix minification error with Emscripten 1.39.9 #52926

Merged
merged 1 commit into from
Sep 29, 2021

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Sep 22, 2021

It used an old vendored version of acorn.js which seems to choke on this
trailing comma. This is not a problem for more recent Emscripten versions.

The error started after #52650 which now makes our AudioWorklet library build on non-threaded builds too (so Mono too with Emscripten 1.39.9). Not strictly related in 3.3 which didn't get this change backported, but might be worth cherry-picking anyway for consistency.

The error was:

em++ -o bin/godot.javascript.opt.debug.mono.js -Os --profiling-funcs -s INITIAL_MEMORY=32MB -s ENVIRONMENT=web,worker -s MODULARIZE=1 -s EXPORT_NAME='Godot' -s ALLOW_MEMORY_GROWTH=1 -s USE_WEBGL2=1 -s INVOKE_RU>
cache:INFO: generating system library: libc++-noexcept.a... (this will be cached in "/root/emsdk_1.39.9/upstream/emscripten/cache/wasm/libc++-noexcept.a" for subsequent builds)
cache:INFO:  - ok
cache:INFO: generating system library: libc++abi-noexcept.a... (this will be cached in "/root/emsdk_1.39.9/upstream/emscripten/cache/wasm/libc++abi-noexcept.a" for subsequent builds)
cache:INFO:  - ok
cache:INFO: generating system library: libgl-webgl2-ofb.a... (this will be cached in "/root/emsdk_1.39.9/upstream/emscripten/cache/wasm/libgl-webgl2-ofb.a" for subsequent builds)
cache:INFO:  - ok
/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:2780
  throw err
  ^

SyntaxError: Unexpected token (12835:6)
                                );
      ^

    at Parser.pp$4.raise (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:2778:13)
    at Parser.pp.unexpected (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:680:8)
    at Parser.pp$3.parseExprAtom (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:2230:10)
    at Parser.pp$3.parseExprSubscripts (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:2075:19)
    at Parser.pp$3.parseMaybeUnary (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:2052:17)
    at Parser.pp$3.parseExprOps (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:1994:19)
    at Parser.pp$3.parseMaybeConditional (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:1977:19)
    at Parser.pp$3.parseMaybeAssign (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:1952:19)
    at Parser.pp$3.parseExprList (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:2676:20)
    at Parser.pp$3.parseNew (/root/emsdk_1.39.9/upstream/emscripten/tools/node_modules/acorn/dist/acorn.js:2343:55) {
  pos: 502055,
  loc: Position { line: 12835, column: 6 },
  raisedAt: 502056
}
shared:ERROR: '/root/emsdk_1.39.9/node/14.15.5_64bit/bin/node /root/emsdk_1.39.9/upstream/emscripten/tools/acorn-optimizer.js /tmp/emscripten_temp_j7fg3q8m/godot.javascript.opt.debug.mono.wasm.o.js AJSDCE minifyWhitespace' failed (1)

Which is super unhelpful, but after patching acorn.js I got:

SyntaxError: Unexpected token (12835:6)
                                        'godot-processor',
                                        {
                                                'outputChannelCount': [channels],
                                        },
                                );
                                ^

                                return Promise.resolve();
                        });
                        GodotAudio.driver = GodotAudioWorklet;
                },start:function (in_buf, out_buf, state) {

This was likely relaxed in later releases of acorn.js, but Emscripten 1.39.9 bundles it own patched version (it was unbundled with emscripten-core/emscripten#11439 in 1.39.19).

@akien-mga akien-mga requested a review from a team as a code owner September 22, 2021 07:43
@akien-mga akien-mga added bug cherrypick:3.3 cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Sep 22, 2021
@akien-mga akien-mga added this to the 4.0 milestone Sep 22, 2021
@akien-mga
Copy link
Member Author

Welp it makes the linter unhappy :(

/home/runner/work/godot/godot/platform/javascript/js/libs/library_godot_audio.js
  232:7  error  Missing trailing comma  comma-dangle

@Faless
Copy link
Collaborator

Faless commented Sep 22, 2021

Needs this patch:

diff --git a/platform/javascript/.eslintrc.js b/platform/javascript/.eslintrc.js
index 0ff9d67d26..2c81f1f02d 100644
--- a/platform/javascript/.eslintrc.js
+++ b/platform/javascript/.eslintrc.js
@@ -39,5 +39,13 @@ module.exports = {
 		// Closure compiler (exported properties)
 		"quote-props": ["error", "consistent"],
 		"dot-notation": "off",
+		// No comma dangle for functions (it's madness, and ES2017)
+		"comma-dangle": ["error", {
+			"arrays": "always-multiline",
+			"objects": "always-multiline",
+			"imports": "always-multiline",
+			"exports": "always-multiline",
+			"functions": "never"
+		}],
 	}
 };

Which should also prevent errors like this in the future.

It used an old vendored version of acorn.js which seems to choke on this
trailing comma. This is not a problem for more recent Emscripten versions.

We disable the `comma-dangle` check in ESLint to prevent this issue.
@akien-mga akien-mga force-pushed the js-acorn-emscripten-1.39.9 branch from 468e221 to 23b51a1 Compare September 29, 2021 07:11
@akien-mga akien-mga merged commit b1237b5 into godotengine:master Sep 29, 2021
@akien-mga akien-mga deleted the js-acorn-emscripten-1.39.9 branch September 29, 2021 07:40
@akien-mga
Copy link
Member Author

Cherry-picked for 3.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 29, 2021
@akien-mga
Copy link
Member Author

Cherry-picked for 3.3.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants