On Mar 3, 2006, at 10:37 AM, firstname.lastname@example.org wrote:
> Author: djames
> Date: Fri Mar 3 12:37:43 2006
> New Revision: 18701
> Refactor svnmerge.py to cache logs, mergeprops and blockprops when
> bidirectional support is enabled. This refactoring helps prepare us
> for efficiently eliminating spurious property conflicts.
> +from bisect import bisect
Is bisect distributed with Python 2.0? I'm without net access as I
write this, so can't check, but I see my Python 2.3 has it.
> + def __init__(self, url, begin, end, find_propchanges=False):
> + """
> + Create a new RevisionLog object, which stores, in
> self.revs, a list
> + of the revisions which affected the specified URL between
> begin and
> + end. If find_propchanges is True, self.propchange_revs
> will contain a
> + list of the URLs which changed properties directly on the
List of the *revisions* which changed properties.
> + # Read the log to look for revision numbers and merge-
> tracking info
> + self.revs = 
> + self.propchange_revs = 
> + for line in launchsvn("log %s" % log_opts):
> + m = revision_re.match(line)
> + if m:
> + rev = int(m.groups())
> + self.revs.append(rev)
> + elif srcdir_change_re.match(line):
> + self.propchange_revs.append(rev)
I guess we're counting upon a line to match ^r before finding a line
that matches the M line, otherwise we'll append a None to
propchange_revs? Isn't it better Python style to set rev to None at
the top of the loop?
> +class VersionedProperty:
> + """A read-only, cached view of a versioned property"""
> + def __init__(self, url, name):
> + """View the history of a versioned property at URL with
Maybe rename 'name' to 'propname', to be a little clearer.
Could you also add an explanation of the method you're using here
(such as bisect) which will explain why revs is set to  and values
to [None] initially.
> + def get(self, rev=None):
> + """
> + Get the property at revision 'rev'. If rev is not
> specified, get
> + the property at 'head'.
> + """
> + if rev is not None:
> + i = bisect(self.revs, rev) - 1
> + if self.values[i] is not None:
> + return self.values[i]
> + return self.raw_get(rev)
Why not insert the values returned by raw_get using the result from
bisect() so you don't have to do another call raw_get() again?
You've already got the i value you need, so mine as well use it.
> + def keys(self):
> + """
> + Get a list of the revisions in which this property changed.
> + """
> + return self.revs[1:-1]
When I see a method names keys(), I don't normally expect an ordered
list to be returned. So later down in the code, when I see it being
used, I wonder if it needs to be sorted.
Can we come up with a better fitting name, such as changed_revs,
which matches the documentation string.
> @@ -671,8 +729,12 @@
> if long(begin) > long(end):
> return RevisionSet(""), RevisionSet(""), RevisionSet("")
> - revs, merges = find_changes(url, begin, end, find_reflected)
> - revs = RevisionSet(",".join(map(str,revs)))
> + logs[url] = RevisionLog(url, begin, end, find_reflected)
> + mergeprops[url] = VersionedProperty(url, opts["prop"])
> + mergeprops[url].load(logs[url])
> + blockprops[url] = VersionedProperty(url, opts["block_prop"])
> + blockprops[url].load(logs[url])
> + revs = RevisionSet(",".join(map(str,logs[url].revs)))
Are you iterating over the block_prop property, which wasn't done
before, to prevent a property conflict on it?
I don't see any code currently using that, so can we remove it to
help improve performance?
BTW, now that logs, mergeprops and blockprops are global, we could
pickle them into file in ~/.subversion/svnmerge.py dictionary keyed
by the repository's UUID, so we can save lookups in the future.
Blair Zajac, Ph.D.
Subversion training, consulting and support
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Mon Mar 6 01:11:55 2006