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

Update Producers Section to match #93 #100

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

lukewagner
Copy link
Member

@lukewagner lukewagner commented Feb 21, 2019

As discussed in #93/comment and the recent CG meeting.

Copy link

@jgravelle-google jgravelle-google left a comment

Choose a reason for hiding this comment

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

Most of the deleted content I think is still useful at capturing intent for toolchains who do find this section useful. In particular, size-optimizing toolchains (like Emscripten) are likely to strip the section, but less size-conscious toolchains should still feel encouraged to support it.

@alexcrichton
Copy link
Collaborator

alexcrichton commented Feb 22, 2019

I personally feel like the phrasing of "size-optimizing toolchains" isn't quite right here because the Rust toolchain, for example, is (as always) size optimizing yet we intend to continue to emit the producers section by default.

There are other examples of "useless in production" sections being emitted by default by toolchains such as the name section and dwarf debugging information. These sections (and the producers section) are all trivially stripped at any point in the build process, and we're always going to ensure there's documentation for standard tools (like wasm-strip in wabt) to strip these sections as well as CLI flags for our own tooling to strip the sections (like --keep-debug, --remove-producers-section, and --remove-name-section).

It seems like it'd be unfortunate to get into a situation where everyone's code-golfing how small their wasm can be to the point where very useful information is being stripped for a proportionally larger loss of functionality.

ProducersSection.md Outdated Show resolved Hide resolved
@kripken
Copy link
Member

kripken commented Feb 22, 2019

I feel like the size comment had value. I think it depends on which level of the tools we focus on. That is, I can see how Rust may not want to make any decision at all about the producers section, and just leave it to later tools. For a complete toolchain though, like say emcc, which does not assume any tool will run after it, it does make sense to strip the producers section by default for size (and information leakage) reasons.

@lukewagner
Copy link
Member Author

Happy to let someone else take over this PR and put the wording just right from a producer's perspective.

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.

4 participants