GH-36456: [CI][R] Unlink system OpenSSL to avoid mixing OpenSSL versions #36522
Conversation
|
|
|
@github-actions crossbow submit r-binary-packages homebrew-r-autobrew |
This comment was marked as outdated.
This comment was marked as outdated.
|
@github-actions crossbow submit r-binary-packages homebrew-r-autobrew |
This comment was marked as outdated.
This comment was marked as outdated.
kou
changed the title
|
This will fix our nightly build but could this not also happen when users build from scratch? |
|
In general, it'll not be happen on user environments because Homebrew's OpenSSL packages don't put symlinks to the default path by default. (If you're interested in it, please search "keg-only".) But GitHub Actions runs |
|
Ah interesting, thanks! |
|
Oh... This doesn't resolve this problem... |
|
@github-actions crossbow submit r-binary-packages homebrew-r-autobrew |
This comment was marked as outdated.
This comment was marked as outdated.
|
@github-actions crossbow submit r-binary-packages homebrew-r-autobrew |
This comment was marked as outdated.
This comment was marked as outdated.
|
@github-actions crossbow submit r-binary-packages homebrew-r-autobrew |
This comment was marked as outdated.
This comment was marked as outdated.
|
@github-actions crossbow submit r-binary-packages homebrew-r-autobrew |
This comment was marked as outdated.
This comment was marked as outdated.
|
@github-actions crossbow submit r-binary-packages homebrew-r-autobrew |
This comment was marked as outdated.
This comment was marked as outdated.
|
@github-actions crossbow submit r-binary-packages homebrew-r-autobrew |
|
Revision: e79c31e Submitted crossbow builds: ursacomputing/crossbow @ actions-32865d51a3
|
| # Ensure removing OpenSSL from the default paths to avoid | ||
| # mixing OpenSSL in the default paths and OpenSSL installed | ||
| # by autobrew. | ||
| brew unlink openssl || : |
There was a problem hiding this comment.
Hmm... can we instead make sure our build chain selects OpenSSL consistently? Users will otherwise probably encounter a similar problem if they try compiling Arrow themselves.
There was a problem hiding this comment.
That would also be my preference though it seems to be tricky and I haven't had the time to investigate.
There was a problem hiding this comment.
I think that it's difficult.
For example, please consider the following case:
- OpenSSL 1.1 is installed in
/usr/local - OpenSSL 3 is install in
/opt/openssl - c-ares is installed in
/usr/local - Build bundled gRPC with c-ares and OpenSSL 3
- gRPC uses
-I/usr/local/includefrom c-ares and-I/opt/openssl/includefrom OpenSSL 3 -I/opt/openssl/include -I/usr/local/includewill work but-I/usr/local/include -I/opt/openssl/includewill not work because OpenSSl 1.1 headers in/usr/local/includeare used- I don't think that we can always use include flags from OpenSSL 3 as the first include flag because there are more version mixable libraries such as Abseil
| find / 2>&1 | grep include/openssl || : | ||
| arrow/ci/scripts/r_test.sh arrow |
There was a problem hiding this comment.
I'm a bit embarassed, but I don't understand what these two lines do...
There was a problem hiding this comment.
I think the first line is just for debug purposes printing all find hits of include/openssl or nothing. The script setup an env for r testing and then test/checks the r package.
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes. It's just a debug print.
I want to find OpenSSl header files in the system but I couldn't find them...
|
Oh great it looks like @paleolimbot was able to repro locally and fix the underlying issue in #36551 🎉 |
|
I close this in favor of #36551. |
github-actions
bot
added
awaiting changes
Rationale for this change
If OpenSSL exists in the default path, we may mix OpenSSL in the default path and OpenSSL installed by autobrew. It may cause a link error, run-time symbol not found error and so on.
What changes are included in this PR?
Ensure unlinking OpenSSL in the default path.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.