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

Add .wasm cc dynamic library extension #17374

Closed

Conversation

zaucy
Copy link
Contributor

@zaucy zaucy commented Feb 1, 2023

Similar to #16091 except also allow the .wasm extension for dynamic libraries.

@zaucy zaucy requested a review from lberki as a code owner February 1, 2023 07:14
@ShreeM01 ShreeM01 added team-Rules-CPP Issues for C++ rules awaiting-user-response Awaiting a response from the author labels Feb 1, 2023
@zaucy
Copy link
Contributor Author

zaucy commented Feb 1, 2023

@kshyanashree was the awaiting-user-response label a mistake? Maybe awaiting-review was intended? :)

I'm happy to answer any questions if there is one.

@ShreeM01
Copy link
Contributor

ShreeM01 commented Feb 1, 2023

@kshyanashree was the awaiting-user-response label a mistake? Maybe awaiting-review was intended? :)

I'm happy to answer any questions if there is one.

Hi @zaucy! I could see some presubmits are failing, so I added label 'awaiting-user-response', Could you please fix the build failures?

@zaucy
Copy link
Contributor Author

zaucy commented Feb 1, 2023

Ah thanks for the clarification on the user response. I did check the presubmit initially, but the error seems unrelated to my change.

From log at https://buildkite.com/bazel/bazel-bazel-github-presubmit/builds/14124#01860bd4-71a2-4a5e-8c28-09abd19b4239

AssertionError: Could not find python binary: python

I can rebase to get any changes from the last 14 hours and try again.

@zaucy zaucy force-pushed the wasm-dynamic-library-extension branch from 01b7135 to d682ba4 Compare February 1, 2023 21:55
@ShreeM01 ShreeM01 added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Feb 1, 2023
Copy link
Contributor

@oquenchil oquenchil left a comment

Choose a reason for hiding this comment

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

The change is fine. The presubmit is failing for unrelated reasons, so merging will be blocked till that is fixed.

@oquenchil oquenchil added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Feb 3, 2023
@oquenchil oquenchil self-assigned this Feb 3, 2023
@copybara-service copybara-service bot closed this in 4b6f014 Feb 7, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Feb 7, 2023
zaucy added a commit to zaucy/bazel that referenced this pull request Feb 8, 2023
Similar to bazelbuild#16091 except also allow the `.wasm` extension for dynamic libraries.

Closes bazelbuild#17374.

PiperOrigin-RevId: 507668339
Change-Id: Id85f98bb804e573e5f93a922b67baf75a22baf16
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
Similar to #16091 except also allow the `.wasm` extension for dynamic libraries.

Closes #17374.

PiperOrigin-RevId: 507668339
Change-Id: Id85f98bb804e573e5f93a922b67baf75a22baf16
keertk pushed a commit that referenced this pull request Feb 28, 2023
* Add .wasm cc artifact executable extension

This allows creating a custom cc_toolchain that compiles to WebAssembly that outputs a wasm file with the standard extension, without needing to have a genrule that renames the file

Closes #16091.

PiperOrigin-RevId: 503921110
Change-Id: I3ed9e50a945d2e415e2ab34bd99ceea481c987f0

* Add .wasm cc dynamic library extension

Similar to #16091 except also allow the `.wasm` extension for dynamic libraries.

Closes #17374.

PiperOrigin-RevId: 507668339
Change-Id: Id85f98bb804e573e5f93a922b67baf75a22baf16

---------

Co-authored-by: Scott Bennett <[email protected]>
Co-authored-by: kshyanashree <[email protected]>
copybara-service bot pushed a commit that referenced this pull request Mar 13, 2023
This is an amendment to #17374

We have a C++ toolchain config that's being developed to support standalone wasm. We discovered that #17374 wasn't complete. Our example was poorly written and didn't actually create a wasm dynamic library. These changes allow us to successfully create a standalone wasm dynamic library. Sorry for the botched attempt previously.

I would like to add tests, but I'm unsure how to approach such a tests considering the automatic toolchain doesn't support wasm AFAICT.

Closes #17698.

PiperOrigin-RevId: 516204125
Change-Id: Iced5cc80a3151ffde7116b6264c89eaf40466ff5
ShreeM01 pushed a commit to ShreeM01/bazel that referenced this pull request Mar 13, 2023
This is an amendment to bazelbuild#17374

We have a C++ toolchain config that's being developed to support standalone wasm. We discovered that bazelbuild#17374 wasn't complete. Our example was poorly written and didn't actually create a wasm dynamic library. These changes allow us to successfully create a standalone wasm dynamic library. Sorry for the botched attempt previously.

I would like to add tests, but I'm unsure how to approach such a tests considering the automatic toolchain doesn't support wasm AFAICT.

Closes bazelbuild#17698.

PiperOrigin-RevId: 516204125
Change-Id: Iced5cc80a3151ffde7116b6264c89eaf40466ff5
ShreeM01 added a commit that referenced this pull request Mar 13, 2023
This is an amendment to #17374

We have a C++ toolchain config that's being developed to support standalone wasm. We discovered that #17374 wasn't complete. Our example was poorly written and didn't actually create a wasm dynamic library. These changes allow us to successfully create a standalone wasm dynamic library. Sorry for the botched attempt previously.

I would like to add tests, but I'm unsure how to approach such a tests considering the automatic toolchain doesn't support wasm AFAICT.

Closes #17698.

PiperOrigin-RevId: 516204125
Change-Id: Iced5cc80a3151ffde7116b6264c89eaf40466ff5

Co-authored-by: Ezekiel Warren <[email protected]>
ShreeM01 pushed a commit to ShreeM01/bazel that referenced this pull request Mar 13, 2023
This is an amendment to bazelbuild#17374

We have a C++ toolchain config that's being developed to support standalone wasm. We discovered that bazelbuild#17374 wasn't complete. Our example was poorly written and didn't actually create a wasm dynamic library. These changes allow us to successfully create a standalone wasm dynamic library. Sorry for the botched attempt previously.

I would like to add tests, but I'm unsure how to approach such a tests considering the automatic toolchain doesn't support wasm AFAICT.

Closes bazelbuild#17698.

PiperOrigin-RevId: 516204125
Change-Id: Iced5cc80a3151ffde7116b6264c89eaf40466ff5
ShreeM01 added a commit that referenced this pull request Mar 14, 2023
This is an amendment to #17374

We have a C++ toolchain config that's being developed to support standalone wasm. We discovered that #17374 wasn't complete. Our example was poorly written and didn't actually create a wasm dynamic library. These changes allow us to successfully create a standalone wasm dynamic library. Sorry for the botched attempt previously.

I would like to add tests, but I'm unsure how to approach such a tests considering the automatic toolchain doesn't support wasm AFAICT.

Closes #17698.

PiperOrigin-RevId: 516204125
Change-Id: Iced5cc80a3151ffde7116b6264c89eaf40466ff5

Co-authored-by: Ezekiel Warren <[email protected]>
fweikert pushed a commit to fweikert/bazel that referenced this pull request May 25, 2023
This is an amendment to bazelbuild#17374

We have a C++ toolchain config that's being developed to support standalone wasm. We discovered that bazelbuild#17374 wasn't complete. Our example was poorly written and didn't actually create a wasm dynamic library. These changes allow us to successfully create a standalone wasm dynamic library. Sorry for the botched attempt previously.

I would like to add tests, but I'm unsure how to approach such a tests considering the automatic toolchain doesn't support wasm AFAICT.

Closes bazelbuild#17698.

PiperOrigin-RevId: 516204125
Change-Id: Iced5cc80a3151ffde7116b6264c89eaf40466ff5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants