Fix Unmarshaling of Date objects marshaled with MRI 1.9+ #878

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@dynamix
dynamix commented Jul 11, 2013

A Date object marshaled with MRI 1.9+ is not unmarshaled correctly in JRuby at the moment. If you don't look to close it seems to be OK but comparing a Date object instantiated in JRuby and a Date object unmarshaled from a MRI marshaled object fails as the @ajd variable is not set correctly. The new format is a six element array and the old one was a three element array.

To see how the marshaling code has changed have a look at https://github.com/ruby/ruby/blob/ruby_1_9_3/ext/date/date_core.c#L7242-L7264

Example:

# MRI irb
require 'date'
Date.today
# => #<Date: 2013-07-11 ((2456485j,0s,0n),+0s,2299161j)>
str = Marshal.dump(Date.today)
# => "\x04\bU:\tDate[\vi\x00i\x03\xA5{%i\x00i\x00i\x00f\f2299161"
Marshal.load(str) == Date.today
# => true

# JRuby irb
require 'date'
str = "\x04\bU:\tDate[\vi\x00i\x03\xA5{%i\x00i\x00i\x00f\f2299161"
date = Marshal.load(str)
# => #<Date: 2013-07-11 (0,2456485,0)>
date == Date.today
# => false
date.to_s
# => "2013-07-11"
Date.today.to_s
# => "2013-07-11"

This patch might not be 100% correct but fixes the problem.

@BanzaiMan
Member

Do we have specs in RubySpec (which we are currently tagging)? If not, do you mind coming up with them?

@dynamix
dynamix commented Jul 12, 2013

I will have a look.

@enebo
Member
enebo commented Jul 12, 2013

I believe that @eregon is going to work on the native Date implementation soonish? I personally don't have a big problem with this patch but we generally try and preserve parity with MRI's stdlib as much as possible since we need to maintain and merge these deltas at patchlevels change.

@eregon
Member
eregon commented Jul 12, 2013

@enebo I will try to take care of that. Many tests not based directly on Julian Day constructors and sub-millisecond precision pass right now.

Current problem is creating a Joda-time DateTime from a Julian Day. This seems rather undocumented and not trivial. I am posting an issue there.

@eregon
Member
eregon commented Jul 18, 2013

@dynamix Seems there is no RubySpec for this, could you add one?

@eregon
Member
eregon commented Sep 19, 2013

Should be fixed in ae6bb5e.
Note it is not yet merged to master though.

Other values of the Array are needed to be taken in account for DateTime so the diff is quite different.

@eregon eregon added a commit to eregon/jruby that referenced this pull request Sep 19, 2013
@eregon eregon handle all knowns formats in Date#marshal_load ae6bb5e
@eregon eregon closed this in cc2fd27 Sep 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment