Blair Zajac wrote:
>
> On May 22, 2008, at 6:29 AM, Mark Phippard wrote:
>
>> On Thu, May 22, 2008 at 1:36 AM, Hyrum K. Wright
>> <hyrum_wright_at_mail.utexas.edu> wrote:
>>> Mark Phippard wrote:
>>>>
>>>> I have a question on recent JavaHL change merged to 1.5.x branch.
>>>>
>>>> In the logMessageCallback we now return a Map of revprops instead of
>>>> the individual revprops. svn:date is returned as the date value.
>>>> This forces us to add code like the following to get the timeMicros:
>>>>
>>>>
>>>> // Really hacky date parser, because Java doesn't support
>>>> // microseconds natively.
>>>> try {
>>>> DateFormat formatter = new SimpleDateFormat(
>>>> "yyyy-MM-dd'T'HH:mm:ss.SSS");
>>>> String datestr = ((String) revprops.get("svn:date"));
>>>> Date date = formatter.parse(datestr.substring(0, 23));
>>>> Calendar cal = Calendar.getInstance();
>>>> cal.setTime(date);
>>>> timeMicros = cal.getTimeInMillis();
>>>>
>>>> timeMicros = timeMicros * 1000
>>>> + Integer.parseInt(datestr.substring(23,
>>>> 26));
>>>>
>>>> I thought the timeMicros were only added in 1.5 to be a more efficient
>>>> way to pass the date along. Or perhaps it was the extra precision.
>>>> Either way, the efficiency and precision are both gone now. It seems
>>>> like we should either:
>>>>
>>>> a) pass the timeMicros as an explicit parameter to the callback
>>>> b) store it in the revprops. Either as the value of svn:date or as a
>>>> made up revprop like svn:timemicros.
>>>> c) forget it altogether and just pass along the date as is
>>>
>>> I don't recall when timeMicros was added (but the fact that I needed to
>>> parse the svn:date property for compat probably means it wasn't in 1.5).
>>
>> Check the branch:
>>
>> http://svn.collab.net/repos/svn/branches/1.4.x/subversion/bindings/java/javahl/src/org/tigris/subversion/javahl/LogMessage.java
>>
>>
>> timeMicros is not there.
>>
>>> As
>>> for your suggestions, I'm opposed to a) on the grounds that we
>>> shouldn't be
>>> treating svn:date as special in this callback. We could do something
>>> like
>>> b) relatively easy (though I'd prefer to do it as an additional
>>> property,
>>> not as svn:date). c) would be easiest, and ideal if it still provides
>>> consumers what they need.
>>
>> So a possible option c) would be to remove the timeMicros stuff
>> altogether and just go back to having a Date. We can wait for Blair
>> to comment. It seems to me that if the original goal was efficiency
>> we definitely no longer have that.
>
> No, the goal was to not have lossy dates. I believe if you get the
> date/timestamp from the 1.4 bindings for a revision and then lookup the
> revision with it, the revision will be - 1 less then the original
> revision, so its lossy.
Does the command line client have this same problem? It would seem that
all the date information is recoverable from svn:date. What part is
being lost?
> I am -1 on removing the time micros, I am +1 on putting timeMicros back
> in as an explicit paramter and -0 on the hacky parameter name.
>
> I think the best solution is to put back explicit parameters for all our
> supported svn:* revprops and use the map for anything else. The C++
> code can easily remove the svn:date, svn:log and svn:author from the map.
True, that it's easy for the C++ code to filter out svn:* revprops, but
I don't see why we should. Getting the property values from a Map isn't
difficult, and it's much more extensible than passing values back
directly. Also, I'd like to keep the JavaHL interface from drifting to
far away from the C interface.
-Hyrum
Received on 2008-05-22 17:10:15 CEST