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

Negative nsec on Time initialisation #5438

Closed
sebastianguarin opened this Issue Nov 12, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@sebastianguarin
Copy link

commented Nov 12, 2018

Environment

  • jRuby 9.2.3.0
  • Rails 4.2.9
  • Darwin 14.5.0 Darwin Kernel Version 14.5.0: Sun Jun 4 21:40:08 PDT 2017; root:xnu-2782.70.3~1/RELEASE_X86_64 x86_64
    TimeZone AEDT

Expected Behavior

  • Keep nano seconds that were given on the args
pry(main)> ::Time.utc(1969, 11, 12, 13, 18, 57, 404240).nsec
=> 404240000

Actual Behavior

jruby-9.2.3.0 :001 > ::Time.utc(1969, 11, 12, 13, 18, 57, 404240).nsec
 => -595760000

I got this issue while running some specs that generate random past dates, which are failing due https://github.com/jruby/jruby/pull/5371/files

(49.years + 1.days).ago

Traceback (most recent call last):
       16: from /Users/sebastianm/.rvm/rubies/jruby-9.2.3.0/lib/ruby/gems/shared/gems/railties-4.2.10/lib/rails/commands/console.rb:110:in `start'
       15: from org/jruby/RubyKernel.java:1181:in `catch'
       14: from org/jruby/RubyKernel.java:1181:in `catch'
       13: from org/jruby/RubyKernel.java:1415:in `loop'
       12: from org/jruby/RubyKernel.java:1043:in `eval'
       11: from (irb):6:in `evaluate'
       10: from /Users/sebastianm/.rvm/rubies/jruby-9.2.3.0/lib/ruby/gems/shared/gems/activesupport-4.2.10/lib/active_support/duration.rb:109:in `ago'
        9: from /Users/sebastianm/.rvm/rubies/jruby-9.2.3.0/lib/ruby/gems/shared/gems/activesupport-4.2.10/lib/active_support/duration.rb:134:in `sum'
        8: from org/jruby/RubyEnumerable.java:1047:in `inject'
        7: from org/jruby/RubyArray.java:1789:in `each'
        6: from /Users/sebastianm/.rvm/rubies/jruby-9.2.3.0/lib/ruby/gems/shared/gems/activesupport-4.2.10/lib/active_support/duration.rb:139:in `block in sum'
        5: from /Users/sebastianm/.rvm/rubies/jruby-9.2.3.0/lib/ruby/gems/shared/gems/activesupport-4.2.10/lib/active_support/time_with_zone.rb:294:in `advance'
        4: from /Users/sebastianm/.rvm/rubies/jruby-9.2.3.0/lib/ruby/gems/shared/gems/activesupport-4.2.10/lib/active_support/time_with_zone.rb:383:in `method_missing'
        3: from /Users/sebastianm/.rvm/rubies/jruby-9.2.3.0/lib/ruby/gems/shared/gems/activesupport-4.2.10/lib/active_support/core_ext/time/calculations.rb:135:in `advance'
        2: from /Users/sebastianm/.rvm/rubies/jruby-9.2.3.0/lib/ruby/gems/shared/gems/activesupport-4.2.10/lib/active_support/core_ext/time/calculations.rb:102:in `change'
        1: from org/jruby/RubyTime.java
@headius

This comment has been minimized.

Copy link
Member

commented Nov 12, 2018

This does not appear to be a new issue...versions as far back as 1.7.27 behave the same way.

@headius

This comment has been minimized.

Copy link
Member

commented Nov 12, 2018

Ok to clarify...

The behavior described here is not new as far back as 1.7.27. The error is new, but correct behavior for negative nsec (assuming MRI would raise at the same place if passed negative nsec).

So the thing to fix is not the error added by #5371, but to figure out why we're returning a negative nsec in these cases.

headius added a commit to headius/jruby that referenced this issue Nov 12, 2018

Time#nsec should produce a positive result always.
The original code was using dt.getMillis which will produce a
native value for dates before the epoch, causing the final
calculation of nsecs to be both native and of the wrong magnitude.
The correct method to use is dt.getMillisOfSecond, which will
provide a proper positive millis value even before the epoch.

Fixes jruby#5438.

headius added a commit to headius/jruby that referenced this issue Nov 12, 2018

headius added a commit that referenced this issue Nov 12, 2018

Time#nsec should always be positive (#5440)
* Time#nsec should produce a positive result always.

The original code was using dt.getMillis which will produce a
native value for dates before the epoch, causing the final
calculation of nsecs to be both native and of the wrong magnitude.
The correct method to use is dt.getMillisOfSecond, which will
provide a proper positive millis value even before the epoch.

Fixes #5438.

* Add specs for positive nsec and usec for pre-epoch dates.

See #5438.

@enebo enebo added this to the JRuby 9.2.4.0 milestone Nov 13, 2018

eregon added a commit to ruby/spec that referenced this issue Nov 27, 2018

Time#nsec should always be positive (jruby/jruby#5440)
* Time#nsec should produce a positive result always.

The original code was using dt.getMillis which will produce a
native value for dates before the epoch, causing the final
calculation of nsecs to be both native and of the wrong magnitude.
The correct method to use is dt.getMillisOfSecond, which will
provide a proper positive millis value even before the epoch.

Fixes jruby/jruby#5438.

* Add specs for positive nsec and usec for pre-epoch dates.

See jruby/jruby#5438.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.