search

alexcrichton / openssl-src-rs

Source code and logic to build OpenSSL from source
Apache License 2.0
69 stars 114 forks source link

Enable engine support for musl #150

Closed neuronull closed 2 years ago

neuronull commented 2 years ago

In one of our (vector.dev) project's dependencies (rdkafka), a hard dependency on openssl Engine support was added. This resulted in cross compilation errors for our project (since openssl-src was not enabling the engine support), the findings were documented here:

https://github.com/vectordotdev/vector/pull/10302

In particular, I found that if I made the below change to openssl-src , I was able to successfully compile to the target x86_64-unknown-linux-musl using cross.

Would this be an acceptable change to make in openssl-src with this knowledge ?

Thanks!

neuronull commented 2 years ago

Hmm , looks like the CI for musl failed :/ It appears to be installing the debian packages for musl directly.

alexcrichton commented 2 years ago

Ah yeah, that's the reason it's disabled right now. If it can be enabled that would work well but it may require fiddling with cross-compile flags or something like that (I don't know myself)

neuronull commented 2 years ago

I'll take a look and see if I can find a solution.

alexcrichton commented 2 years ago

Unfortunately I don't think the right fix is to fix the container the build happens in here. Otherwise anyone else is going to have to apply the same fix which is a breaking change relative to today.

If you'd like you can also add a boolena parameter to configuration to opt-in to the engine being built.

neuronull commented 2 years ago

Unfortunately I don't think the right fix is to fix the container the build happens in here. Otherwise anyone else is going to have to apply the same fix which is a breaking change relative to today.

If you'd like you can also add a boolena parameter to configuration to opt-in to the engine being built.

Oh yes... that makes sense. I'll revert the CI change and add a configuration param 👍

neuronull commented 2 years ago

Realized I need to do this for the 111 release