Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
Support mysql2 0.4.0, first release with prepared statements support
Known failure on Ruby 2.3/trunk: brianmario/mysql2#671 Fixes #21544
- Loading branch information
1 parent
35842aa
commit 2dfff4d
Showing
3 changed files
with
4 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason this is causing problems for current users is because
rails new something --database=mysql
generates a local Gemfile with an unconstrainedgem 'mysql2'
. But then mysql2_adapter.rb, as above, was loading themysql2
gem withgem 'mysql2', '~> 0.3.13'
, which means only0.3.x
greater than0.3.13
.Now that mysql2 0.4.0 is out, for a bundle install/update on the Gemfile from a generated Rails app, bundler will install mysql2 0.4.0, because of the unconstrained line in a newly generated Gemfile. But mysql2_adapter will refuse to use it, giving a somewhat confusing (although not wrong) error message.
You've updated the spec in mysql2_adapter above to
gem 'mysql2', '>= 0.3.13', '< 0.5'
-- but the basic problem remains lying in wait for whenever a mysql2 0.5.0 is released. No?I think the same problem will occur when mysql 0.5.0 is released in the future, which it surely will be at some point --
rails new something --database=mysql
will add an unconstrained mysql to the app Gemfile, which will allow 0.5.0, but mysql2_adapter will refuse to use it. So this is not a great fix.I think the right solution is that the spec generated by
rails new --database=mysql
needs to match the spec in thegem
line in mysql2_adapter. Either they both need to be unconstrained (like the generated Gemfile is now), or they both need to be constrained, and matching each other (like thegem
line in mysql2_adapter is now).Am I wrong?
2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right. No optional version requirements in RubyGems! Otherwise we'd put the constraint on the activerecord gem and this'd just work.
Please do take a look at adding gem requirements to the generator! It supports gem names only currently, but should definitely allow for more. See
gem_for_database
in Railties: https://github.com/rails/rails/blob/master/railties/lib/rails/generators/app_base.rb#L163-L167. Provide version requirements also and it'll Just Work 😁2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only enforce a minimum version for
pg
; maybe we should do likewise formysql2
?2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tough when a point release actually breaks stuff, though. We could go
~> 0.4
as a middle ground, cover up to 1.0.2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, the point release did NOT break stuff, apparently. According to this fix, mysql2 0.4.0 works fine -- it just wasn't allowed with the 'optional dependency version specification' in the mysql2_adapter.
I think changing the entire way 'optional dependencies' work in AR is probably out of scope here -- apparently AR doesn't want to express explicit dependencies on underlying database gems for each of it's possible databases, or this would indeed just be in the gemspec.
So I have no idea what the right answer is, except that one or the other, remove the upper bound from the 'optional dependency' in mysql2_adapter, OR change the generator to generate the same version specification. I guess I'd lean towards the first, because a generated version spec in your Gemfile is going to be something a lot of people never change, and get locked into old versions without realizing it.
Also out of scope here, probably, is discussing why the heck the
mysql2
gem is still on a pre-1.0 version number -- when it's widely used in production -- instead of being on post-1.0 and committing to semver. If it did that, it would at least make things less confusing.2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd conclude that the best route is to write version deps to Gemfile when apps are generated. Thereafter, app authors are in charge of upgrades, as with any other library.
Please do investigate 😁 See https://github.com/rails/rails/blob/master/railties/lib/rails/generators/app_base.rb#L163-L167 — add current version requirements to the list, test, and you're done.
2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will motivate or instruct future committers making changes to either of these two places, to make sure the two places in code that have a version spec always match?
Especially since mysql2 does not use semver so far as we know, and we have no reason in particular to think 0.5.0 will be backwards incompatible with 0.4.0 (0.4.0 was apparently not backwards incompat with 0.3.0, so we have some reason to think 0.5.0 will be similar...)
...why not just remove the upwards bound on the optional dependency in mysql2_adapter?
There aren't any great solutions here. Anyway, sorry I can not commit to making the PR to copy-paste the version spec to the generator too.
2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For anyone else interested in this little morsel, you can start here. Take a look at how the app generator decides which gem to use for which db: https://github.com/rails/rails/blob/master/railties/lib/rails/generators/app_base.rb#L163-L167
2dfff4d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope you guys realize that every n00b trying to use Rails with MySQL right now is flailing around in complete confusion over this. I know it's not really Rails fault that the mysql2 gem updated it's version, but when I searched for this error, the first 3 Google entries were all completely confused beginners, who don't understand why their "just generated" Rails apps don't work.