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.

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

Fix compat with RubyInstaller-2.4 on Windows #875

Merged
merged 1 commit into from
Nov 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ source 'https://rubygems.org'
gemspec

gem 'rake', '~> 10.4.2'
gem 'rake-compiler', '~> 0.9.5'
gem 'rake-compiler', '~> 1.0'

group :test do
gem 'eventmachine' unless RUBY_PLATFORM =~ /mswin|mingw/
Expand All @@ -22,7 +22,7 @@ end

group :development do
gem 'pry'
gem 'rake-compiler-dock', '~> 0.5.1'
gem 'rake-compiler-dock', '~> 0.6.0'
end

platforms :rbx do
Expand Down
11 changes: 8 additions & 3 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
version: "{build}"
clone_depth: 10
install:
- SET PATH=C:\MinGW\msys\1.0\bin;%PATH%
- SET PATH=C:\Ruby%ruby_version%\bin;%PATH%
- ruby --version
- gem --version
- gem install bundler --quiet --no-ri --no-rdoc
- bundler --version
- bundle install --without benchmarks --path vendor/bundle
- IF DEFINED MINGW_PACKAGE_PREFIX (ridk exec pacman -S --noconfirm --needed %MINGW_PACKAGE_PREFIX%-libmariadbclient)
build_script:
- bundle exec rake compile
test_script:
Expand All @@ -20,11 +22,14 @@ test_script:
FLUSH PRIVILEGES;
"
- bundle exec rake spec
# Where do I get Unix find?
#on_failure:
# - find tmp -name "*.log" -exec cat {};
on_failure:
- find tmp -name "*.log" | xargs cat
environment:
matrix:
- ruby_version: "24-x64"
MINGW_PACKAGE_PREFIX: "mingw-w64-x86_64"
- ruby_version: "24"
MINGW_PACKAGE_PREFIX: "mingw-w64-i686"
- ruby_version: "23-x64"
- ruby_version: "23"
- ruby_version: "22-x64"
Expand Down
4 changes: 2 additions & 2 deletions ext/mysql2/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def add_ssl_defines(header)
else
_, usr_local_lib = dir_config('mysql', '/usr/local')

asplode("mysql client") unless find_library('mysqlclient', 'mysql_query', usr_local_lib, "#{usr_local_lib}/mysql")
asplode("mysql client") unless find_library('mysqlclient', nil, usr_local_lib, "#{usr_local_lib}/mysql")

rpath_dir = usr_local_lib
end
Expand Down Expand Up @@ -177,7 +177,7 @@ def add_ssl_defines(header)
$CFLAGS << ' -g -fno-omit-frame-pointer'
end

if RUBY_PLATFORM =~ /mswin|mingw/
if RUBY_PLATFORM =~ /mswin|mingw/ && !defined?(RubyInstaller)
# Build libmysql.a interface link library
require 'rake'

Expand Down
14 changes: 9 additions & 5 deletions lib/mysql2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,20 @@
ENV['RUBY_MYSQL2_LIBMYSQL_DLL']
elsif File.exist?(File.expand_path('../vendor/libmysql.dll', File.dirname(__FILE__)))
# Use vendor/libmysql.dll if it exists, convert slashes for Win32 LoadLibrary
File.expand_path('../vendor/libmysql.dll', File.dirname(__FILE__)).tr('/', '\\')
File.expand_path('../vendor/libmysql.dll', File.dirname(__FILE__))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I remember the backslashes being required, since the LoadLibrary call is native Windows and not Ruby.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LoadLibrary accepts both, forward and backward slashes as all filesystem related WINAPI functions do. There's no need to translate path separators. cmd and powershell don't recognize forward slashs, but can handle them as well.

elsif defined?(RubyInstaller)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is RUBY_PLATFORM when RubyInstaller is defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

x64-mingw32 and i386-mingw32 - the same as previous RubyInstaller versions.

