Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix error due to Time.at #10686

Closed
wants to merge 1 commit into from

5 participants

@vipulnsward

In changes to fix https://bugs.ruby-lang.org/issues/8173. Time.at now requires an object to implement to_int. Aliasing to_int to to_i fixes this.

@vipulnsward vipulnsward In changes to fix https://bugs.ruby-lang.org/issues/8173. "Time.at" n…
…ow requires an object to implement "to_int". Aliasing "to_int" to "to_i" fixes this.
bbc6162
@likethesky

Thanks @vipulnsward ! Will this be backported to all supported Rails versions? 3.2.x at least (3.1.x as well)? As folks update to Ruby 1.9.3-p429, they will encounter this.

@aselder

+1

Also a big +1 to backport to 3.2-branch.

@pixeltrix
Owner

I'm not sure this is the right fix. The change made to Ruby was to prevent Time objects being treated as integers - this would nullify the effect of the change as AS::TWZ is meant to be compatible with Time. I'll investigate further.

@pixeltrix pixeltrix was assigned
@pixeltrix
Owner

Ugh, it looks as though Time.at checks the type of the first argument to see if it's a Time value and converts it if it does, however the type check explicitly checks that it's not a subclass of Time.

Just blindly adding to_int isn't really the answer as it's saying that TWZ values can be treated like integers all of the time and may have consequences elsewhere. We may have to consider overriding Time.at to convert TWZ values first before calling super.

@vipulnsward

@pixeltrix Right. ruby/ruby@c64f26a had motivated me to make this small fix. There should be a better approach. I hope overriding Time.at won't add an extra overhead.

@aminariana

+10

Ruby 1.9.3-p429 FAIL

I use Rails 3.2.12. As a quick work around, I went down to p392 patch level, in development, which works if anybody is in a rush to get things working while a solution is in the works:

rvm upgrade 1.9.3-p429 1.9.3-p392

though note that Heroku for example uses p429 in its stack, so you'd be broken if you're using ruby 1.9.3 and Rails 3.2.x in Heroku.

@pixeltrix
Owner

Fixed in master: b7f9de2
Fixed in 4-0-stable: 927df2d
Fixed in 4-0-0: 214e377
Fixed in 3-2-stable: f42e0fd

@vipulnsward thanks for reporting

@pixeltrix pixeltrix closed this
@vipulnsward vipulnsward deleted the vipulnsward:fix_time_at branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 19, 2013
  1. @vipulnsward

    In changes to fix https://bugs.ruby-lang.org/issues/8173. "Time.at" n…

    vipulnsward authored
    …ow requires an object to implement "to_int". Aliasing "to_int" to "to_i" fixes this.
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 0 deletions.
  1. +1 −0  activesupport/lib/active_support/time_with_zone.rb
View
1  activesupport/lib/active_support/time_with_zone.rb
@@ -316,6 +316,7 @@ def to_i
utc.to_i
end
alias_method :tv_sec, :to_i
+ alias_method :to_int, :to_i
def to_r
utc.to_r
Something went wrong with that request. Please try again.