[Svnmerge] request for review: #2869

Dustin J. Mitchell dustin at zmanda.com
Thu Aug 9 10:36:43 PDT 2007


I rolled up the cleanup-abstraction patch (see "request for review:
#2862 - cleanup pathid abstraction") with the actual use of the
abstraction, to produce pathids-combined.patch, which is attached.

I'd love feedback on this!

Dustin

-- 
        Dustin J. Mitchell
        Storage Software Engineer, Zmanda, Inc.
        http://www.zmanda.com/
-------------- next part --------------
[[[
* svnmerge.py
  (PathIdentifier): new class
  (RevisionLog.__init__): extract repo_relative_path from pathid
  (dict_from_revlist_prop): recognize string forms of pathids
  (format_merge_props): use '%s' to force pathids to strings
  (is_url): add an optional validity check for URLs
  (is_pathid, pathid_hint, target_to_pathid): new functions
  (get_copyfrom): extract repo_relative_path from pathid, use '%s'
    to force pathids to strings
  (check_old_prop_version): removed
  (action_init): use pathids
  (action_integrated, action_rollback): remove call to 
    check_old_prop_version
  (common_opts, command_table): new options
  (main): use pathids, set up hints
* svnmerge_test.py:
  (TestCase_PathIdentifier): new test class
  (TestCase_SvnMerge.svnmerge2): clear new caches between runs
  (testUninitForce, testBidirectionalMergesMultiBranch): allow properties
    to come back in arbitrary order
  (test_dict_from_revlist_prop, test_pathid_fns, test_pathid_hint_url,
    test_is_pathid): new tests
* svnmerge.README:
  describe new functionality
]]]
Index: svnmerge.py
===================================================================
--- svnmerge.py.orig	2007-08-09 12:14:40.993142399 -0500
+++ svnmerge.py	2007-08-09 12:16:56.005855776 -0500
@@ -27,6 +27,8 @@
 #   Blair Zajac <blair at orcaware dot com> - random improvements
 #   Raman Gupta <rocketraman at fastmail dot fm> - bidirectional and transitive merging
 #     support
+#   Dustin J. Mitchell <dustin at zmanda dot com> - support for multiple
+#     location identifier formats
 #
 # $HeadURL: https://svn.collab.net/repos/svn/trunk/contrib/client-side/svnmerge/svnmerge.py $
 # $LastChangedDate: 2007-08-03 11:59:44 -0500 (Fri, 03 Aug 2007) $
@@ -53,6 +55,7 @@
 # - Add --force option to skip working copy check
 # - Add --record-only option to "svnmerge merge" to avoid performing
 #   an actual merge, yet record that a merge happened.
+# - Can use a variety of location-identifier formats
 #
 # TODO:
 #  - Add "svnmerge avail -R": show logs in reverse order
@@ -328,6 +331,88 @@
     if out and out[0].strip():
         error('"%s" has local modifications; it must be clean' % dir)
 
+class PathIdentifier:
+    """Abstraction for a path identifier, so that we can start talking
+    about it before we know the form that it takes in the properties (its
+    external_form).  Objects are referenced in the global variable 'locobjs',
+    keyed by all known forms."""
+    def __init__(self, repo_relative_path, uuid=None, url=None, external_form=None):
+        self.repo_relative_path = repo_relative_path
+        self.uuid = uuid
+        self.url = url
+        self.external_form = external_form
+
+    def __repr__(self):
+        return "<PathIdentifier " + ', '.join('%s=%r' % i for i in self.__dict__.items()) + '>'
+
+    def __str__(self):
+        """Return a printable string representation"""
+        if self.external_form:
+            return self.external_form
+        if self.url:
+            return self.format('url')
+        if self.uuid:
+            return self.format('uuid')
+        return self.format('path')
+
+    def __eq__(self, other):
+        return self is other
+
+    def __ne__(self, other):
+        return self is not other
+
+    def __hash__(self):
+        return id(self)
+
+    def format(self, fmt):
+        if fmt == 'path':
+            return self.repo_relative_path
+        elif fmt == 'uuid':
+            return "uuid://%s%s" % (self.uuid, self.repo_relative_path)
+        elif fmt == 'url':
+            return self.url
+        else:
+            error("Unkonwn path type '%s'" % fmt)
+
+    def match_substring(self, str):
+        """Test whether str is a substring of any representation of this
+        PathIdentifier."""
+        if self.repo_relative_path.find(str) >= 0:
+            return True
+
+        if self.uuid:
+            if ("uuid://%s%s" % (self.uuid, self.repo_relative_path)).find(str) >= 0:
+                return True
+
+        if self.url:
+            if (self.url + self.repo_relative_path).find(str) >= 0:
+                return True
+
+        return False
+
+    def get_url(self):
+        """Convert a pathid into a URL.  If this is not possible, error out."""
+        if self.url:
+            return self.url
+        # if we have a uuid and happen to know the URL for it, use that
+        elif self.uuid and repo_hints.has_key(self.uuid):
+            self.url = repo_hints[self.uuid] + self.repo_relative_path
+            locobjs[self.url] = self
+            return self.url
+        # if we've only seen one rep, use that (a guess, but an educated one)
+        elif not self.uuid and len(repo_hints) == 1:
+            uuid, root = repo_hints.items()[0]
+            if uuid:
+                self.uuid = uuid
+                locobjs['uuid://%s%s' % (uuid, self.repo_relative_path)] = self
+            self.url = root + self.repo_relative_path
+            locobjs[self.url] = self
+            report("Guessing that '%s' refers to '%s'" % (self, self.url))
+            return self.url
+        else:
+            error("Cannot determine URL for '%s'; " % self +
+                  "Explicit source argument (-S/--source) required.\n")
+
 class RevisionLog:
     """
     A log of the revisions which affected a given URL between two
@@ -351,7 +436,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(repos_pathid.repo_relative_path))
 
         # Setup the log options (--quiet, so we don't show log messages)
         log_opts = '--quiet -r%s:%s "%s"' % (begin, end, url)
@@ -669,8 +754,35 @@
     # Multiple sources are separated by any whitespace.
     for L in propvalue.split():
         # We use rsplit to play safe and allow colons in pathids.
-        source, revs = rsplit(L.strip(), ":", 1)
-        prop[source] = revs
+        pathid_str, revs = rsplit(L.strip(), ":", 1)
+
+        # convert pathid_str to a PathIdentifier
+        if not locobjs.has_key(pathid_str):
+            if is_url(pathid_str):
+                # we can determine every form; pathid_hint knows how to do that
+                pathid_hint(pathid_str)
+            elif pathid_str[:7] == 'uuid://':
+                mo = re.match('uuid://([^/]*)(.*)', pathid_str)
+                if not mo:
+                    error("Invalid path identifier '%s'" % pathid_str)
+                uuid, repo_relative_path = mo.groups()
+                pathid = PathIdentifier(repo_relative_path, uuid=uuid)
+                # we can cache this by uuid:// pathid and by repo-relative path
+                locobjs[pathid_str] = locobjs[repo_relative_path] = pathid
+            elif pathid_str and pathid_str[0] == '/':
+                # strip any trailing slashes
+                pathid_str = pathid_str.rstrip('/')
+                pathid = PathIdentifier(repo_relative_path=pathid_str)
+                # we can only cache this by repo-relative path
+                locobjs[pathid_str] = pathid
+            else:
+                error("Invalid path identifier '%s'" % pathid_str)
+        pathid = locobjs[pathid_str]
+
+        # cache the "external" form we saw
+        pathid.external_form = pathid_str
+
+        prop[pathid] = revs
     return prop
 
 def get_revlist_prop(url_or_dir, propname, rev=None):
@@ -711,7 +823,7 @@
     props.sort()
     L = []
     for h, r in props:
-        L.append(h + ":" + r)
+        L.append("%s:%s" % (h, r))
     return sep.join(L)
 
 def _run_propset(dir, prop, value):
@@ -755,15 +867,21 @@
         del props[source_pathid]
     set_block_props(dir, props)
 
-def is_url(url):
+def is_url(url, check_valid=False):
     """Check if url is a valid url."""
-    return re.search(r"^[a-zA-Z][-+\.\w]*://", url) is not None
+    if re.search(r"^[a-zA-Z][-+\.\w]*://", url) is not None and url[:4] != 'uuid':
+        if not check_valid: return True
+        return get_svninfo(url, fail_on_invalid=False) != {}
+    return False
 
 def is_wc(dir):
     """Check if a directory is a working copy."""
     return os.path.isdir(os.path.join(dir, ".svn")) or \
            os.path.isdir(os.path.join(dir, "_svn"))
 
+def is_pathid(pathid):
+    return isinstance(pathid, PathIdentifier)
+
 _cache_svninfo = {}
 def get_svninfo(target, fail_on_invalid=True):
     """Extract the subversion information for a target (through 'svn info').
@@ -832,14 +950,60 @@
 
     error("svn repos root of %s not found" % target)
 
-def target_to_pathid(target):
-    """Convert a target (either a working copy path or an URL) into a
-    path identifier."""
-    root = get_repo_root(target)
+# a global cache of PathIdentifier instances, keyed by all pathids by
+# which they might be known.  This dictionary is primed by pathid_hint(),
+# and further adjusted as queries against it are performed.
+locobjs = {}
+
+# a map of UUID (or None) to repository root URL.
+repo_hints = {}
+
+def pathid_hint(target):
+    """Cache some information about target, as it may be referenced by
+    repo-relative path in subversion properties; the cache can help to
+    expand such a relative path to a full path identifier."""
+    if locobjs.has_key(target): return
+    if not is_url(target) and not is_wc(target): return
+
     url = target_to_url(target)
+
+    root = get_repo_root(url)
     assert root[-1] != "/"
     assert url[:len(root)] == root, "url=%r, root=%r" % (url, root)
-    return url[len(root):]
+    repo_relative_path = url[len(root):]
+
+    try:
+        uuid = get_svninfo(target, fail_on_invalid=False)['Repository UUID']
+        uuid_pathid = 'uuid://%s%s' % (uuid, repo_relative_path)
+    except KeyError:
+        uuid = None
+        uuid_pathid = None
+
+    locobj = locobjs.get(url) or \
+             (uuid_pathid and locobjs.get(uuid_pathid))
+    if not locobj:
+        locobj = PathIdentifier(repo_relative_path, uuid=uuid, url=url)
+
+    repo_hints[uuid] = root # (uuid may be None)
+
+    locobjs[target] = locobj
+    locobjs[url] = locobj
+    if uuid_pathid:
+        locobjs[uuid_pathid] = locobj
+    if not locobjs.has_key(repo_relative_path):
+        locobjs[repo_relative_path] = locobj
+
+def target_to_pathid(target):
+    """Convert a target (either a working copy path or an URL) into a
+    path identifier."""
+    # prime the cache first if we don't know about this target yet
+    if not locobjs.has_key(target):
+        pathid_hint(target)
+
+    try:
+        return locobjs[target]
+    except KeyError:
+        error("Could not recognize path identifier '%s'" % target)
 
 def getAttributeOrNone(element, name):
     '''Returns None if not found.'''
@@ -884,16 +1048,16 @@
                 return getAttributeOrNone(self._node, "copyfrom-path")
 
 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 = target_to_pathid(target).repo_relative_path
     for chg in SvnLogParser(launchsvn('log -v --xml --stop-on-copy "%s"' % target,
                                       split_lines=False)):
         for p in chg.paths():
@@ -988,35 +1152,11 @@
         err_msg += "Explicit source argument (-S/--source) required.\n"
         err_msg += "The merge sources available are:"
         for prop in props:
-          err_msg += "\n  " + prop
+          err_msg += "\n  " + str(prop)
         error(err_msg)
 
     return props.keys()[0]
 
-def check_old_prop_version(branch_target, branch_props):
-    """Check if branch_props (of branch_target) are svnmerge properties in
-    old format, and emit an error if so."""
-
-    # Previous svnmerge versions allowed trailing /'s in the repository
-    # local path.  Newer versions of svnmerge will trim trailing /'s
-    # appearing in the command line, so if there are any properties with
-    # trailing /'s, they will not be properly matched later on, so require
-    # the user to change them now.
-    fixed = {}
-    changed = False
-    for source, revs in branch_props.items():
-        src = rstrip(source, "/")
-        fixed[src] = revs
-        if src != source:
-            changed = True
-
-    if changed:
-        err_msg = "old property values detected; an upgrade is required.\n\n"
-        err_msg += "Please execute and commit these changes to upgrade:\n\n"
-        err_msg += 'svn propset "%s" "%s" "%s"' % \
-                   (opts["prop"], format_merge_props(fixed), branch_target)
-        error(err_msg)
-
 def analyze_revs(target_pathid, url, begin=1, end=None,
                  find_reflected=False):
     """For the source of the merges in the source URL being merged into
@@ -1144,58 +1284,80 @@
     # Check that directory is ready for being modified
     check_dir_clean(target_dir)
 
+    target_pathid = target_to_pathid(target_dir)
+    source_pathid = opts['source-pathid']
+    if source_pathid == target_pathid:
+        error("cannot init integration source path '%s'\nIts path identifier does not "
+              "differ from the path identifier of the current directory, '%s'."
+              % (source_pathid, target_pathid))
+
+    source_url = opts['source-url']
+
     # If the user hasn't specified the revisions to use, see if the
     # "source" is a copy from the current tree and if so, we can use
     # the version data obtained from it.
     revision_range = opts["revision"]
     if not revision_range:
-        # Determining a default endpoint for the revision range that "init"
-        # 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)
-
-        if target_path == cf_source:
-            # 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 
-            # integrated up to the rev in which the copy was committed which 
-            # created the merge source:
-            report('the source "%s" is a branch of "%s"' %
-                   (opts["source-url"], target_dir))
+        # If source was originally copied 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
+        # integrated up to the rev in which the copy was committed which
+        # created the merge source:
+        cf_source, cf_rev, copy_committed_in_rev = get_copyfrom(source_url)
+
+        cf_pathid = None
+        if cf_source:
+            cf_url = get_repo_root(source_url) + cf_source
+            if is_url(cf_url, check_valid=True):
+                cf_pathid = target_to_pathid(cf_url)
+
+        if target_pathid == cf_pathid:
+            report('the source "%s" was copied from "%s" in rev %s and committed in rev %s' %
+                   (source_url, target_dir, cf_rev, copy_committed_in_rev))
             revision_range = "1-" + copy_committed_in_rev
-        else:
-            # If the copy source is the merge source, and 
-            # the copy target is the merge target, then we want to
-            # mark as integrated up to the specific rev of the merge
-            # target from which the merge source was copied.  (Longer
-            # discussion at:
-            # http://subversion.tigris.org/issues/show_bug.cgi?id=2810  )
-            target_url = target_to_url(target_dir)
-            source_path = target_to_pathid(opts["source-url"])
-            cf_source_path, cf_rev, copy_committed_in_rev = get_copyfrom(target_url)
-            if source_path == cf_source_path:
-                report('the merge source "%s" is the copy source of "%s"' %
-                       (opts["source-url"], target_dir))
-                revision_range = "1-" + cf_rev
+
+    if not revision_range:
+        # If the reverse is true: copy source is the merge source, and
+        # the copy target is the merge target, then we want to mark as
+        # integrated up to the specific rev of the merge target from
+        # which the merge source was copied.  (Longer discussion at:
+        # http://subversion.tigris.org/issues/show_bug.cgi?id=2810  )
+        cf_source, cf_rev, copy_committed_in_rev = get_copyfrom(target_dir)
+
+        cf_pathid = None
+        if cf_source:
+            cf_url = get_repo_root(target_dir) + cf_source
+            if is_url(cf_url, check_valid=True):
+                cf_pathid = target_to_pathid(cf_url)
+
+        source_pathid = target_to_pathid(source_url)
+        if source_pathid == cf_pathid:
+            report('the target "%s" was copied the source "%s" in rev %s and committed in rev %s' %
+                   (target_dir, source_url, cf_rev, copy_committed_in_rev))
+            revision_range = "1-" + cf_rev
 
     # When neither the merge source nor target is a copy of the other, and 
     # the user did not specify a revision range, then choose a default which is 
     # the current revision; saying, in effect, "everything has been merged, so 
     # mark as integrated up to the latest rev on source url).
-    revs = revision_range or "1-" + get_latest_rev(opts["source-url"])
-    revs = RevisionSet(revs)
+    if not revision_range:
+        revision_range = "1-" + get_latest_rev(source_url)
+
+    revs = RevisionSet(revision_range)
 
     report('marking "%s" as already containing revisions "%s" of "%s"' %
-           (target_dir, revs, opts["source-url"]))
+           (target_dir, revs, source_url))
 
-    revs = str(revs)
     # If the local svnmerge-integrated property already has an entry 
     # for the source-pathid, simply error out.
-    if not opts["force"] and target_props.has_key(opts["source-pathid"]):
+    if not opts["force"] and target_props.has_key(source_pathid):
         error('Repository-relative path %s has already been initialized at %s\n'
-              'Use --force to re-initialize' % (opts["source-pathid"], target_dir))
-    target_props[opts["source-pathid"]] = revs
+              'Use --force to re-initialize' % (source_pathid, target_dir))
+    # set the pathid's external_form based on the user's options
+    source_pathid.external_form = source_pathid.format(opts['location-type'])
+
+    revs = str(revs)
+    target_props[source_pathid] = revs
 
     # Set property
     set_merge_props(target_dir, target_props)
@@ -1205,7 +1367,7 @@
         f = open(opts["commit-file"], "w")
         print >>f, 'Initialized merge tracking via "%s" with revisions "%s" from ' \
             % (NAME, revs)
-        print >>f, '%s' % opts["source-url"]
+        print >>f, '%s' % source_url
         f.close()
         report('wrote commit message to "%s"' % opts["commit-file"])
 
@@ -1247,7 +1409,6 @@
     creation revision."""
     # Extract the integration info for the branch_dir
     branch_props = get_merge_props(branch_dir)
-    check_old_prop_version(branch_dir, branch_props)
     revs = merge_props_to_revision_set(branch_props, opts["source-pathid"])
 
     # Lookup the oldest revision on the branch path.
@@ -1431,7 +1592,6 @@
 
     # Extract the integration info for the branch_dir
     branch_props = get_merge_props(branch_dir)
-    check_old_prop_version(branch_dir, branch_props)
     # Get the list of all revisions already merged into this source-pathid.
     merged_revs = merge_props_to_revision_set(branch_props,
                                               opts["source-pathid"])
@@ -1853,9 +2013,9 @@
     OptionArg("-S", "--source", "--head",
               default=None,
               help="specify a merge source for this branch.  It can be either "
-                   "a path, a full URL, or an unambiguous substring of one "
-                   "of the paths for which merge tracking was already "
-                   "initialized.  Needed only to disambiguate in case of "
+                   "a working directory path, a full URL, or an unambiguous "
+                   "substring of one of the locations for which merge tracking was "
+                   "already initialized.  Needed only to disambiguate in case of "
                    "multiple merge sources"),
 ]
 
