GH-36329: [C++][CI] Use OpenSSL 3 on macOS #36336

kou · 2023-06-28T00:59:52Z

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: [C++][CI] OpenSSL link error in Gandiva on macOS #36329

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. 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.

github-actions · 2023-06-28T01:00:19Z

⚠️ GitHub issue #36329 has been automatically assigned in GitHub to PR creator.

kou · 2023-06-28T02:19:08Z

+1

The "C++ / AMD64 macOS 12 C++" is still failing but it's caused by #36331/#36248 .

pitrou · 2023-06-28T09:55:03Z

cpp/cmake_modules/FindOpenSSLAlt.cmake

                      OUTPUT_STRIP_TRAILING_WHITESPACE)
-      if(OPENSSL_BREW_PREFIX)
-        set(OPENSSL_ROOT_DIR ${OPENSSL_BREW_PREFIX})
+      if(OPENSSL11_BREW_PREFIX)


Should this be OPENSSL3_BREW_PREFIX?

Ah, yes. Good catch!

Ah, wait. I should have used OPENSSL30_... for it.

pitrou · 2023-06-28T09:56:34Z

cpp/cmake_modules/FindOpenSSLAlt.cmake

@@ -22,17 +22,24 @@ endif()
 if(APPLE AND NOT OPENSSL_ROOT_DIR)
  find_program(BREW brew)
  if(BREW)
