Skip to content

Conversation

@xxlaykxx
Copy link

Rationale for this change

GitHub Actions self-hosted runner for macOS has
/usr/local/include/openssl/ provided by OpenSSL 3 (openssl@ 3). Our include paths have ... -isystem /usr/local/include -isystem /usr/local/opt/openssl@ 1.1/include .... It means that /usr/local/include/openssl/... is used for #include <openssl/...>.

If we mix OpenSSL 3 headers and OpenSSL 1.1 libraries, we may get some problems such as a link error.

What changes are included in this PR?

This uses OpenSSL 3 instead of OpenSSL 1.1 because GitHub Actions self-hosted runner for macOS provides OpenSSL 3 by /usr/local/include/openssl/. Note that $(brew --prefix openssl@ 3)/include isn't linked as /usr/local/include/openssl` by default. So I think that Homebrew GitHub Actions self-hosted runner for macOS does it explicitly.

Other solution: Unlinking /usr/local/include/openssl by brew unlink openssl@ 3. But there is no reason to use OpenSSL 1.1 for us. So this PR doesn't use this solution.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Authored-by: Sutou Kouhei [email protected]

### Rationale for this change

GitHub Actions self-hosted runner for macOS has
/usr/local/include/openssl/ provided by OpenSSL 3 (`openssl@ 3`). Our include paths have `... -isystem /usr/local/include -isystem /usr/local/opt/openssl@ 1.1/include ...`. It means that `/usr/local/include/openssl/...` is used for `#include <openssl/...>`.

If we mix OpenSSL 3 headers and OpenSSL 1.1 libraries, we may get some problems such as a link error.

### What changes are included in this PR?

This uses OpenSSL 3 instead of OpenSSL 1.1 because GitHub Actions self-hosted runner for macOS provides OpenSSL 3 by /usr/local/include/openssl/. Note that `$(brew --prefix openssl@ 3)/include` isn't linked as /usr/local/include/openssl` by default. So I think that Homebrew GitHub Actions self-hosted runner for macOS does it explicitly.

Other solution: Unlinking `/usr/local/include/openssl` by `brew unlink openssl@ 3`. But there is no reason to use OpenSSL 1.1 for us. So this PR doesn't use this solution.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* Closes: apache#36329

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
@github-actions
Copy link

❌ GitHub issue apache#36329 could not be retrieved.

@xxlaykxx xxlaykxx merged commit cfe29f2 into dremio:dremio_24.3_12.0 Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants