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

Conversation

larskanis
Copy link
Contributor

Since RubyInstaller-2.4+ is bundled with MSYS2 and the libmariadbclient can be installed per gemspec library dependency, it is easy to build the mysql2 gem in Windows.

The MSYS2/MINGW dependency feature is documented here: https://github.com/oneclick/rubyinstaller2/wiki/For-gem-developers#msys2-library-dependency

This also adds ruby-2.4 binaries (per rake-compiler-dock-0.6.0), so that the mysql2 is still useabel as a binary gem.

Fixes #861

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

Due to the stdcall convention on i686, the mysql_query() function check fails, so that it is omitted, now.

Since RubyInstaller-2.4+ is bundled with MSYS2 and the libmariadbclient
can be installed per gemspec library dependency, it is easy to build
the mysql2 gem in Windows.

The MSYS2/MINGW dependency feature is documented here:
https://github.com/oneclick/rubyinstaller2/wiki/For-gem-developers#msys2-library-dependency

This also adds ruby-2.4 binaries, so that the mysql2 is still usabel as
a binary gem.

Fixes brianmario#861

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

Due to the stdcall convention on i686, the mysql_query() function check
fails, so that it is omitted, now.
@sodabrew sodabrew added this to the 0.5.0 milestone Aug 1, 2017
@@ -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.

@@ -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__))
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.

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

@@ -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!

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

@larskanis
Copy link
Contributor Author

Is there anything I can do to get this merged and released?

@sodabrew sodabrew merged commit a2fadb6 into brianmario:master Nov 25, 2017
junaruga added a commit to junaruga/mysql2 that referenced this pull request Nov 27, 2017
Since RubyInstaller-2.4+ is bundled with MSYS2 and the libmariadbclient
can be installed per gemspec library dependency, it is easy to build
the mysql2 gem in Windows.

The MSYS2/MINGW dependency feature is documented here:
https://github.com/oneclick/rubyinstaller2/wiki/For-gem-developers#msys2-library-dependency

This also adds ruby-2.4 binaries, so that the mysql2 is still usabel as
a binary gem.

Fixes brianmario#861

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

Due to the stdcall convention on i686, the mysql_query() function check
fails, so that it is omitted, now.

Signed-off-by: Jun Aruga <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants