You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mark Phippard <ma...@gmail.com> on 2008/05/22 00:04:46 UTC
Question on JavaHL change
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 am adding Blair to the cc: as I recall he added the original
timeMicros feature for his own needs.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by Blair Zajac <bl...@orcaware.com>.
Mark,
I'll take a look at this tomorrow morning.
Regards,
Blair
On May 21, 2008, at 5:04 PM, 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 am adding Blair to the cc: as I recall he added the original
> timeMicros feature for his own needs.
>
> --
> Thanks
>
> Mark Phippard
> http://markphip.blogspot.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by Mark Phippard <ma...@gmail.com>.
We went a slightly different direction, see trunk.
Sent from my iPhone
On May 23, 2008, at 6:02 PM, Blair Zajac <bl...@orcaware.com> wrote:
> Mark Phippard wrote:
>> On Fri, May 23, 2008 at 9:15 AM, Mark Phippard <ma...@gmail.com>
>> wrote:
>>> On Fri, May 23, 2008 at 8:55 AM, Mark Phippard
>>> <ma...@gmail.com> wrote:
>>>> The LogDate idea is a good solution. Do we think we can get it
>>>> implemented in time for the release? If not, I still think we
>>>> should
>>>> just back out these changes and do this in 1.6.
>>> I have attached a patch which adds a LogDate class and uses it in
>>> SVNClient. The C++ would need to be changed to store a LogDate
>>> object
>>> in the Map for the svn:date entry.
>> Attaching a second patch. This adds a test case for the LogDate
>> class. This patch just includes the test.
>
>
> The LogDate looks good. A couple of comments.
>
> Does JDK < 1.5 support annotations?
>
> I think getDate should be synchronized.
>
> In this code
>
> + LogDate date = (LogDate) revprops.get("svn:date");
> + if (date == null) date = new LogDate(0);
>
> That assumes revprops will return a LogDate for the svn:date
> property? And people have to cast it? Seems clumsy. I like a
> stronger interface where people don't have to do casts when possible.
>
> Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by Blair Zajac <bl...@orcaware.com>.
Blair Zajac wrote:
> Mark Phippard wrote:
>> On Fri, May 23, 2008 at 9:15 AM, Mark Phippard <ma...@gmail.com>
>> wrote:
>>> On Fri, May 23, 2008 at 8:55 AM, Mark Phippard <ma...@gmail.com>
>>> wrote:
>>>> The LogDate idea is a good solution. Do we think we can get it
>>>> implemented in time for the release? If not, I still think we should
>>>> just back out these changes and do this in 1.6.
>>> I have attached a patch which adds a LogDate class and uses it in
>>> SVNClient. The C++ would need to be changed to store a LogDate object
>>> in the Map for the svn:date entry.
>>
>> Attaching a second patch. This adds a test case for the LogDate
>> class. This patch just includes the test.
>>
>>
>
>
> The LogDate looks good. A couple of comments.
>
> Does JDK < 1.5 support annotations?
>
> I think getDate should be synchronized.
>
> In this code
>
> + LogDate date = (LogDate) revprops.get("svn:date");
> + if (date == null) date = new LogDate(0);
>
> That assumes revprops will return a LogDate for the svn:date property?
> And people have to cast it? Seems clumsy. I like a stronger interface
> where people don't have to do casts when possible.
Nevermind, I wrote this email while offline and hadn't seen the latest commits.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by Blair Zajac <bl...@orcaware.com>.
Mark Phippard wrote:
> On Fri, May 23, 2008 at 9:15 AM, Mark Phippard <ma...@gmail.com> wrote:
>> On Fri, May 23, 2008 at 8:55 AM, Mark Phippard <ma...@gmail.com> wrote:
>>> The LogDate idea is a good solution. Do we think we can get it
>>> implemented in time for the release? If not, I still think we should
>>> just back out these changes and do this in 1.6.
>> I have attached a patch which adds a LogDate class and uses it in
>> SVNClient. The C++ would need to be changed to store a LogDate object
>> in the Map for the svn:date entry.
>
> Attaching a second patch. This adds a test case for the LogDate
> class. This patch just includes the test.
>
>
The LogDate looks good. A couple of comments.
Does JDK < 1.5 support annotations?
I think getDate should be synchronized.
In this code
+ LogDate date = (LogDate) revprops.get("svn:date");
+ if (date == null) date = new LogDate(0);
That assumes revprops will return a LogDate for the svn:date property? And
people have to cast it? Seems clumsy. I like a stronger interface where people
don't have to do casts when possible.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, May 23, 2008 at 9:15 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Fri, May 23, 2008 at 8:55 AM, Mark Phippard <ma...@gmail.com> wrote:
>> The LogDate idea is a good solution. Do we think we can get it
>> implemented in time for the release? If not, I still think we should
>> just back out these changes and do this in 1.6.
>
> I have attached a patch which adds a LogDate class and uses it in
> SVNClient. The C++ would need to be changed to store a LogDate object
> in the Map for the svn:date entry.
Attaching a second patch. This adds a test case for the LogDate
class. This patch just includes the test.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Question on JavaHL change
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, May 23, 2008 at 8:55 AM, Mark Phippard <ma...@gmail.com> wrote:
> The LogDate idea is a good solution. Do we think we can get it
> implemented in time for the release? If not, I still think we should
> just back out these changes and do this in 1.6.
I have attached a patch which adds a LogDate class and uses it in
SVNClient. The C++ would need to be changed to store a LogDate object
in the Map for the svn:date entry.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Question on JavaHL change
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, May 23, 2008 at 2:35 AM, Hyrum K. Wright
<hy...@mail.utexas.edu> wrote:
>> Looking more closely at the code:
>>
>> 1) Only the java.util.Date is lossy, it only represents times to the
>> millisecond, so I put in the extra methods in LogMessage so you can get the
>> time in micros without doing any extra work.
>
> Would it be possible to have a LogDate class which extends java.util.Date
> (and can therefore be passed around as such), but which isn't lossy?
This sounds like a great idea. Make a LogDate class that you get from
the Map when you ask for svn:date. Internally, this class could just
store the timeMicros as a long and then have getter methods for
getting the date as a long, Date or String.
The point about LogMessage being deprecated is good. I had also
forgotten that we ask for the revprops we want. So I think all of
your comments make sense. My concern was just the current code
passing the date as a String, which is then parsed to a Date so that
we can turn it into a long and then the consumers of LogMessage have
all of that happen in reverse. It is ugly.
The LogDate idea is a good solution. Do we think we can get it
implemented in time for the release? If not, I still think we should
just back out these changes and do this in 1.6.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Blair Zajac wrote:
> C. Michael Pilato wrote:
>> Hyrum K. Wright wrote:
>>>>> 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?
>>
>> Yeah, I'm interested in this, too.
>>
>>>> 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.
>>
>> I don't use the Java bindings directly, but +1 on the sentiment of
>> keeping them from drifting unnecessarily from the C interface. I
>> don't see how returning svn:date's value asis would be "lossy". I
>> mean, if it is, it would seem we certainly couldn't remedy that
>> client-side. Why not just keep Hyrum's changes, but provide a handle
>> utility function that converts Subversion dates into timeMicros?
>
> Looking more closely at the code:
>
> 1) Only the java.util.Date is lossy, it only represents times to the
> millisecond, so I put in the extra methods in LogMessage so you can get
> the time in micros without doing any extra work.
Would it be possible to have a LogDate class which extends
java.util.Date (and can therefore be passed around as such), but which
isn't lossy?
> 2) The LogMessage class is already not similar to the C interface in
> that it provides a strongly typed date as a java.util.Date. My work
> just extended it to add a long microseconds values.
See note farther down about LogMessage being deprecated.
> 3) The current LogMessage class doesn't have the non-svn revision
> properties in it, which I believe should be the real point of this
> work. Looking at the current interface, it only exposes the svn primary
> properties through methods (getAuthor, getMessage, etc), there's no way
> to get the map of other properties.
>
> 4) I don't see the point of moving the svn:date string to time parsing
> from the C code to the Java code, it's probably slower.
Higher level languages are usually easier to maintain. Generally, I've
tried to keep any JavaHL specific stuff in the highest layer possible.
That's why all the compat wrappers are written in Java, instead of C++.
Also, I don't think the speed increase would be noticeable, but I
haven't run any tests, have you?
> So I think we can have the best of both worlds.
>
> 1) LogMessageCallback can have its String author, long timeMicros,
> String message arguments returned and keep the of Map of revprops as an
> argument. Put the time conversion back in the C code and remove the
> time parsing from the Java code.
>
> 2) LogMessage can grow a Map<String,String> field and a
> getRevisionProperties() method for getting all revision properties.
LogMessage is (or should be) deprecated, since it is only used by
deprecated APIs. We shouldn't be adding anything to it.
> We can either remove svn:date, svn:author and svn:log from the revprops
> map or keep them in. This is a style issue. Maybe keeping them all in
> is better? This seems more consistent with the meaning of the map???
> And the meaning of the C code where the map has all properties.
>
> So we can take Mark's patch and revert parts of the C++ code in r31273
> to get the desired code.
If we do include explicit parameters to the callback method, what would
we return as svn:author, svn:date, and svn:log if the caller doesn't
include those in the list of revprops to be fetched? The logMessages()
interface is now an arbitrary revprop request mechanism, not just a way
of getting log messages, which is a good reason for not giving the svn:*
revprops special treatment.
It looks like the svn:date value is the major problem here. If we are
concerned about date parsing, let's just provide a utility function for
interested clients and call it good.
-Hyrum
Re: Question on JavaHL change
Posted by Blair Zajac <bl...@orcaware.com>.
C. Michael Pilato wrote:
> Hyrum K. Wright wrote:
>>>> 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?
>
> Yeah, I'm interested in this, too.
>
>>> 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.
>
> I don't use the Java bindings directly, but +1 on the sentiment of
> keeping them from drifting unnecessarily from the C interface. I don't
> see how returning svn:date's value asis would be "lossy". I mean, if it
> is, it would seem we certainly couldn't remedy that client-side. Why
> not just keep Hyrum's changes, but provide a handle utility function
> that converts Subversion dates into timeMicros?
Looking more closely at the code:
1) Only the java.util.Date is lossy, it only represents times to the
millisecond, so I put in the extra methods in LogMessage so you can get the time
in micros without doing any extra work.
2) The LogMessage class is already not similar to the C interface in that it
provides a strongly typed date as a java.util.Date. My work just extended it to
add a long microseconds values.
3) The current LogMessage class doesn't have the non-svn revision properties in
it, which I believe should be the real point of this work. Looking at the
current interface, it only exposes the svn primary properties through methods
(getAuthor, getMessage, etc), there's no way to get the map of other properties.
4) I don't see the point of moving the svn:date string to time parsing from the
C code to the Java code, it's probably slower.
So I think we can have the best of both worlds.
1) LogMessageCallback can have its String author, long timeMicros, String
message arguments returned and keep the of Map of revprops as an argument. Put
the time conversion back in the C code and remove the time parsing from the Java
code.
2) LogMessage can grow a Map<String,String> field and a getRevisionProperties()
method for getting all revision properties.
We can either remove svn:date, svn:author and svn:log from the revprops map or
keep them in. This is a style issue. Maybe keeping them all in is better?
This seems more consistent with the meaning of the map??? And the meaning of
the C code where the map has all properties.
So we can take Mark's patch and revert parts of the C++ code in r31273 to get
the desired code.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by "C. Michael Pilato" <cm...@collab.net>.
Hyrum K. Wright wrote:
>>> 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?
Yeah, I'm interested in this, too.
>> 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.
I don't use the Java bindings directly, but +1 on the sentiment of keeping
them from drifting unnecessarily from the C interface. I don't see how
returning svn:date's value asis would be "lossy". I mean, if it is, it
would seem we certainly couldn't remedy that client-side. Why not just keep
Hyrum's changes, but provide a handle utility function that converts
Subversion dates into timeMicros?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Question on JavaHL change
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
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
>> <hy...@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
Re: Question on JavaHL change
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, May 22, 2008 at 10:14 AM, Blair Zajac <bl...@orcaware.com> wrote:
>
> On May 22, 2008, at 6:29 AM, Mark Phippard wrote:
>
>> On Thu, May 22, 2008 at 1:36 AM, Hyrum K. Wright
>> <hy...@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.
>
> 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.
I have attached a patch for the Java side of this proposal. Obviously
the C++ changes are the hard part and I have no idea how to do it. I
agree with the concept here though.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Question on JavaHL change
Posted by Blair Zajac <bl...@orcaware.com>.
On May 22, 2008, at 6:29 AM, Mark Phippard wrote:
> On Thu, May 22, 2008 at 1:36 AM, Hyrum K. Wright
> <hy...@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.
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.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, May 22, 2008 at 1:36 AM, Hyrum K. Wright
<hy...@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.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Question on JavaHL change
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
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).
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.
-Hyrum