[Svnmerge] request for review: #2862 - cleanup pathid abstraction
Dustin J. Mitchell
dustin at zmanda.com
Fri Aug 3 14:21:45 PDT 2007
This patch cleans up the pathid abstraction in preparation for adding
multiple underlying pathid representations. It passes all of the
existing unit tests, and adds some more, but otherwise shouldn't have
any user-visible ramifications.
I'd love any and all comments/reviews.
Patch is attached and at
http://subversion.tigris.org/nonav/issues/showattachment.cgi/695/cleanup-abstraction.patch
Dustin
--
Dustin J. Mitchell
Storage Software Engineer, Zmanda, Inc.
http://www.zmanda.com/
-------------- next part --------------
clean up the pathid abstraction, removing a few assumptions about the
form of location identifiers
depends on
call-it-pathid.patch
Index: svnmerge.py
===================================================================
--- svnmerge.py.orig 2007-08-03 13:27:34.334393323 -0500
+++ svnmerge.py 2007-08-03 13:27:39.334235658 -0500
@@ -351,7 +351,7 @@
# Look for changes which contain merge tracking information
repos_pathid = target_to_pathid(url)
- srcdir_change_re = re.compile(r"\s*M\s+%s\s+$" % re.escape(repos_pathid))
+ srcdir_change_re = re.compile(r"\s*M\s+%s\s+$" % re.escape(pathid_path(repos_pathid)))
# Setup the log options (--quiet, so we don't show log messages)
log_opts = '--quiet -r%s:%s "%s"' % (begin, end, url)
@@ -764,6 +764,9 @@
return os.path.isdir(os.path.join(dir, ".svn")) or \
os.path.isdir(os.path.join(dir, "_svn"))
+def is_pathid(pathid):
+ return pathid and pathid[0] == '/'
+
_cache_svninfo = {}
def get_svninfo(target):
"""Extract the subversion information for a target (through 'svn info').
@@ -806,10 +809,14 @@
# Try using "svn info URL". This works only on SVN clients >= 1.2
try:
info = get_svninfo(url)
- return info["Repository Root"]
except LaunchError:
pass
+ try:
+ return info["Repository Root"]
+ except KeyError:
+ pass
+
# Constrained to older svn clients, we are stuck with this ugly
# trial-and-error implementation. It could be made faster with a
# binary search.
@@ -874,17 +881,40 @@
def copyfrom_pathid(self):
return getAttributeOrNone(self._node, "copyfrom-path")
+def pathid_to_url(pathid, *targets):
+ """Convert a pathid into a URL. If this is not possible, error out. Extra
+ arguments are any targets the caller knows about, which may be repositories
+ containing the pathid."""
+ if not targets:
+ error("Cannot determine URL for location '%s'; Explicit source "
+ + "argument (-S/--source) required.")
+
+ # append pathid (a path within the repository) to the repostitory root of
+ # the first target found
+ return get_repo_root(targets[0]) + pathid
+
+def equivalent_pathids(pathid1, pathid2, *targets):
+ """Check the equivalency of two pathid's. Extra arguments are any targets
+ the caller knows about, which will be used to qualify any ambiguity in the
+ pathids"""
+ # for repo-relative paths, mere equivalence suffices
+ if pathid1 == pathid2: return True
+
+def pathid_path(pathid):
+ """Get the repository-relative path from a location identifier."""
+ return pathid
+
def get_copyfrom(target):
- """Get copyfrom info for a given target (it represents the directory from
- where it was branched). NOTE: repos root has no copyfrom info. In this case
- None is returned.
+ """Get copyfrom info for a given target (it represents the
+ repository-relative path from where it was branched). NOTE:
+ repos root has no copyfrom info. In this case None is returned.
Returns the:
- source file or directory from which the copy was made
- revision from which that source was copied
- revision in which the copy was committed
"""
- repos_path = target_to_pathid(target)
+ repos_path = pathid_path(target_to_pathid(target))
for chg in SvnLogParser(launchsvn('log -v --xml --stop-on-copy "%s"' % target,
split_lines=False)):
for p in chg.paths():
@@ -1152,9 +1182,17 @@
# will use, since none was provided by the user.
cf_source, cf_rev, copy_committed_in_rev = \
get_copyfrom(opts["source-url"])
- target_path = target_to_pathid(target_dir)
+ target_pathid = target_to_pathid(target_dir)
+
+ if cf_source:
+ cf_url = get_repo_root(opts["source-url"]) + cf_source
+ cf_pathid = target_to_pathid(cf_url)
+ report("'%s' was branched from location '%s'" %
+ (opts["source-url"], cf_pathid))
+ else:
+ cf_pathid = None
- if target_path == cf_source:
+ if equivalent_pathids(target_pathid, cf_pathid, target_dir):
# If source was originally copyied from target, and we are merging
# changes from source to target (the copy target is the merge source,
# and the copy source is the merge target), then we want to mark as
@@ -2055,15 +2093,17 @@
if not cf_source:
error('no copyfrom info available. '
'Explicit source argument (-S/--source) required.')
- opts["source-pathid"] = cf_source
+ opts["source-url"] = get_repo_root(branch_dir) + cf_source
+ opts["source-pathid"] = target_to_pathid(opts["source-url"])
+
if not opts["revision"]:
opts["revision"] = "1-" + cf_rev
else:
opts["source-pathid"] = get_default_source(branch_dir, branch_props)
+ opts["source-url"] = pathid_to_url(opts["source-pathid"], branch_dir)
- # (assumes pathid is a repository-relative-path)
- assert opts["source-pathid"][0] == '/'
- opts["source-url"] = get_repo_root(branch_dir) + opts["source-pathid"]
+ assert is_pathid(opts["source-pathid"])
+ assert is_url(opts["source-url"])
else:
# The source was given as a command line argument and is stored in
# SOURCE. Ensure that the specified source does not end in a /,
@@ -2079,15 +2119,16 @@
if pathid.find(source) > 0:
found.append(pathid)
if len(found) == 1:
- # (assumes pathid is a repository-relative-path)
- source = get_repo_root(branch_dir) + found[0]
+ source_pathid = found[0]
+ source = pathid_to_url(source_pathid, branch_dir)
else:
error('"%s" is neither a valid URL, nor an unambiguous '
'substring of a repository path, nor a working directory' % source)
+ else:
+ source_pathid = target_to_pathid(source)
- source_pathid = target_to_pathid(source)
if str(cmd) == "init" and \
- source_pathid == target_to_pathid("."):
+ equivalent_pathids(source_pathid, target_to_pathid("."), "."):
error("cannot init integration source path '%s'\nIts repository-relative path must "
"differ from the repository-relative path of the current directory." % source_pathid)
opts["source-pathid"] = source_pathid
Index: svnmerge_test.py
===================================================================
--- svnmerge_test.py.orig 2007-08-03 13:27:34.330393450 -0500
+++ svnmerge_test.py 2007-08-03 13:27:39.338235532 -0500
@@ -183,6 +183,17 @@
rs = svnmerge.RevisionSet("")
self.assertEqual(str(rs), "")
+class TestCase_pathid_fns(unittest.TestCase):
+ def test_is_pathid(self):
+ self.assertTrue(svnmerge.is_pathid("/projname/trunk"))
+ self.assertFalse(svnmerge.is_pathid("http://svn.proj.com/projname/trunk"))
+
+ def test_equivalent_pathids(self):
+ self.assertTrue(svnmerge.equivalent_pathids("/projname/trunk", "/projname/trunk"))
+ self.assertFalse(svnmerge.equivalent_pathids("/projname/trunk", "/projname/branches/foo"))
+
+ # pathid_to_url requires a repository; see TestCase_SvnMerge
+
class TestCase_MinimalMergeIntervals(unittest.TestCase):
def test_basic(self):
rs = svnmerge.RevisionSet("4-8,12,18,24")
@@ -1182,6 +1193,22 @@
except AssertionError:
self.assertTrue(os.path.isfile("dir_conflicts.prej"))
+ def test_pathid_fns(self):
+ # (see also TestCase_pathid_fns for tests that don't need a repos)
+ os.chdir("..")
+ self.assertEqual(svnmerge.pathid_to_url( "/branches/testYYY-branch", "./trunk"),
+ "%s/branches/testYYY-branch" % self.test_repo_url)
+
+ self.assertTrue(
+ svnmerge.equivalent_pathids("/branches/testYYY-branch",
+ "/branches/testYYY-branch",
+ "./trunk"))
+
+ self.assertFalse(
+ svnmerge.equivalent_pathids("/branches/test-branch",
+ "/branches/testYYY-branch",
+ "./trunk"))
+
if __name__ == "__main__":
# If an existing template repository and working copy for testing
# exists, then always remove it. This prevents any problems if
More information about the Svnmerge
mailing list