anatoly techtonik wrote:
>> -__version__ = '2012.05.24'
>
> Because svnlook.py is used in hook scripts, it is usually copied
> out of revision control. Version is useful to track new changes.
That makes sense. I'm not sure what to put in the __version__ string, but the
most common way to identify the version of other scripts in the 'tools'
directory in our repository seems to be by adding these three lines:
# $HeadURL$
# $LastChangedDate$
# $LastChangedRevision$
Would you be happy with this? I've committed that in r1541878, but am happy to
change it.
>> @@ -76,7 +74,7 @@ class SVNLook(object):
>> self.txn_ptr = None
>> if rev is None:
>> rev = fs.youngest_rev(self.fs_ptr)
>> - self.rev = int(rev)
>> + self.rev = rev
>
> I believe that this is critical for some API calls and arithmetic that
> self.rev is not a string.
OK, so this is basically for the convenience of callers that might want to pass
a string. I'll do it like this:
# if set, txn takes precendence
if txn:
self.txn_ptr = fs.open_txn(self.fs_ptr, txn)
else:
self.txn_ptr = None
if rev is None:
rev = fs.youngest_rev(self.fs_ptr)
+ else:
+ rev = int(rev)
self.rev = rev
Also related to revision numbers, I noticed that the 'changed' and
'dirs-changed' and 'diff' commands would fail with rev=0, because they tried to
set base_rev=-1. To fix those, I'll add:
def _walk_tree(self, e_factory, base_rev=None, pass_root=0, callback=None):
if base_rev is None:
# a specific base rev was not provided. use the transaction base,
# or the previous revision
if self.txn_ptr:
base_rev = fs.txn_base_revision(self.txn_ptr)
+ elif self.rev == 0:
+ base_rev = 0
else:
base_rev = self.rev - 1
(With the current code, a 'return' statement would be equivalent, but this way
avoids making assumptions.)
I committed these two changes (and a tweak to the doc string) in revision
1541873.
>> def cmd_date(self):
>> - # duplicate original svnlook format, which is
>> - # 2010-02-08 21:53:15 +0200 (Mon, 08 Feb 2010)
>> secs = self.get_date(unixtime=True)
>> - # convert to tuple, detect time zone and format
>> - stamp = time.localtime(secs)
>> - isdst = stamp.tm_isdst
>> - utcoffset = -(time.altzone if (time.daylight and isdst) else
> time.timezone) // 60
>> -
>> - suffix = "%+03d%02d" % (utcoffset // 60, abs(utcoffset) % 60)
>> - outstr = time.strftime('%Y-%m-%d %H:%M:%S ', stamp) + suffix
>> - outstr += time.strftime(' (%a, %d %b %Y)', stamp)
>> - print(outstr)
>> + if secs is None:
>> + print("")
>> + else:
>> + # assume secs in local TZ, convert to tuple, and format
>> + ### we don't really know the TZ, do we?
>> + print(time.strftime('%Y-%m-%d %H:%M', time.localtime(secs)))
>
> I believe that svnlook.py output was different than of svnlook, but I may
> mistake. Anyway, this change seems ok.
You are right that the output was (and is) different. I am happy to change the
format to be the same as 'svnlook'); the only problems were (1) it was an
unrelated change and (2) it broke if a revision had no date property.
I have added "if secs is None: print('')" to fix the latter, and committed it
as revision 1541877.
>> def get_date(self, unixtime=False):
>> """return commit timestamp in RFC 3339 format
> (2010-02-08T20:37:25.195000Z)
>> if unixtime is True, return unix timestamp
>> - return None if date is not set (txn properties)
>> + return None for a txn, or if date property is not set
>
> What are other cases when date may be empty?
It's possible for a committed revision to not have a svn:date property.
Although the property is always created on commit, it can be removed by "svn
propdel --revprop -r X svn:date".
> Anyway, LGTM without any more fixes.
Thanks. Let me know if you would like any more changes.
- Julian
