MacOS: Add a Homebrew OpenSSL directory configuration option. #1301

junaruga · 2023-01-13T15:24:19Z

This PR is to add Homebrew OpenSSL directory configuration option in MacOS environment. This fixes #1300. It is related to and #1297, as this PR passes CI MacOS cases.

There are 3 commits. The first 2 commits come from another PR: #1299. The third commit is the main one of this PR.

What do you think? If the PR will be merged, then we need to add this option to the README.md - Configuration options section, and I am happy to help. Note I don't have MacOS environment. I appreciate if someone can help to test this PR on MacOS environment.

I confirmed the CI passed on my forked repository. (CI link), and the error case by directory existing check (CI link).

In the GitHub Actions build.yml, the way of how to set the matrix environment variables is used in ruby/ruby. You can see this.

Commit message

MacOS: Add a Homebrew OpenSSL directory configuration option.

This commit is to enable users to set a custom Homebrew openssl directory
configuration option to set the openssl library path on MacOS.

Previously we got the openssl library path by brew --prefix openssl on MacOS.
However, there is a case where the directory actually doesn't exist. And this
option enables users to build mysql2 in the case.

+ brew --prefix openssl
/usr/local/opt/openssl@3

+ ls -ld /usr/local/opt/openssl*
lrwxr-xr-x  1 runner  admin  36 Dec 16 01:28 /usr/local/opt/openssl -> /usr/local/Cellar/[email protected]/1.1.1s
lrwxr-xr-x  1 runner  admin  28 Dec 16 01:19 /usr/local/opt/[email protected] -> ../Cellar/[email protected]/1.1.1s
/usr/local/opt/[email protected]

== How to use ==

In the development, you can run like this.

$ RUBY_MYSQL2_BREW_SSL_DIR=/usr/local/opt/[email protected] rake

In the installation, you can run like this.

$ gem install mysql2 -- --with-brew-ssl-dir=/usr/local/opt/[email protected]

== Note ==

In this commit, the directory existing check is added in the extconf.rb. If
the openssl directory doesn't exist, the build fails with the error message
below.

Cannot find library dir(s) /usr/local/opt/openssl@3/lib

psulightning · 2023-01-13T15:32:56Z

Am I missing it where the builds do not factor in the case without the ENV var set, nor the case where openssl@3 is installed under the Mac OS?

junaruga · 2023-01-13T15:39:18Z

Am I missing it where the builds do not factor in the case without the ENV var set, nor the case where openssl@3 is installed under the Mac OS?

Good question! As far as I know, we don't find how to install openssl3 on MacOS environment because of this issue: #1297.

If the env is not set, and the openssl@3 directory by brew --prefix openssl doesn't exist, the mysql2 build fails with the error message "Cannot find library dir(s) /usr/local/opt/openssl@3/lib". I tested it on my forked GitHub Actions.

psulightning · 2023-01-13T15:43:28Z

That would be an interesting case also to test against: in essence, the wholly negative case where there is no ENV var, no command line switch, and brew install openssl never run.

junaruga · 2023-01-13T16:06:04Z

That would be an interesting case also to test against: in essence, the wholly negative case where there is no ENV var, no command line switch, and brew install openssl never run.

If there is no ENV var, there is no command line switch. That is the MacOS cases on the current master branch. I added the 2 cases to the CI on my forked repo's testing branch. I am not sure that we need total 4 MacOS cases on the CI. If the maintainers want to add the cases, I am happy to add the cases to this PR.

https://github.com/junaruga/mysql2/actions/runs/3912863766/jobs/6688047019
junaruga@3ff8199

sodabrew · 2023-01-18T20:37:55Z

