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

4-2-stable + ruby 2.4 #27473

Merged
merged 16 commits into from Jan 3, 2017
Merged

4-2-stable + ruby 2.4 #27473

merged 16 commits into from Jan 3, 2017

Conversation

@matthewd
Copy link
Member

@matthewd matthewd commented Dec 27, 2016

No description provided.

matthewd and others added 14 commits Dec 27, 2016
…table

Make `SchemaMigration.drop_table` to one SQL
Ruby 2.4 introduces `Array#sum`, but it only supports numeric elements,
breaking our `Enumerable#sum` which supports arbitrary `Object#+`.
To fix, override `Array#sum` with our compatible implementation.

Native Ruby 2.4:

    %w[ a b ].sum
    # => TypeError: String can't be coerced into Fixnum

With `Enumerable#sum` shim:

    %w[ a b ].sum
    # => 'ab'

We tried shimming the fast path and falling back to the compatible path
if it fails, but that ends up slower even in simple causes due to the cost
of exception handling. Our only choice is to override the native `Array#sum`
with our `Enumerable#sum`.
Fix AJ tests on ruby 2.4 being caused since classes are unified for Integer
Fixnum and Bignum are deprecated in Ruby trunk
Use built-in #transform_values when available.
…rom-ruby-24

Use Hash#compact and Hash#compact! from Ruby 2.4
…lues

Ensure `#transform_values` of HWIDA to return HWIDA
BigDecimal('an invalid string') has changed its behavior to raise an ArgumentError since 1.3.0
https://bugs.ruby-lang.org/issues/10286
change return value of `duplicable?` with Ruby 2.4+
Fix Fixnum deprecated warning in Ruby 2.4+
Very similar to PR #25758, see more in depth reasoning there.
Revise the "XML is not HTML" test
@rails-bot
Copy link

@rails-bot rails-bot commented Dec 27, 2016

warning Warning warning

  • Pull requests are usually filed against the master branch for this repo, but this one is against 4-2-stable. Please double check that you specified the right target!
They're not good, but they're not what these tests care about.
@matthewd matthewd force-pushed the matthewd:4-2-2-4 branch Dec 27, 2016
@matthewd
Copy link
Member Author

@matthewd matthewd commented Dec 27, 2016

@pixeltrix do you have any thoughts on the smallest / safest / least back-branch-disruptive subset of c9c5788 & friends that would be sensible to backport?

I guess we're only behaving differently (either potentially slower, or potentially cachier, or both) if the configured preserve_timezone doesn't match the intrinsic behaviour of the running version, right? So preserve_timezone=false on pre-2.4 Rubies is zero cost?

This is a bit over-constraining for the committed lock file, but Travis
ignores that anyway.
@pixeltrix
Copy link
Member

@pixeltrix pixeltrix commented Jan 1, 2017

I guess we're only behaving differently (either potentially slower, or potentially cachier, or both) if the configured preserve_timezone doesn't match the intrinsic behaviour of the running version, right? So preserve_timezone=false on pre-2.4 Rubies is zero cost?

Nearly zero - there's still the overhead of checking the config option at runtime. We could change it so that we rewrite the method at boot time, I guess.

As for backporting I think we should be able to do it fairly cleanly - IIRC there's a couple of gotchas but I can have a look at it this week.

@@ -1,5 +1,7 @@
source 'https://rubygems.org'

ruby "~> #{RUBY_VERSION}" if ENV["TRAVIS"]

This comment has been minimized.

@pixeltrix

pixeltrix Jan 3, 2017
Member

Does rvm on Travis not blow up on this line? See rvm/rvm#3705

This comment has been minimized.

@matthewd

matthewd Jan 3, 2017
Author Member

I think that only matters if rvm is trying to parse the ruby version from the Gemfile.

If it's causing local problems, we can probably just obfuscate it a bit so rvm doesn't see it at all (e.g., make it a send).

This comment has been minimized.

@pixeltrix

pixeltrix Jan 3, 2017
Member

Is there an option to turn parsing of the Gemfile off?

@matthewd matthewd force-pushed the matthewd:4-2-2-4 branch to fa61939 Jan 3, 2017
@matthewd
Copy link
Member Author

@matthewd matthewd commented Jan 3, 2017

Merging ahead of #27553, which will get the build green again.

@matthewd matthewd merged commit 158a856 into rails:4-2-stable Jan 3, 2017
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
jrafanie added a commit to jrafanie/fast_gettext that referenced this pull request May 8, 2017
jrafanie added a commit to jrafanie/fast_gettext that referenced this pull request May 8, 2017
jrafanie added a commit to jrafanie/fast_gettext that referenced this pull request May 8, 2017
jrafanie added a commit to jrafanie/parallel that referenced this pull request May 8, 2017
@matthewd matthewd deleted the matthewd:4-2-2-4 branch Jun 12, 2017
nikolai-b added a commit to cyclestreets/cyclescape-chef that referenced this pull request Nov 20, 2017
rails/rails#27473
and other things make ruby 2.4 a bit too much work
@notapatch notapatch mentioned this pull request Jan 14, 2020
3 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants