WebAssembly / binaryen

Optimizer and compiler/toolchain library for WebAssembly
Apache License 2.0
7.4k stars 729 forks source link

adding bazel support #5902

Open mobileink opened 1 year ago

mobileink commented 1 year ago

I've begin adding bazel support. See https://github.com/obazl/binaryen. Building the libraries in src/ went pretty smoothly but I need some help with other stuff, since I'm just getting started with wasm and don't really understand how binaryen works. For example the compile/link flags in the root CMakeLists.txt file.

Any interest?

kripken commented 1 year ago

Thanks for the suggestion @mobileink !

cc @walkingeyerobot who would know best.

walkingeyerobot commented 1 year ago

Hey, thanks for taking a shot at this! I'm happy to take a look.

What are you hoping to do with binaryen after you've got it building in bazel? Consume it as a library? Execute the tools in genrules / other rules? There's a few options for how we can add bazel support depending on what you're interested in doing with it.

mobileink commented 1 year ago

What are you hoping to do with binaryen after you've got it building in bazel?

At this point I'm not sure, TBH. Just getting started with wasm. The backstory is: I'm bazelizing tree-sitter (stylites; I changed the name because I'm experimenting with some code reorg), and it has some wasm support involving emscripten. So I took a look at emscripten and found binaryen. The ultimate goal would be a hermetic build, where Bazel builds everything from sources. My general approach is to just make all functionality available as bazel targets.

Bazel module support is critical for my purposes. So I suppose a binaryen bazel module would have an extension to create a toolchain, maybe some other extension stuff - don't know enough about it yet to say.

mobileink commented 1 year ago

As an example of the kind of help I could use: src/passes/WasmIntrinsics.cpp.in is processed by src/passes/CMakeLists.txt:

file(READ wasm-intrinsics.wat WASM_INTRINSICS_WAT HEX)

string(REGEX MATCHALL "([A-Fa-f0-9][A-Fa-f0-9])" SEPARATED_HEX ${WASM_INTRINSICS_WAT})

set(WASM_INTRINSICS_SIZE 1)
foreach (hex IN LISTS SEPARATED_HEX)
  string(APPEND WASM_INTRINSICS_EMBED "0x${hex},")
  math(EXPR WASM_INTRINSICS_SIZE "${WASM_INTRINSICS_SIZE}+1")
endforeach ()
string(APPEND WASM_INTRINSICS_EMBED "0x00")

configure_file(WasmIntrinsics.cpp.in WasmIntrinsics.cpp @ONLY)

I believe that just converts wasm-intrinsics.wat to hex and appends 0x00. If so, there are various ways to do this in Bazel. The most portable way would be to write custom Bazel rule and a little C program to do the job. A shell script would work but not on Windows. But before I do that I'd like to make sure I'm interpreting the CMake code correctly, since I don't know CMake very well.

Then there are all the executables in tools/CMakeLists.txt and how those targets interact with the root CMakeLists.txt, for compile flags and linking - I think they would be pretty easy to bazelize if I knew CMake well, but since I don't I could use some help.

walkingeyerobot commented 1 year ago

I think you're on the right track wanting to abstract that out, and you're right that a shell script isn't portable enough. A little C program would be fine, but I think python is a more suitable tool for something like this. I also happen to have already written this python script and I can probably just submit it.

Stepping back a bit, I have a bazel BUILD file for binaryen that's been working for awhile. I haven't submitted it because I didn't really think it would be useful for anyone else and because I didn't want to add extra maintenance costs onto binaryen owners. I was clearly mistaken.

Give me a few days and I'll get you what I have. From there we can figure out what the best solution is for all those interested.

walkingeyerobot commented 1 year ago

Apologies for the delay. Turns out there is yet more interest in making this happen, so I think this is definitely a good idea to add.

A colleague of mine (@mollyibot) has our BUILD file here: https://github.com/WebAssembly/binaryen/tree/57c0841ebb4537b29cc23a0fc53dfdbe3f3b8f55. She's going to be working on adding bazel + binaryen support anyway, so might as well combine efforts.

mobileink commented 12 months ago

Sounds good. Maybe we should start a discussion topic?

kripken commented 12 months ago

@mobileink see #5925

walkingeyerobot commented 12 months ago

There's a fair bit of talk about "google3" in the PR and I want to give some context to @mobileink so they can better follow and participate in the conversation.

From https://opensource.google/documentation/reference/glossary#google3:

"The internal name for our main Piper source repository, and identifies the third incarnation of the source layout for Google production code."

@mollyibot and myself are both Google engineers on different team. I work on C++ toolchains and making binaryen work within Google's internal infrastructure is part of my job. Molly works on j2cl which is making use of binaryen in their Java to WebAssembly compiler. Internally, many years ago, I wrote binaryen + bazel support for j2cl and other internal customers. However, there are significant differences in Google's internal implementation versus the open source version of bazel that prevent me from simply committing what I have.

At the same time as you filed this issue, Molly and her team approached me asking about upstream bazel support for binaryen (which is pretty great timing imo). I previously didn't believe there was anyone interested in building binaryen via bazel, so I didn't spend any effort open-sourcing what I had. I'm happy to have been proven wrong.

Going forward, I'd like to keep as much conversation about this as public as possible. For this to succeed I believe it's important that we consider open source first and then Google's internal version second. I also believe @mobileink has done some excellent work in their fork that should be incorporated, and they've made insightful and productive comments in the PR already.

There are also some Google internal only concerns. Our goal is to not just open source what we have, but to make github the source of truth for any bazel infrastructure. There will need to be a transformation step internally to compensate for differences between internal and external bazel, but hopefully we don't clutter the conversation too much with talk of this. Again, I believe we need to build bazel support for open source first and Google's internal version second.

mobileink commented 12 months ago

Thanks, very helpful. I hope to find time to look at binaryen again later this week.

On Mon, Sep 11, 2023, 4:44 PM walkingeyerobot @.***> wrote:

There's a fair bit of talk about "google3" in the PR and I want to give some context to @mobileink https://github.com/mobileink so they can better follow and participate in the conversation.

From https://opensource.google/documentation/reference/glossary#google3:

"The internal name for our main Piper source repository, and identifies the third incarnation of the source layout for Google production code."

@mollyibot https://github.com/mollyibot and myself are both Google engineers on different team. I work on C++ toolchains and making binaryen work within Google's internal infrastructure is part of my job. Molly works on j2cl https://github.com/google/j2cl which is making use of binaryen in their Java to WebAssembly compiler. Internally, many years ago, I wrote binaryen + bazel support for j2cl and other internal customers. However, there are significant differences in Google's internal implementation versus the open source version of bazel that prevent me from simply committing what I have.

At the same time as you filed this issue, Molly and her team approached me asking about upstream bazel support for binaryen (which is pretty great timing imo). I previously didn't believe there was anyone interested in building binaryen via bazel, so I didn't spend any effort open-sourcing what I had. I'm happy to have been proven wrong.

Going forward, I'd like to keep as much conversation about this as public as possible. For this to succeed I believe it's important that we consider open source first and then Google's internal version second. I also believe @mobileink https://github.com/mobileink has done some excellent work in their fork that should be incorporated, and they've made insightful and productive comments in the PR already.

There are also some Google internal only concerns. Our goal is to not just open source what we have, but to make github the source of truth for any bazel infrastructure. There will need to be a transformation step internally to compensate for differences between internal and external bazel, but hopefully we don't clutter the conversation too much with talk of this. Again, I believe we need to build bazel support for open source first and Google's internal version second.

— Reply to this email directly, view it on GitHub https://github.com/WebAssembly/binaryen/issues/5902#issuecomment-1714626400, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAES2NBTWSUXQTKXF2L7L6DXZ6A2ZANCNFSM6AAAAAA37WAVZA . You are receiving this because you were mentioned.Message ID: @.***>