Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 3 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
769c422
CI: GitHub Actions MacOS images now include openssl
sodabrew Dec 28, 2022
bedfa55
CI: reduce MacOS CI time, don't need to clean a runtime that isn't saved
sodabrew Dec 28, 2022
b8987e3
MacOS: Add a OpenSSL directory configuration option.
junaruga Jan 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jobs:
- '2.2'
- '2.1'
db: ['']
ssl: ['']
include:
# Comment out due to ci/setup.sh stucking.
# - {os: ubuntu-18.04, ruby: 2.4, db: mariadb10.1}
Expand All @@ -39,29 +40,29 @@ jobs:
# MariaDB lastet version
# Allow failure due to the following test failures that rarely happens.
# https://github.com/brianmario/mysql2/issues/1194
- {os: macos-latest, ruby: '2.6', db: mariadb, allow-failure: true}
- {os: macos-latest, ruby: '2.6', db: mariadb, ssl: [email protected], allow-failure: true}
# MySQL latest version
# Allow failure due to the issue #1194.
- {os: macos-latest, ruby: '2.6', db: mysql, allow-failure: true}
- {os: macos-latest, ruby: '2.6', db: mysql, ssl: [email protected], allow-failure: true}
# On the fail-fast: true, it cancels all in-progress jobs
# if any matrix job fails unlike Travis fast_finish.
fail-fast: false
env:
BUNDLE_WITHOUT: development
# reduce MacOS CI time, don't need to clean a runtime that isn't saved
HOMEBREW_NO_INSTALL_CLEANUP: 1
HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK: 1
steps:
- uses: actions/checkout@v3
- name: Install openssl
if: matrix.os == 'macos-latest'
run: |
brew update
brew install openssl
# 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

- run: sudo echo "127.0.0.1 mysql2gem.example.com" | sudo tee -a /etc/hosts
- run: bash ci/setup.sh
- run: bundle exec rake spec
12 changes: 10 additions & 2 deletions ext/mysql2/extconf.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,16 @@ def add_ssl_defines(header)

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move dir_config(openssl) outside of the Darwin block

openssl_location = `brew --prefix openssl`.strip
$LDFLAGS << " -L#{openssl_location}/lib" if openssl_location
_, lib = dir_config('ssl')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

unless lib
openssl_location = `brew --prefix openssl`.strip
lib = "#{openssl_location}/lib" if openssl_location
end

if lib
abort "-----\nCannot find library dir(s) #{lib}\n-----" unless lib && lib.split(File::PATH_SEPARATOR).any? { |dir| File.directory?(dir) }
$LDFLAGS << " -L#{lib}"
end
end

# 2.1+
Expand Down
3 changes: 3 additions & 0 deletions tasks/compile.rake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ Rake::ExtensionTask.new("mysql2", Mysql2::GEMSPEC) do |ext|
POST_INSTALL_MESSAGE
end
end

ssl_dir = ENV['RUBY_MYSQL2_SSL_DIR']
ext.config_options << "--with-ssl-dir=#{ssl_dir}" if ssl_dir
end
Rake::Task[:spec].prerequisites << :compile

Expand Down