Fix "cross" configuration for release workflow #14
Conversation
This is going to install the "libssl-dev" along wigh "openssl" in targets that don't have these packages by default. It also fix a configuration to pass down the "RUSTLER_NIF_VERSION" env variable to containers building with cross.
| dpkg --add-architecture $CROSS_DEB_ARCH | ||
|
|
||
| apt-get update | ||
| apt-get install --assume-yes libssl-dev:$CROSS_DEB_ARCH openssl:$CROSS_DEB_ARCH |
There was a problem hiding this comment.
@philss did you successfully build on arm-unknown-linux-gnueabihf and x86_64-unknown-linux-musl? When I tried that it failed for gnueabihf (I think because of cross-rs/cross#1018), while with musl I don't think there's a package available (looking here).
There was a problem hiding this comment.
@jonatanklosko I could build for those targets locally. I'm trying to build using the CI.
Did you try to install using the "pre-build" command in the "Cross.toml" file? I also bumped cross version, so maybe this is the reason it worked for me.
There was a problem hiding this comment.
That's what I did, but I may have missed something, if you can build then it's great :D
There was a problem hiding this comment.
That's what I did, but I may have missed something, if you can build then it's great :D
Oh, I think I got it right now: https://github.com/philss/tokenizers/actions/runs/3198809038. It needs the vendored package for arm-unknown-linux-gnueabihf and x86_64-unknown-linux-musl, and we can't install the package on musl, but that works fine.
There was a problem hiding this comment.
@jonatanklosko I think it should work good now.
There was a problem hiding this comment.
Yeah, vendoring definitely works, we can even make it such that this only happens on CI via feature flags:
[dependencies]
# ...
openssl = { version = "0.10", optional = true }
[features]
static_openssl = ["openssl/vendored"]
and cross build ... --features "static_openssl"
This does link openssl statically though.
There was a problem hiding this comment.
Good idea! I couldn't avoid vendoring for those targets, so I'm going to add the feature flag.
| @@ -9,6 +9,7 @@ env: | |||
| ELIXIR_VERSION: 1.13 | |||
| OTP_VERSION: 24.2 | |||
| MIX_ENV: test | |||
| TOKENIZERS_BUILD: "true" | |||
There was a problem hiding this comment.
In the "native" module. We want to always force the build in the CI. See:
tokenizers/lib/tokenizers/native.ex
Line 11 in 1222399
|
Looks good to me, thanks @philss! @josevalim let me know if we should roll with this for the release or move downloading to elixir already :) |
|
If it works, it works. Ship it! :) |
There was a problem hiding this comment.
Sounds good! @seanmor5 @cigrainger you can try a new release whenever :D
This is going to install the "libssl-dev" along wigh "openssl" in targets that don't have these packages by default.
It also fix a configuration to pass down the "RUSTLER_NIF_VERSION" env variable to containers building with cross.