Would it be more general to solve this by adding a --with-ssl-dir, so the brew command can be passed in like --with-ssl-dir=$(brew --prefix [email protected] ?

junaruga · 2023-01-18T21:47:56Z

Sure. Good point! I will try it. You meant the --with-ssl-dir option is implemented in both Mac and Linux cases or only in the Mac cases in this PR?

junaruga · 2023-01-19T16:35:31Z

I just rebased this PR with the way you mentioned on the lastet master branch, and updated the commit message too.

The /usr/local/opt/[email protected] in the RUBY_MYSQL2_SSL_DIR: '/usr/local/opt/[email protected] in the .github/workflows/build.yml is equivalent with the brew --prefix [email protected]. I changed the logic to set the RUBY_MYSQL2_SSL_DIR in the CI yaml. I think this way is more aligned with the current style.

This commit is to enable users to set a custom Homebrew openssl directory configuration option to set the openssl library path on MacOS. Previously we got the openssl library path by `brew --prefix openssl` on MacOS. However, there is a case where the directory actually doesn't exist. And this option enables users to build mysql2 in the case. ``` + brew --prefix openssl /usr/local/opt/openssl@3 + ls -ld /usr/local/opt/openssl* lrwxr-xr-x 1 runner admin 36 Dec 16 01:28 /usr/local/opt/openssl -> /usr/local/Cellar/[email protected]/1.1.1s lrwxr-xr-x 1 runner admin 28 Dec 16 01:19 /usr/local/opt/[email protected] -> ../Cellar/[email protected]/1.1.1s /usr/local/opt/[email protected] ``` == How to use == In the development, you can run like this. ``` $ RUBY_MYSQL2_SSL_DIR=/usr/local/opt/[email protected] rake ``` In the installation, you can run like this. ``` $ gem install mysql2 -- --with-ssl-dir=/usr/local/opt/[email protected] ``` == Note == In this commit, the directory existing check is added in the `extconf.rb`. If the openssl directory doesn't exist, the build fails with the error message below. ``` Cannot find library dir(s) /usr/local/opt/openssl@3/lib ```

sodabrew · 2023-01-19T17:55:14Z

.github/workflows/build.yml

      # https://github.com/ruby/setup-ruby
      - uses: ruby/setup-ruby@v1
        with:
          ruby-version: ${{ matrix.ruby }}
          bundler-cache: true # runs 'bundle install' and caches installed gems automatically
      - if: matrix.db != ''
        run: echo 'DB=${{ matrix.db }}' >> $GITHUB_ENV
+      - if: startsWith(matrix.os, 'macos') && matrix.ssl != ''
+        run: echo "RUBY_MYSQL2_SSL_DIR=$(brew --prefix ${{ matrix.ssl }})" >> $GITHUB_ENV


Rather than threading a new environment variable into the app code, I'd prefer to add an argument to the build step, like:

bundle exec rake clean compile -- --with-mysql-dir=/var/tmp

(demonstrating that this will forcibly fail the build because there's no /var/tmp/lib or /var/tmp/include)

sodabrew · 2023-01-19T17:57:28Z

ext/mysql2/extconf.rb

@@ -28,8 +28,16 @@ def add_ssl_defines(header)

 # Homebrew openssl
 if RUBY_PLATFORM =~ /darwin/ && system("command -v brew")
-  openssl_location = `brew --prefix openssl`.strip
-  $LDFLAGS << " -L#{openssl_location}/lib" if openssl_location
+  _, lib = dir_config('ssl')


I double-checked, Ruby is calling with --with-openssl-dir so we should go with that. I'm thinking a recommendation to be made is something like:

bundle exec rake clean compile -- $(ruby -r rbconfig -e 'puts RbConfig::CONFIG["configure_args"]' | xargs -n1 | grep with-openssl-dir)

The same could be made a Bundle config option so that bundle install builds mysql2 against the version of Ruby and whichever openssl it's linked against

I think the include dir is required as well:

inc, lib = dir_config('openssl') $INCFLAGS << " -I#{inc}" $LDFLAGS << " -L#{lib}"

Oh maybe the include dir isn't required -- I don't think any openssl functions are called from code, rather, the libmysqlclient needs to be able to load an openssl library at runtime.

sodabrew · 2023-01-19T18:02:41Z

ext/mysql2/extconf.rb

@@ -28,8 +28,16 @@ def add_ssl_defines(header)

 # Homebrew openssl
 if RUBY_PLATFORM =~ /darwin/ && system("command -v brew")


Move dir_config(openssl) outside of the Darwin block

sodabrew · 2023-01-19T21:32:12Z

Superseded by #1303

This was referenced Jan 13, 2023

OpenSSL3 required on macOS and homebrew? #1300

Closed

CI: update runtime environments #1299

Merged

junaruga marked this pull request as ready for review January 13, 2023 15:31

junaruga force-pushed the wip/ci-macos-openssl branch from 2fac913 to 97e2cbf Compare January 16, 2023 22:00

sodabrew added 2 commits January 19, 2023 16:34

CI: GitHub Actions MacOS images now include openssl

769c422

CI: reduce MacOS CI time, don't need to clean a runtime that isn't saved

bedfa55

junaruga force-pushed the wip/ci-macos-openssl branch 2 times, most recently from 56353bd to 90214cf Compare January 19, 2023 16:34

junaruga force-pushed the wip/ci-macos-openssl branch from 90214cf to b8987e3 Compare January 19, 2023 16:37

sodabrew requested changes Jan 19, 2023

View reviewed changes

sodabrew reviewed Jan 19, 2023

View reviewed changes

sodabrew mentioned this pull request Jan 19, 2023

Add an OpenSSL directory configuration option #1303

Merged

sodabrew closed this Jan 19, 2023

junaruga deleted the wip/ci-macos-openssl branch January 20, 2023 15:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MacOS: Add a Homebrew OpenSSL directory configuration option. #1301

MacOS: Add a Homebrew OpenSSL directory configuration option. #1301

junaruga commented Jan 13, 2023 •

edited

Loading

psulightning commented Jan 13, 2023

junaruga commented Jan 13, 2023

psulightning commented Jan 13, 2023 •

edited

Loading

junaruga commented Jan 13, 2023 •

edited

Loading

sodabrew commented Jan 18, 2023

junaruga commented Jan 18, 2023

junaruga commented Jan 19, 2023

sodabrew Jan 19, 2023

sodabrew Jan 19, 2023

sodabrew Jan 19, 2023

sodabrew Jan 19, 2023

sodabrew commented Jan 19, 2023

		@@ -28,8 +28,16 @@ def add_ssl_defines(header)

		# Homebrew openssl
		if RUBY_PLATFORM =~ /darwin/ && system("command -v brew")

MacOS: Add a Homebrew OpenSSL directory configuration option. #1301

MacOS: Add a Homebrew OpenSSL directory configuration option. #1301

Conversation

junaruga commented Jan 13, 2023 • edited Loading

Commit message

psulightning commented Jan 13, 2023

junaruga commented Jan 13, 2023

psulightning commented Jan 13, 2023 • edited Loading

junaruga commented Jan 13, 2023 • edited Loading

sodabrew commented Jan 18, 2023

junaruga commented Jan 18, 2023

junaruga commented Jan 19, 2023

sodabrew Jan 19, 2023

Choose a reason for hiding this comment

sodabrew Jan 19, 2023

Choose a reason for hiding this comment

sodabrew Jan 19, 2023

Choose a reason for hiding this comment

sodabrew Jan 19, 2023

Choose a reason for hiding this comment

sodabrew commented Jan 19, 2023

junaruga commented Jan 13, 2023 •

edited

Loading

psulightning commented Jan 13, 2023 •

edited

Loading

junaruga commented Jan 13, 2023 •

edited

Loading