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

wasm: enable on Linux-aarch64 and macOS-arm64. #15253

Merged
merged 6 commits into from
Mar 10, 2021

Conversation

PiotrSikora
Copy link
Contributor

@PiotrSikora PiotrSikora commented Mar 2, 2021

Skip Proxy-Wasm C++ SDK tests, since they require Emscripten,
which doesn't provide releases for arm64.

Addresses #14027.

Signed-off-by: Piotr Sikora piotrsikora@google.com

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Mar 2, 2021
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #15253 was synchronize by PiotrSikora.

see: more, trace.

@PiotrSikora PiotrSikora force-pushed the wasm-aarch64 branch 4 times, most recently from 584027d to fe4a46c Compare March 2, 2021 11:10
@PiotrSikora PiotrSikora changed the title wasm: enable Wasm on Linux-aarch64. wasm: enable Wasm on Linux-aarch64 and Darwin-arm64. Mar 2, 2021
@PiotrSikora PiotrSikora changed the title wasm: enable Wasm on Linux-aarch64 and Darwin-arm64. wasm: enable Wasm on Linux-aarch64 and macOS-arm64. Mar 2, 2021
Skip Proxy-Wasm C++ SDK tests, since they require Emscripten,
which doesn't provide releases for arm64.

Addresses envoyproxy#14027.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora PiotrSikora marked this pull request as ready for review March 2, 2021 11:36
@PiotrSikora PiotrSikora requested a review from lizan as a code owner March 2, 2021 11:36
@PiotrSikora
Copy link
Contributor Author

Skip Proxy-Wasm C++ SDK tests, since they require Emscripten, which doesn't provide releases for arm64.

Alternatives include:
a) check-in prebuilt .wasm test files and use them only in tests on arm64,
b) build Emscripten from sources on arm64 CI (possibly including LLVM),
c) wait for Emscripten to provide arm64 releases (see: emscripten-core/emsdk#547).

@PiotrSikora
Copy link
Contributor Author

NOTE: macOS-arm64 build is not tested, since I don't have access to the hardware, but I believe it should work... It would be great if someone could test it before we merge it, though.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Check envoy-presubmit didn't fail.

🐱

Caused by: a #15253 (comment) was created by @PiotrSikora.

see: more, trace.

@PiotrSikora
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15253 (comment) was created by @PiotrSikora.

see: more, trace.

@moderation
Copy link
Contributor

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Mar 4, 2021
Copy link
Contributor

@moderation moderation left a comment

Choose a reason for hiding this comment

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

Single change to dependency and sha256 and date LGTM

Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

I'm fine waiting for emscripten release, meanwhile skipping sounds fine to me.

@PiotrSikora PiotrSikora changed the title wasm: enable Wasm on Linux-aarch64 and macOS-arm64. wasm: enable on Linux-aarch64 and macOS-arm64. Mar 8, 2021
@lizan lizan merged commit 4d7d412 into envoyproxy:main Mar 10, 2021
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