[patch] commit timestamp incorrectly cast if specified

[John A Meinel]

Jamie Wilkinson wrote:
> I found an interesting quirk whilst testing a plugin that iterates over
> revisions in a branch.
> 
> The pseudocode looks like:
> 
>   rev_a = branch_a.get_revision(revno)
> 
>   branch_b.commit(rev_a.message, rev_a.timestamp, rev_a.timezone, ...)
> 
>   rev_b = branch_b.get_revision(branch_b.last_revision())
> 
>   assertEqual(rev_a.timestamp, rev_b.timestamp)
> 
> and fails because rev_b.timestamp appears to be truncated.
> 
> commit.py:219 reads thusly:
> 
>         if timestamp is None:
>             self.timestamp = time.time()
>         else:
>             self.timestamp = long(timestamp)
> 
> pydoc time.time tells me that time.time() returns a floating point number.
> 
> I think the last line above shouldn't be casting to long.
> 
> I've got a test for this in my branch, revno 1536, and a patch to fix it in
> revno 1537:
> 
>   http://repo.spacepants.org/bzr/bzr.jaq
> 
> (This URL has changed since my last patch.)

I agree. I'm not sure why we would have used long(), maybe it makes the
test suite easier to run.
Considering we specifically print the floating point timestamp to 9
digits of precision. We definitely should not be casting it to a long first.

I know we had discussed breaking it up into a seconds + milliseconds
(micro?) field, so we wouldn't have to worry about floating point
rounding issues. Maybe it was an attempt to switch, or maybe it was old
cruft that never really got sorted out.

gannotate and bzrk show that the change happened when Commit() was
refactored into a class, which was when we introduced weaves. So it has
been around a long time. Maybe Martin remembers why he changed it.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060117/7b2036da/attachment.pgp