@@ -1875,6 +2035,12 @@
     the branch point (unless you teach it with --revision).""" % NAME,
     [
         "-f", "-r", # import common opts
+        OptionArg("-L", "--location-type",
+               dest="location-type",
+               default="path",
+               help="Use this type of location identifier in the new " +
+                    "Subversion properties; 'uuid', 'url', or 'path' " +
+                    "(default)"),
     ]),
 
     "avail": (action_avail,
@@ -2044,9 +2210,12 @@
     if not is_wc(branch_dir):
         error('"%s" is not a subversion working directory' % branch_dir)
 
+    # give out some hints as to potential pathids
+    pathid_hint(branch_dir)
+    if source: pathid_hint(source)
+
     # Extract the integration info for the branch_dir
     branch_props = get_merge_props(branch_dir)
-    check_old_prop_version(branch_dir, branch_props)
 
     # Calculate source_url and source_path
     report("calculate source path for the branch")
@@ -2056,15 +2225,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"] = opts["source-pathid"].get_url()
 
-        # (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 /,
@@ -2077,20 +2248,17 @@
             # within the branch properties.
             found = []
             for pathid in branch_props.keys():
-                if pathid.find(source) > 0:
+                if pathid.match_substring(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 = source_pathid.get_url()
             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("."):
-            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
         opts["source-url"] = target_to_url(source)
 
Index: svnmerge_test.py
===================================================================
--- svnmerge_test.py.orig	2007-08-09 12:14:40.993142399 -0500
+++ svnmerge_test.py	2007-08-09 12:16:56.013855581 -0500
@@ -183,6 +183,42 @@
         rs = svnmerge.RevisionSet("")
         self.assertEqual(str(rs), "")
 
+class TestCase_PathIdentifier(unittest.TestCase):
+    rrp = "/trunk/contrib/client-side/svnmerge"
+    uuid = "65390229-12b7-0310-b90b-f21a5aa7ec8e"
+    uuidrl = 'uuid://'+uuid+rrp
+    url= "http://svn.collab.net/repos/svn/trunk/contrib/client-side/svnmerge"
+    ext = "uuid://65390229-12b7-0310-b90b-f21a5aa7ec8e/trunk/contrib/client-side/svnmerge"
+    def try_pathid(self, rrp, uuid, url, ext, expected_str, expected_formats):
+        l = svnmerge.PathIdentifier(rrp, uuid, url, ext)
+        self.assertEqual(str(l), expected_str,
+            "str() gave '%s' instead of '%s'" % (str(l), expected_str))
+        for k, v in expected_formats.items():
+            self.assertEqual(l.format(k), v,
+                "format('%s') gave '%s' instead of '%s'" % (k, l.format(k), v))
+        svnmerge.locobjs = {}
+        svnmerge.repo_hints = {}
+
+    def test_PathIdentifer_just_path(self):
+        self.try_pathid(self.rrp, None, None, None,
+                self.rrp, { 'path' : self.rrp })
+
+    def test_PathIdentifer_uuid(self):
+        self.try_pathid(self.rrp, self.uuid, None, None,
+                self.uuidrl, { 'path' : self.rrp, 'uuid' : self.uuidrl })
+
+    def test_PathIdentifer_url(self):
+        self.try_pathid(self.rrp, None, self.url, None,
+                self.url, { 'path' : self.rrp, 'url' : self.url })
+
+    def test_PathIdentifer_prefer_url(self):
+        self.try_pathid(self.rrp, self.uuid, self.url, None,
+                self.url, { 'path' : self.rrp, 'url' : self.url, 'uuid' : self.uuidrl })
+
+    def test_PathIdentifer_external_form(self):
+        self.try_pathid(self.rrp, self.uuid, self.url, self.ext,
+                self.ext, { 'path' : self.rrp, 'url' : self.url, 'uuid' : self.uuidrl })
+
 class TestCase_MinimalMergeIntervals(unittest.TestCase):
     def test_basic(self):
         rs = svnmerge.RevisionSet("4-8,12,18,24")
@@ -199,9 +235,11 @@
         sys.stdout = sys.stderr = out
         try:
             try:
-                # Clear svnmerge's internal cache before running any
+                # Clear svnmerge's internal caches before running any
                 # commands.
                 svnmerge._cache_svninfo = {}
+                svnmerge.locobjs = {}
+                svnmerge.repo_hints = {}
 
                 ret = svnmerge.main(args)
             except SystemExit, e:
@@ -735,7 +773,9 @@
                     match=r"Committed revision")
 
         p = self.getproperty()
-        self.assertTrue(re.search("/branches/testYYY-branch:1-\d+? /trunk:1-\d+?", p))
+        # properties come back in arbitrary order, so search for them individually
+        self.assertTrue(re.search("/branches/testYYY-branch:1-\d+?", p))
+        self.assertTrue(re.search("/trunk:1-\d+?", p))
 
         open("test1", "a").write("foo")
 
@@ -942,7 +982,10 @@
         self.svnmerge("merge -vv -S branch2",
                       match=r"merge --force -r 18:19")
         p = self.getproperty()
-        self.assertEqual("/branches/test-branch:1-16 /branches/test-branch2:1-19", p)
+
+        # allow properties to come back in arbitrary order
+        self.assertTrue(re.search("/branches/test-branch2:1-19", p))
+        self.assertTrue(re.search("/branches/test-branch:1-16", p))
 
         self.svnmerge("integrated -S branch2", match=r"^14-19$")
         self.svnmerge("integrated -S ../test-branch", match=r"^13-16$")
@@ -1198,6 +1241,48 @@
         finally:
             svnmerge.error = olderror
 
+    def test_dict_from_revlist_prop(self):
+        locdict = svnmerge.dict_from_revlist_prop("/trunk:1-10 uuid://65390229-12b7-0310-b90b-f21a5aa7ec8e/branches/foo:20-30")
+        for k, v in locdict.items():
+            if str(k) == '/trunk':
+                self.assertEqual(str(v), '1-10')
+            elif str(k) == 'uuid://65390229-12b7-0310-b90b-f21a5aa7ec8e/branches/foo':
+                self.assertEqual(str(v), '20-30')
+            else:
+                self.fail("Unknown pathid '%s'" % k)
+
+    def test_pathid_fns(self):
+        os.chdir("..")
+
+        # run svnmerge once to get things rolling
+        self.svnmerge("init", error=True)
+
+        branch = svnmerge.target_to_pathid("test-branch")
+        trunk = svnmerge.target_to_pathid("trunk")
+        yy = svnmerge.target_to_pathid("%s/branches/testYYY-branch" % self.test_repo_url)
+        branchurl = svnmerge.target_to_pathid("%s/branches/test-branch" % self.test_repo_url)
+        trunkurl = svnmerge.target_to_pathid("%s/trunk" % self.test_repo_url)
+
+        self.assertTrue(branchurl == branch)
+        self.assertFalse(branchurl != branch)
+        self.assertTrue(trunkurl == trunk)
+        self.assertTrue(yy != trunk)
+
+    def test_pathid_hint_url(self):
+        os.chdir("..")
+        # prime the cache with our repo URL
+        svnmerge.pathid_hint(self.test_repo_url + '/trunk')
+        expected = svnmerge.locobjs['/trunk']
+
+        # and then we should get the same pathid for all of these
+        self.assertEqual(expected, svnmerge.target_to_pathid('trunk'))
+        self.assertEqual(expected, svnmerge.target_to_pathid(self.test_repo_url + '/trunk'))
+
+    def test_is_pathid(self):
+        os.chdir("..")
+        l = svnmerge.target_to_pathid('trunk')
+        self.assertTrue(svnmerge.is_pathid(l))
+
 if __name__ == "__main__":
     # If an existing template repository and working copy for testing
     # exists, then always remove it.  This prevents any problems if
Index: svnmerge.README
===================================================================
--- svnmerge.README.orig	2007-08-09 12:14:40.993142399 -0500
+++ svnmerge.README	2007-08-09 12:16:56.013855581 -0500
@@ -26,3 +26,9 @@
   * "svnmerge avail" has grown two new options:
     -B to display a list of the blocked revisions.
     -A to display both the blocked and the available revisions.
+
+  * The Subverson properties used for tracking can represent a branch
+    in one of several ways:
+    - path (the old repo-relative path; the default)
+    - uuid (uuid://XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX/repo/relative/path
+    - url


More information about the Svnmerge mailing list