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

Relay Compiler v13: Providing a statically linked binary? #3739

Closed
Tracked by #3749
henkerik opened this issue Jan 11, 2022 · 6 comments
Closed
Tracked by #3749

Relay Compiler v13: Providing a statically linked binary? #3739

henkerik opened this issue Jan 11, 2022 · 6 comments
Labels
fixready help wanted rust Related to the compiler written in Rust

Comments

@henkerik
Copy link

henkerik commented Jan 11, 2022

Relay compiler v13 fails to run on our CI system:

/worker/build/f83b02af11d8b45f/root/node_modules/relay-compiler/linux-x64/relay: error while loading shared libraries: libssl.so.1.1: cannot open shared object file: No such file or directory

The relay executable requires a couple of shared libraries:

> objdump -p relay | grep NEEDED
  NEEDED               libssl.so.1.1
  NEEDED               libcrypto.so.1.1
  NEEDED               libgcc_s.so.1
  NEEDED               libpthread.so.0
  NEEDED               libm.so.6
  NEEDED               libdl.so.2
  NEEDED               libc.so.6
  NEEDED               ld-linux-x86-64.so.2 

Since we are running our builds in a Bazel sandbox (with remote build execution) we can have no control over the CI runner, so we can't simply install the missing libraries. This would also make our build non-hermetic which is not something we want. A solution would be to provide self contained statically linked binaries instead.

Related to: #3725

@alunyov
Copy link
Contributor

alunyov commented Jan 11, 2022

A solution would be to provide self contained statically linked binaries instead.

Yes. Our build configuration is very simple at the moment:

https://github.com/facebook/relay/blob/main/.github/workflows/ci.yml#L121-L125

What should we change?

@alunyov alunyov added help wanted rust Related to the compiler written in Rust labels Jan 11, 2022
@ch1ffa
Copy link
Contributor

ch1ffa commented Jan 13, 2022

@alunyov I can confirm that building with –target x86_64-unknown-linux-musl helps, but for CI it requires a custom docker image like this one https://gitlab.com/rust_musl_docker/image or GitHub action like this https://github.com/marketplace/actions/rust-musl-builder-slim. I'm not sure what would you prefer.

@alunyov
Copy link
Contributor

alunyov commented Jan 13, 2022

Thanks for looking into this @ch1ffa!

This action https://github.com/marketplace/actions/rust-musl-builder-slim were not updated since 2019. Not sure if we should use that version.

but for CI it requires a custom docker image

Can we add our own Dockerfile here and build with --target x86_64-unknown-linux-musl?

@ch1ffa
Copy link
Contributor

ch1ffa commented Jan 13, 2022

@alunyov yeah, I think so. My personal preference is to have a dedicated repo with an action, so the main pipeline would be looking cleaner. But I can test both options and then we can choose.

facebook-github-bot pushed a commit that referenced this issue Jan 19, 2022
Summary:
This PR attempts to fix #3725 and #3739.

A new target `x86_64-unknown-linux-musl` has been added to `ci.yml` along with the additional `musl-tools` package (can be removed in the future when [this issue](actions-rs/toolchain#102) is resolved) and the `hyper-tls/vendored` feature which allow to build openssl-sys package for the new target.

I ended up with building directly on GitHub Action VM without custom docker image because it allows to implement it just as a new target in matrix. Building in docker image requires a [separated job](https://github.com/ch1ffa/relay/blob/1c4651ff81cb901a487e4d9ca0a719058833f5a0/.github/workflows/ci.yml#L133).

[detect-libc](https://www.npmjs.com/package/detect-libc) is using for detecting correct platform.

Compiled binary has been tested inside `node:alpine` image. I'm not sure how to test it in the Bazel Sandbox, maybe it is better to ask the issue author.

Pull Request resolved: #3754

Test Plan:
Imported from GitHub, without a `Test Plan:` line.

**Static Docs Preview: relay**
|[Full Site](https://our.intern.facebook.com/intern/staticdocs/eph/1642521860/relay/)|

|**Modified Pages**|

Reviewed By: kassens

Differential Revision: D33622425

Pulled By: alunyov

fbshipit-source-id: 8c16074f2d15fd400ad657cd2497fde85b54ef6d
@alunyov
Copy link
Contributor

alunyov commented Jan 19, 2022

Hey @henkerik the fix by @ch1ffa just landed and available on NPM as relay-compiler@main. Can you verify if it fixes the issue for you?

Thank you!

@crubier
Copy link

crubier commented Jan 19, 2022

For people running in this problem, if the latest npm version does not fix the problem, a dirty workaround using docker is:

docker run -v $(pwd):/mnt node:14 bash -c "cd /mnt && npm i -g relay-compiler && relay-compiler"

(Might need to adapt to your repo file structure, but you get the idea), worked for us.

@alunyov alunyov closed this as completed Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixready help wanted rust Related to the compiler written in Rust
Projects
None yet
Development

No branches or pull requests

4 participants