emscripten: Use the latest emsdk 3.1.68 by workingjubilee · Pull Request #131533 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation13 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@workingjubilee

This should fix our precompiled std being unsound in std::fs code.

Should resolve #131467 I hope.

@workingjubilee

This should fix our precompiled std being unsound in std::fs code.

@rustbot

r? @Kobzol

rustbot has assigned @Kobzol.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite

Area: The testsuite used to check the correctness of rustc

S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-infra

Relevant to the infrastructure team, which will review and decide on the PR/issue.

labels

Oct 11, 2024

@Kobzol

I have to say that I'm often clueless when reviewing these kinds of PRs :) Do we have some documentation about what does it mean to update this? Do we even document somewhere which version of emscripten we use for given targets, or is it just an "internal implementation detail".

@workingjubilee

I have to say that I'm often clueless when reviewing these kinds of PRs :) Do we have some documentation about what does it mean to update this? Do we even document somewhere which version of emscripten we use for given targets, or is it just an "internal implementation detail".

We do not, but we should, probably. I can add the relevant details for our platform support pages?

I think since we build the SDK, it shouldn't have a significant consequence? (aside from the bugfix, of course)

I am not entirely sure what the consequences of this are, tbh. I'm also not entirely sure this won't break Pyodide for @juntyr, either, given 3.1.68 is not 3.1.46, but I'm not sure what to make of emsdk's versioning. It could be SemVer, in which case this should be fine, but then why did 3.1.42 break ABI?

Perhaps @alexcrichton or @Manishearth might know?

@juntyr

Updating the Emscripten version to something newer is definitely a good idea. But it should go hand-in-hand with some documentation (I'll submit a PR later) on this coupling between Emscripten and Rust that won't matter in most cases until you get a weird bug, and how to work around it with -Zbuild-std.

I think I was experiencing this issue for a long while but simply never noticed. Pyodide can use ~2GB RAM before anything breaks, and up until a few days ago even the senseless overallocation during file reading never hit the boundary. So it was by pure chance and additional up-front memory pressure that allocating an extra 1.8GB was no longer fine and I noticed the ABI mismatch.

kleisauke added a commit to kleisauke/libc that referenced this pull request

Oct 11, 2024

@kleisauke

@hoodmane

Thanks for helping to support the Emscripten target @workingjubilee! I really appreciate it.

@alexcrichton

Alas I'm ignorant in the ways of Emscripten here so I also don't understand the full consequences of what an update here might have. In the absence of more information it seems reasonable to go ahead with the update and use it as a learning process for what to and what not to do in the future.

@workingjubilee

atm I don't think anyone understands how Rust interacts with Emscripten, tbh.

@sbc100 if you could weigh in?

@sbc100

I also don't know much about how emscripten and rust work together, but updating certainly seems like a good idea. 2.0.5 is ancient.

I assume there is some amount of CI testing that occurs here to make sure at least some things work?

@workingjubilee

I also don't know much about how emscripten and rust work together, but updating certainly seems like a good idea. 2.0.5 is ancient.

I assume there is some amount of CI testing that occurs here to make sure at least some things work?

...we make sure things build! :D

@workingjubilee

Only a tier 1 target is actually expected to run tests, and they run all the tests, so tier 2 targets like emscripten Basically Don't. There's a few which run anyways, but we do not extensively test tier 2 non-host targets. In fact there were quite a few ignores in the test suite just to make sure that no one even tries to run them for emscripten. Some of that was unavoidable because e.g. emscripten doesn't have enough IO support and some tests are of IO-using programs.

But uh, I am the very person who ripped out asmjs-unknown-emscripten support because it had been silently broken for years, as the only vestiges of CI testing that we have for tier 2 had been completely disabled for it, and no one noticed or complained.

@Kobzol

It seems like merging this and seeing if it breaks anyone externally might be the only reasonable way forward (assuming that it passes CI). You can r=me in that regard.

@workingjubilee workingjubilee added the O-emscripten

Target: 50% off wasm32-unknown-musl. the savings come out of stdio.h, but hey, you get SDL!

label

Oct 13, 2024

@workingjubilee

Yeah, that was my feeling as well.

It would be nice to land more tests for emscripten but even for the tests that we do run, it already has a custom test harness in compiletest. More tests may thus require wrangling more Fun, and such tests will not necessarily be checked in CI. Indeed, a large portion of the UI test suite seems to fall prey to linking failures when I try to build it, for reasons that don't have any clear relationship with the EMSDK version, and probably have been true for quite some time.

As this is a soundness concern, it doesn't seem worth blocking on a task which will take an indefinite amount of time. Thus, let us give scream-based deployment testing a go.

@bors r=Kobzol

@bors

📌 Commit 2c38ecf has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Oct 13, 2024

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Oct 13, 2024

@matthiaskrgr

…ersion, r=Kobzol

emscripten: Use the latest emsdk 3.1.68

This should fix our precompiled std being unsound in std::fs code.

Should resolve rust-lang#131467 I hope.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Oct 14, 2024

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Oct 14, 2024

@bors

…iaskrgr

Rollup of 8 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Oct 14, 2024

@bors

…iaskrgr

Rollup of 8 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Oct 14, 2024

@bors

…iaskrgr

Rollup of 8 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Oct 14, 2024

@rust-timer

Rollup merge of rust-lang#131533 - workingjubilee:update-emscripten-version, r=Kobzol

emscripten: Use the latest emsdk 3.1.68

This should fix our precompiled std being unsound in std::fs code.

Should resolve rust-lang#131467 I hope.

@Urgau Urgau added the relnotes

Marks issues that should be documented in the release notes of the next release.

label

Oct 14, 2024

rgonzalezfluendo added a commit to rgonzalezfluendo/meson that referenced this pull request

Dec 31, 2024

@rgonzalezfluendo

Rust 1.84 uses the latest emsdk 3.1.68 [1], and it fixed an issue with Emscripten dynamic linking and libc [2]. After that no native-static-libs in the output if running:

rustc --target=wasm32-unknown-emscripten --crate-type staticlib --print native-static-libs - < /dev/null

[1] rust-lang/rust#131533 [2] rust-lang/libc#4002

jhpratt added a commit to jhpratt/rust that referenced this pull request

Jan 12, 2025

@jhpratt

…mulacrum

Remove emsdk version update from 1.84.0 relnotes

See [this comment](rust-lang#131467 (comment)). The reproducer in that comment does indeed show that rustup's rust-std component is still compiled with the old emscripten ABI because libc's config flag emscripten_new_stat_abi is not set.

rust-lang#131533 presumably had no effect because the wrong CI file was modified. So nothing has changed since 1.83.0. The PR author (workingjubilee) is currently on vacation. Also the issue rust-lang#131467 should be reopened.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Jan 12, 2025

@matthiaskrgr

…mulacrum

Remove emsdk version update from 1.84.0 relnotes

See [this comment](rust-lang#131467 (comment)). The reproducer in that comment does indeed show that rustup's rust-std component is still compiled with the old emscripten ABI because libc's config flag emscripten_new_stat_abi is not set.

rust-lang#131533 presumably had no effect because the wrong CI file was modified. So nothing has changed since 1.83.0. The PR author (workingjubilee) is currently on vacation. Also the issue rust-lang#131467 should be reopened.

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Jan 12, 2025

@rust-timer

Rollup merge of rust-lang#135266 - kadiwa4:no_emsdk_update, r=Mark-Simulacrum

Remove emsdk version update from 1.84.0 relnotes

See [this comment](rust-lang#131467 (comment)). The reproducer in that comment does indeed show that rustup's rust-std component is still compiled with the old emscripten ABI because libc's config flag emscripten_new_stat_abi is not set.

rust-lang#131533 presumably had no effect because the wrong CI file was modified. So nothing has changed since 1.83.0. The PR author (workingjubilee) is currently on vacation. Also the issue rust-lang#131467 should be reopened.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Feb 2, 2025

@he32

Pkgsrc changes:

Upstream changes:

Version 1.84.1 (2025-01-30)

Version 1.84.0 (2025-01-09)

Language

Compiler

Libraries

Stabilized APIs

These APIs are now stable in const contexts

Cargo

Rustdoc

Compatibility Notes

rgonzalezfluendo added a commit to rgonzalezfluendo/meson that referenced this pull request

Apr 16, 2025

@rgonzalezfluendo

Rust 1.84 uses the latest emsdk 3.1.68 [1], and it fixed an issue with Emscripten dynamic linking and libc [2]. After that no native-static-libs in the output if running:

rustc --target=wasm32-unknown-emscripten --crate-type staticlib --print native-static-libs - < /dev/null

[1] rust-lang/rust#131533 [2] rust-lang/libc#4002

rgonzalezfluendo added a commit to rgonzalezfluendo/meson that referenced this pull request

Jun 20, 2025

@rgonzalezfluendo

Rust 1.84 uses the latest emsdk 3.1.68 [1], and it fixed an issue with Emscripten dynamic linking and libc [2]. After that no native-static-libs in the output if running:

rustc --target=wasm32-unknown-emscripten --crate-type staticlib --print native-static-libs - < /dev/null

[1] rust-lang/rust#131533 [2] rust-lang/libc#4002

jpakkane pushed a commit to mesonbuild/meson that referenced this pull request

Aug 10, 2025

@rgonzalezfluendo @jpakkane

Rust 1.84 uses the latest emsdk 3.1.68 [1], and it fixed an issue with Emscripten dynamic linking and libc [2]. After that no native-static-libs in the output if running:

rustc --target=wasm32-unknown-emscripten --crate-type staticlib --print native-static-libs - < /dev/null

[1] rust-lang/rust#131533 [2] rust-lang/libc#4002

Labels

A-testsuite

Area: The testsuite used to check the correctness of rustc

O-emscripten

Target: 50% off wasm32-unknown-musl. the savings come out of stdio.h, but hey, you get SDL!

relnotes

Marks issues that should be documented in the release notes of the next release.

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-infra

Relevant to the infrastructure team, which will review and decide on the PR/issue.