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.

Sign up for GitHub

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

Jump to bottom

Add wasm32-unknown-emscripten platform support document #131582

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Add wasm32-unknown-emscripten platform support document #131582

merged 1 commit into from
Oct 16, 2024

Conversation

juntyr
Copy link
Contributor

@juntyr juntyr commented Oct 12, 2024 •
edited
Loading

This PR adds the platform support document for wasm32-unknown-emscripten, and adds a warning about breaks in Emscripten ABI compatibility (see #131467).

I mostly based the document off the wasm32-unknown-unknown docs and some of the information may still be missing (e.g. who's the target maintainer) or outdated (e.g. the build requirements). I still hope that it provides a good starting point.

r? @workingjubilee

@rustbot
Copy link
Collaborator

rustbot commented Oct 12, 2024

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 12, 2024

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@tgross35
Copy link
Contributor

e.g. who's the target maintainer

I tried to figure this out before and it seems like we don't really have one. It doesn't seem to have been much of a problem for this target (presumably overlap with the other wasm targets helps) but we should definitely try to find somebody willing to put their name here. Of course this doesn't block improving the docs, this is needed regardless.

@hoodmane and @tlively you seem to have done a lot of the work for this target, any interest in being added as a target maintainer? The bar is very low, it mostly means you are willing to be pinged when anyone has emscripten-specific questions.

@hoodmane
Copy link
Contributor

I would like to be pinged with emscripten related questions. I would also suggest @sbc100 should be kept in the loop.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@juntyr
Copy link
Contributor Author

juntyr commented Oct 13, 2024

I'm unfortunately not sure which links are broken, at least on GitHub all new links on my branch work

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@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
This was referenced Oct 14, 2024
Closed
Closed
@juntyr
Copy link
Contributor Author

juntyr commented Oct 15, 2024

Not sure why linkcheck is failing though

I have no idea either. Is there any way to get the HTML files from CI to see which links are broken or do I need to investigate in a local build?

@juntyr
Copy link
Contributor Author

juntyr commented Oct 15, 2024

I would like to be pinged with emscripten related questions. I would also suggest @sbc100 should be kept in the loop.

If you want, you can setup a wasm/emscripten notification group and ask interested parties to join, so people can ping the group via @rustbot ping emscripten or wasm. Target-specific expertise is very welcomed if target experts can occasionally help to answer questions and such.

I’d also be interested in being pinged as well (for both wasm in general and emscripten). Should this be two groups or one with an alias (if that’s possible)? The docs show how to join one, how can a new group be created?

@jieyouxu
Copy link
Member

I’d also be interested in being pinged as well (for both wasm in general and emscripten). Should this be two groups or one with an alias (if that’s possible)? The docs show how to join one, how can a new group be created?

The forge docs has steps for this: https://forge.rust-lang.org/compiler/notification-groups.html. It presently says it will need an MCP to create the ping groups (... and if you file one I'll just immediately second it anyway), but I'll ask the T-compiler leads if this is still current policy (i.e. if the MCP is needed).

@jieyouxu
Copy link
Member

jieyouxu commented Oct 15, 2024 •
edited
Loading

Just to be explicit, @sbc100, can you confirm that you are willing to be added as a target maintainer for the wasm32-unknown-emscripten target?

@jieyouxu
Copy link
Member

jieyouxu commented Oct 15, 2024 •
edited
Loading

@juntyr I think having a general-purpose wasm group and also an emscripten-specific ping group separately is a good idea. You'll need to create two marker teams over at the team repo. See https://forge.rust-lang.org/compiler/notification-groups.html#notification-groups for the steps. Previous example PR: rust-lang/team#670.

The forge docs says a MCP is required, but I discussed the ping group matter with @davidtwco, one of our T-compiler co-leads, and we determined that a MCP is not required for this situation, so you can go ahead with the team PRs without needing a MCP.

@jieyouxu
Copy link
Member

I'm looking at the linkcheck failures to see if I can repro them locally.

@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Oct 15, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jieyouxu
Copy link
Member

@juntyr linkchecker is failing because we need to declare the page exists in SUMMARY.md, lol.

@juntyr juntyr mentioned this pull request Oct 15, 2024
Merged
@juntyr
Copy link
Contributor Author

juntyr commented Oct 15, 2024

@juntyr linkchecker is failing because we need to declare the page exists in SUMMARY.md, lol.

Thanks @jieyouxu, that fixed it!

@jieyouxu
Copy link
Member

jieyouxu commented Oct 16, 2024 •
edited
Loading

@juntyr thanks, this looks mostly ready to merge. Can you please squash the commits into one? Also, do you have contact with @sbc100, because I would like explicit confirmation from @sbc100 that they are happy to be listed as a target maintainer. If you don't want to wait for their confirmation, then you can also drop them from maintainer list for now, and ask them to open a PR to add themselves in a follow-up PR.

@jieyouxu jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2024
@juntyr @hoodmane
@alexcrichton @kleisauke
Add wasm32-unknown-emscripten platform support document
0804d1b 
Co-authored-by: Hood Chatham <[email protected]>
Co-authored-by: Alex Crichton <[email protected]>
Co-authored-by: Kleis Auke Wolthuizen <[email protected]>
@juntyr juntyr force-pushed the emscripten-platform-support branch from fef9b29 to 0804d1b Compare October 16, 2024 05:27
@juntyr
Copy link
Contributor Author

juntyr commented Oct 16, 2024

@juntyr thanks, this looks mostly ready to merge. Can you please squash the commits into one? Also, do you have contact with @sbc100, because I would like explicit confirmation from @sbc100 that they are happy to be listed as a target maintainer. If you don't want to wait for their confirmation, then you can also drop them from maintainer list for now, and ask them to open a PR to add themselves in a follow-up PR.

@jieyouxu I've squashed the commits.

I had hoped we would get an answer from @sbc100, but since that's not the case I've removed @sbc100 from the maintainer list for the wasm-unknown-emscripten Rust target for now, which can always be easily changed in a future PR.

@jieyouxu
Copy link
Member

Thanks.
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 16, 2024

📌 Commit 0804d1b has been approved by jieyouxu

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 16, 2024
@Urgau Urgau mentioned this pull request Oct 16, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 16, 2024
@bors
Auto merge of rust-lang#131775 - Urgau:rollup-yc4a3sf, r=Urgau
0037048 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#131582 (Add wasm32-unknown-emscripten platform support document)
 - rust-lang#131694 (Make fuchsia-test-runner.py compatible with new JSON output from llvm-readelf)
 - rust-lang#131700 (Fix match_same_arms in stable_mir)
 - rust-lang#131712 (Mark the unstable LazyCell::into_inner const)
 - rust-lang#131746 (Relax a memory order in `once_box`)
 - rust-lang#131754 (Don't report bivariance error when nesting a struct with field errors into another struct)
 - rust-lang#131760 (llvm: Match aarch64 data layout to new LLVM layout)
 - rust-lang#131764 (Fix unnecessary nesting in run-make test output directories)
 - rust-lang#131766 (Add mailmap entry for my dev-desktop setup)
 - rust-lang#131771 (Handle gracefully true/false in `cfg(target(..))` compact)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f7b91ca into rust-lang:master Oct 16, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 16, 2024
@rust-timer
Unrolled build for rust-lang#131582
2bcdcc3 
Rollup merge of rust-lang#131582 - juntyr:emscripten-platform-support, r=jieyouxu

Add wasm32-unknown-emscripten platform support document

This PR adds the platform support document for wasm32-unknown-emscripten, and adds a warning about breaks in Emscripten ABI compatibility (see rust-lang#131467).

I mostly based the document off the wasm32-unknown-unknown docs and some of the information may still be missing (e.g. who's the target maintainer) or outdated (e.g. the build requirements). I still hope that it provides a good starting point.

r? `@workingjubilee`
@juntyr juntyr deleted the emscripten-platform-support branch October 18, 2024 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgross35 tgross35 tgross35 left review comments

@kleisauke kleisauke kleisauke left review comments

@hoodmane hoodmane hoodmane approved these changes

@jieyouxu jieyouxu jieyouxu approved these changes

@alexcrichton alexcrichton alexcrichton approved these changes

Assignees

@jieyouxu jieyouxu

Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools O-emscripten Target: 50% off wasm32-unknown-musl. the savings come out of stdio.h, but hey, you get SDL! S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.84.0
Development

Successfully merging this pull request may close these issues.