Skip to content

[deps] Update google-closure-compiler to 20260429.0.0#26869

Merged
sbc100 merged 4 commits into
emscripten-core:mainfrom
garymathews:closure-20260429.0.0
May 11, 2026
Merged

[deps] Update google-closure-compiler to 20260429.0.0#26869
sbc100 merged 4 commits into
emscripten-core:mainfrom
garymathews:closure-20260429.0.0

Conversation

@garymathews
Copy link
Copy Markdown
Contributor

@garymathews garymathews commented May 6, 2026

@garymathews garymathews changed the title [deps] Update closure to 20260429.0.0 [deps] Update google-closure-compiler to 20260429.0.0 May 6, 2026
@garymathews garymathews force-pushed the closure-20260429.0.0 branch from a8dfaeb to 20cc03a Compare May 6, 2026 01:31
@sbc100
Copy link
Copy Markdown
Collaborator

sbc100 commented May 6, 2026

I guess this is kind of a dup of #26015 and #24679.

I think the block with #24679 was that we saw a small regression that we were not sure we wanted to accept with the upgrade.

@garymathews garymathews marked this pull request as ready for review May 6, 2026 04:24
@garymathews garymathews force-pushed the closure-20260429.0.0 branch from 20cc03a to 980d4bc Compare May 6, 2026 18:54
Comment thread tools/ports/emdawnwebgpu.py
Comment thread src/lib/libatomic.js Outdated
Comment thread src/lib/libatomic.js
emscripten_atomic_wait_async__deps: ['$atomicWaitStates', '$liveAtomicWaitAsyncs', '$liveAtomicWaitAsyncCounter', '$polyfillWaitAsync', '$callUserCallback'],
// Closure's Atomics.waitAsync extern incorrectly returns Promise<string>,
// but the spec returns a result object with async/value fields.
emscripten_atomic_wait_async__docs: '/** @suppress {missingProperties} */',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we instead re-declare it with the correct signature somewhere in src/closure-externs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, updating the waitAsync signature in closure-externs.js does not override the Closure compilers built in waitAsync signature. This seems to be the only way until the signature in Closure is fixed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've opened a PR for the Closure compiler google/closure-compiler#4317

"$pthread_mutexattr_setpshared",
"$pthread_mutexattr_settype",
"$pthread_rwlockattr_init",
"$pthread_rwlockattr_setpshared",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this change perhaps means that you are not running against the tot version of emsdk when you are doing the rebase? Did you do emsdk install tot?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just re-ran against tot, only test_codesize_cxx_lto.json was updated

Copy link
Copy Markdown
Contributor Author

@garymathews garymathews May 8, 2026

Choose a reason for hiding this comment

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

@sbc100 it seems the current codesize results are not up to date in main. If you --rebaseline on main you'll see a few files are updated including test_codesize_hello_dylink_all.json with this change.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

main should be up-to-date, but this stuff is little sensitive and in constant flux.

Try ./emcc --clear-cache && ./test/runner codesize --rebase after emsdk install tot.

You should see no changes if your run those commands on main, unless it really is out-of-sync, which is unusual.

When main is out-of-sync we run the "Rebaseline tests" github action to create commits like #26884

@garymathews garymathews force-pushed the closure-20260429.0.0 branch 5 times, most recently from d4f3430 to de56a93 Compare May 8, 2026 18:43
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

LGTM!

Sorry for all the codesize churn. I know its a pain to constantly rebase, but I think its worth it in the long run.

Should we perhaps add a ChangeLog entry for this? (Mostly to mention that macOS arm64 users will now get a native closure binaryen).

garymathews added a commit to garymathews/emscripten that referenced this pull request May 10, 2026
@garymathews garymathews force-pushed the closure-20260429.0.0 branch from de56a93 to 7cf9894 Compare May 10, 2026 18:58
garymathews added a commit to garymathews/emscripten that referenced this pull request May 10, 2026
@garymathews garymathews force-pushed the closure-20260429.0.0 branch from 7cf9894 to 7a64e56 Compare May 10, 2026 19:05
@garymathews garymathews force-pushed the closure-20260429.0.0 branch from 7a64e56 to d1c252a Compare May 11, 2026 00:25
@garymathews
Copy link
Copy Markdown
Contributor Author

garymathews commented May 11, 2026

@sbc100 Updated and also removed some redundant externs from closure-externs.js since they are now provided by Closure

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Do you know why the package-lock.json changes so much from this single upgrade? I'm surprised because I didn't think closure-compiler had any/many dependencies the JS ecosystem.

{
"a.out.js": 26251,
"a.out.js.gz": 11195,
"a.out.js": 26452,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is an example of the rather unfortunate regression. Its kind of a shame that we upgrade the compiler and the compression (its only job) gets worse. But I guess this is still a very small percentage even though its 200 bytes.

@sbc100 sbc100 merged commit 5667bf9 into emscripten-core:main May 11, 2026
30 checks passed
@sbc100
Copy link
Copy Markdown
Collaborator

sbc100 commented May 11, 2026

Do you know why the package-lock.json changes so much from this single upgrade? I'm surprised because I didn't think closure-compiler had any/many dependencies the JS ecosystem.

I verified locally and I get the same large change when I did npm install locally.

sbc100 pushed a commit that referenced this pull request May 23, 2026
- Continuation of #26869
- google/closure-compiler#4317 was merged into `20260519.0.0` which
fixes the `Atomics.waitAsync` signature
  - The `@suppress` annotations are no longer necessary
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.

2 participants