-    execute_process(COMMAND ${BREW} --prefix "openssl@1.1"
-                    OUTPUT_VARIABLE OPENSSL11_BREW_PREFIX
+    execute_process(COMMAND ${BREW} --prefix "openssl"


I'm curious: why first try "openssl", then "openssl@3.0", then "openssl@1.1"?

Wouldn't "openssl" cover all other cases? I don't know how brew works...

openssl is the default OpenSSL formula. In general, it refers the latest OpenSSL formula (openssl@3.1 now).
If openssl@3.1 isn't installed, brew --prefix openssl is failed.

Then, this process falls back to openssl@3.0, then openssl@1.1. Because we want to use newer OpenSSL as much as possible.

raulcd · 2023-06-28T11:36:28Z

@github-actions crossbow submit java-jars

raulcd · 2023-06-28T11:37:19Z

Triggering java-jars because they seem to be failing on the nightlies due to this reason:

 Undefined symbols for architecture arm64:
  "_EVP_MD_get_size", referenced from:
      gandiva::gdv_hash_using_openssl(long long, void const*, unsigned long, evp_md_st const*, unsigned int, int*) in libgandiva.a(unity_3_cxx.cxx.o)
ld: symbol(s) not found for architecture arm64

github-actions · 2023-06-28T11:39:11Z

Revision: fc0a16e

Submitted crossbow builds: ursacomputing/crossbow @ actions-b784334a54

Task	Status
java-jars

raulcd · 2023-06-28T11:51:46Z

There seems to be a lot of failures around gandiva on the MacOs java-jars job. I am not sure if they are related:

 The following tests FAILED:
	 20 - arrow-compute-scalar-type-test (Failed)
	 38 - arrow-substrait-substrait-test (Failed)
	 44 - arrow-acero-asof-join-node-test (Failed)
	 78 - gandiva-internals-test (Failed)
	 80 - gandiva-filter-test (Failed)
	 81 - gandiva-projector-test (Failed)
	 82 - gandiva-projector-build-validation-test (Failed)
	 83 - gandiva-if-expr-test (Failed)
	 84 - gandiva-literal-test (Failed)
	 85 - gandiva-boolean-expr-test (Failed)
	 86 - gandiva-binary-test (Failed)
	 87 - gandiva-date-time-test (Failed)
	 88 - gandiva-to-string-test (Failed)
	 89 - gandiva-utf8-test (Failed)
	 90 - gandiva-hash-test (Failed)
	 91 - gandiva-in-expr-test (Failed)
	 92 - gandiva-null-validity-test (Failed)
	 93 - gandiva-decimal-test (Failed)
	 94 - gandiva-decimal-single-test (Failed)
	 95 - gandiva-filter-project-test (Failed)
	 96 - gandiva-projector-test-static (Failed)

kou · 2023-06-29T02:12:16Z

gandiva-* tests were crashed without backtrace... So we can't debug this. We may be able debug this by ssh to the host.

Anyway, how about tracking the failures as a separated issue? Because Gandiva tests in "AMD64 macOS 12 C++" aren't failed.

raulcd · 2023-06-29T14:10:04Z

Anyway, how about tracking the failures as a separated issue? Because Gandiva tests in "AMD64 macOS 12 C++" aren't failed.

Sounds good to me.

kou · 2023-06-30T00:41:38Z

Created: #36404

I'll merge this.

conbench-apache-arrow · 2023-07-05T19:07:37Z

Conbench analyzed the 6 benchmark runs on commit 9d92ed4d.

There were 5 benchmark results indicating a performance regression:

Commit Run on arm64-m6g-linux-compute at 2023-06-30 02:29:29Z
- params=, source=cpp-micro, suite=parquet-bloom-filter-benchmark
- params=, source=cpp-micro, suite=parquet-bloom-filter-benchmark
and 3 more (see the report linked below)

The full Conbench report has more details.

### 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 <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

### 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 <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com> Co-authored-by: Sutou Kouhei <kou@clear-code.com>

github-actions bot added Component: C++ awaiting committer review Awaiting committer review labels Jun 28, 2023

pitrou reviewed Jun 28, 2023

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jun 29, 2023

Simplify

3d9da20

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 29, 2023

kou merged commit 9d92ed4 into apache:main Jun 30, 2023
28 of 31 checks passed

kou deleted the cpp-macos-gandiva-openssl branch June 30, 2023 00:41

kou removed the awaiting change review Awaiting change review label Jun 30, 2023

paleolimbot mentioned this pull request Jul 4, 2023

[R] R package is linking to the wrong OpenSSL on MacOS BigSur? #36456

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-36329: [C++][CI] Use OpenSSL 3 on macOS #36336

GH-36329: [C++][CI] Use OpenSSL 3 on macOS #36336

kou commented Jun 28, 2023 •

edited by github-actions bot

github-actions bot commented Jun 28, 2023

kou commented Jun 28, 2023

pitrou Jun 28, 2023

kou Jun 29, 2023

pitrou Jun 28, 2023

kou Jun 29, 2023

raulcd commented Jun 28, 2023

raulcd commented Jun 28, 2023

github-actions bot commented Jun 28, 2023

raulcd commented Jun 28, 2023

kou commented Jun 29, 2023

raulcd commented Jun 29, 2023

kou commented Jun 30, 2023

conbench-apache-arrow bot commented Jul 5, 2023

GH-36329: [C++][CI] Use OpenSSL 3 on macOS #36336

GH-36329: [C++][CI] Use OpenSSL 3 on macOS #36336

Conversation

kou commented Jun 28, 2023 • edited by github-actions bot

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

github-actions bot commented Jun 28, 2023

kou commented Jun 28, 2023

pitrou Jun 28, 2023

Choose a reason for hiding this comment

kou Jun 29, 2023

Choose a reason for hiding this comment

pitrou Jun 28, 2023

Choose a reason for hiding this comment

kou Jun 29, 2023

Choose a reason for hiding this comment

raulcd commented Jun 28, 2023

raulcd commented Jun 28, 2023

github-actions bot commented Jun 28, 2023

raulcd commented Jun 28, 2023

kou commented Jun 29, 2023

raulcd commented Jun 29, 2023

kou commented Jun 30, 2023

conbench-apache-arrow bot commented Jul 5, 2023

kou commented Jun 28, 2023 •

edited by github-actions bot