# RubyInstaller-2.4+ native build doesn't need DLL preloading
else
# This will use default / system library paths
'libmysql.dll'
end

require 'Win32API'
LoadLibrary = Win32API.new('Kernel32', 'LoadLibrary', ['P'], 'I')
if 0 == LoadLibrary.call(dll_path)
abort "Failed to load libmysql.dll from #{dll_path}"
if dll_path
require 'Win32API'
LoadLibrary = Win32API.new('Kernel32', 'LoadLibrary', ['P'], 'I')
if 0 == LoadLibrary.call(dll_path)
abort "Failed to load libmysql.dll from #{dll_path}"
end
end
end

Expand Down
2 changes: 2 additions & 0 deletions mysql2.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ Mysql2::GEMSPEC = Gem::Specification.new do |s|

s.files = `git ls-files README.md CHANGELOG.md LICENSE ext lib support`.split
s.test_files = `git ls-files spec examples`.split

s.metadata['msys2_mingw_dependencies'] = 'libmariadbclient'
end
1 change: 0 additions & 1 deletion spec/mysql2/statement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,6 @@ def stmt_count

it 'should return number of rows affected by an insert' do
stmt = @client.prepare 'INSERT INTO lastIdTest (blah) VALUES (?)'
expect(stmt.affected_rows).to eq 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As commented: The change in the spec is required for mariadbclient. It throws an error if no query was executed.

stmt.execute 1
expect(stmt.affected_rows).to eq 1
end
Expand Down
8 changes: 6 additions & 2 deletions tasks/compile.rake
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Rake::ExtensionTask.new("mysql2", Mysql2::GEMSPEC) do |ext|
# clean compiled extension
CLEAN.include "#{ext.lib_dir}/*.#{RbConfig::CONFIG['DLEXT']}"

if RUBY_PLATFORM =~ /mswin|mingw/
if RUBY_PLATFORM =~ /mswin|mingw/ && !defined?(RubyInstaller)
# Expand the path because the build dir is 3-4 levels deep in tmp/platform/version/
connector_dir = File.expand_path("../../vendor/#{vendor_mysql_dir}", __FILE__)
ext.config_options = ["--with-mysql-dir=#{connector_dir}"]
Expand All @@ -26,6 +26,10 @@ Rake::ExtensionTask.new("mysql2", Mysql2::GEMSPEC) do |ext|
Rake::Task['lib/mysql2/mysql2.rb'].invoke
# vendor/libmysql.dll is invoked from extconf.rb
Rake::Task['vendor/README'].invoke

# only the source gem has a package dependency - the binary gem ships it's own DLL version
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: its for the possessive :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, but thanks for the hint!

spec.metadata.delete('msys2_mingw_dependencies')

spec.files << 'lib/mysql2/mysql2.rb'
spec.files << 'vendor/libmysql.dll'
spec.files << 'vendor/README'
Expand Down Expand Up @@ -77,7 +81,7 @@ task :devkit do
end

if RUBY_PLATFORM =~ /mingw|mswin/
Rake::Task['compile'].prerequisites.unshift 'vendor:mysql'
Rake::Task['compile'].prerequisites.unshift 'vendor:mysql' unless defined?(RubyInstaller)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not following why the unless RubyInstaller case is needed. This file shouldn't execute in the source code case, only when I'm building the binaries at my desktop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rake spec depends on compile, which is OK, but it shouldn't depend on the vendored mysql library on RubyInstaller-2.4.

The reason is: the simplest way to build the mysql2 gem is by using the libmariadbclient pacman package. It is automatically installed though the metadata['msys2_mingw_dependencies'] tag when running gem install mysql2

On the other hand the vendored library works as well, provided that the unzip command is installed. This can be done per pacman -S unzip. But I think that this shouldn't the default way on RubyInstaller-2.4.

Rake::Task['compile'].prerequisites.unshift 'devkit'
else
if Rake::Task.tasks.map(&:name).include? 'cross'
Expand Down