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

[Feature Request] Add minimal support to link core-bridge against musl #1271

Closed
2 tasks
mjameswh opened this issue Oct 26, 2023 · 3 comments
Closed
2 tasks
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@mjameswh
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Some TS SDK users have strong requirement of only using alpine-based Docker images. This is a problem, as the alpine distribution is based on the musl C library (rather than the GNU's libc found in most other distributions), which isn't supported at the moment by the @temporalio/core-bridge package. Is is therefore not possible to run a TS SDK Worker in an alpine-based container.

The team is hesitant about shipping a musl-based native library out of the box, as this would increase the download size for all of our users.

In theory, self-building the native library would be a reasonably simple solution, since core-bridge is designed to build the native lib if it can't find the binary file corresponding to the current target environment. However, the TS logic that identify the current target can't recognize the fact that it is running on a musl-based system (node itself provides no information on that regard).

Describe the solution you'd like

  • Modify core-bridge's target environment detection logic to recognize that it is running in a musl-based system, and to consequently return the proper build target for those environments.

    With this change, the @temporalio/core-bridge will correctly identify that it is running in a musl environment, and therefore try to load the musl-based native library. Assuming that this library can't be found (ie. because we don't ship the musl), and that the proper go toolchain is installed, then the package would automatically build the native library. This build operation would only happen the first time the application execute, and the built artifact could be copied over. It would therefore be reasonably easy to include this build step as part of a multi-stage Dockerfile.

  • Add some musl-based environment to the CI test matrix

@mjameswh mjameswh added the enhancement New feature or request label Oct 26, 2023
@mjameswh
Copy link
Contributor Author

Marking this issue as Good First Contribution — specifically the first part: "adding detection logic to recognize musl environment". That requires modifying this file. Can probably find hints of a musl build by looking into process or os, or maybe process.report.getReport() (IIRC, that last one provides the list of loaded libraries).

Adding musl to the CI test matrix might be best left to official maintainers.

@mjameswh mjameswh added the good first issue Good for newcomers label Sep 27, 2024
@korbin
Copy link

korbin commented Nov 20, 2024

angellist@c34ba64

I don't have time to pull through the CI-related changes to make this work, but, this does seem to work without issue on an Alpine system with:

apk add --no-cache build-base cmake protoc protobuf-dev rustup
rustup-init -y

export RUSTFLAGS="-C target-feature=-crt-static"
export BUILD_CORE_RELEASE="true"

I added strip = true to the release profile and the musl binary is only 6.9MB. This build takes long enough (6m on c8g.16xlarges) that it's probably worth including in the prebuilt binaries list.

@mjameswh
Copy link
Contributor Author

mjameswh commented Feb 7, 2025

Superseded by #1621.

@mjameswh mjameswh closed this as completed Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants