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

feat: build wasm in common wasm-builder #105

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

mhdawson
Copy link
Member

Refs: nodejs/security-wg#1236

Update to build in docker using wasm-builder image maintained in https://github.com/nodejs/wasm-builder.

As a side effect this also updates the versions of the wasm tools to a newer version.

undici and amaro have already been moved over and I plan to document in Node.js docs that this is the projects standard way to build WASM blobs.

@mhdawson
Copy link
Member Author

This action https://github.com/nodejs/cjs-module-lexer/blob/main/.github/workflows/build.yml might need an update if there is any setup required to use docker (I'm not sure if there is or not, we'll find out as soon as the action runs).

Either way it likely needs an update to remove the steps that install the wasi sdk etc. as they will no longer be needed after this PR lands.

@mhdawson
Copy link
Member Author

@guybedford FYI as discussed. Have already moved over undici and amaro, so the cjs-module-lexer is the last one.

@mhdawson
Copy link
Member Author

Looks lioke the test will never run as its trying to use ubuntu18, opened #106 to fix that.

In addition I think this will be needed in the action:

  - name: Set up Docker
    uses: docker/setup-buildx-action@c47758b77c9736f4b2ef4073d4d51994fabfe349 # v3.7.1

Once #106 lands I'll rebase and then update the action to add this as well as remove the parts that are no longer needed.

Refs: nodejs/security-wg#1236

Update to build in docker using wasm-builder image
maintained in https://github.com/nodejs/wasm-builder.

As a side effect this also updates the versions of the
wasm tools to a newer version.

undici and amaro have already been moved over and I plan to
document in Node.js docs that this is the projects standard
way to build WASM blobs.

Signed-off-by: Michael Dawson <midawson@redhat.com>
@mhdawson mhdawson force-pushed the common-wasm-builder branch from bafd138 to 731b4cb Compare November 21, 2024 14:29
@mhdawson
Copy link
Member Author

@guybedford just needed one small tweak to create the dist directory if it does not exist and build/tests pass.

@guybedford guybedford merged commit 8411abd into nodejs:main Nov 21, 2024
1 check passed
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.

2 participants