[Svnmerge] request for review: #2863 - analyze_source_revs fix

[Dustin J. Mitchell]

On Mon, Aug 06, 2007 at 02:15:48AM +0200, Giovanni Bajo wrote:
> > The code in question is to determine end_rev for supply to analyze_revs.
> > It's the only code to call analyze_revs, and always supplies end_rev.
> > But analyze_rev contains conditional code to deal with the case where
> > end_rev is unknown.  My point was that end_rev is not strictly required,
> > so one possibility is to drop it entirely.  Other possibilities:
> >  - use the target HEAD on single-repository merges and the source head
> >    on multi-repository merges, either with an explicit test in
> >    analyze_source_revs or by improving the caching in get_latest_rev()
> 
> This is what I was suggesting, in fact.

I've attached a patch to do just that.  This patch also fixes a typo in
the RevisionSet constructor which triggered when end_rev was None,
although that code remains unused.  I did remove the support for a
nonexistent end_rev from analyze_revs.

I don't have the birds-eye understanding to know if the
repositories-equal check
  get_repo_root(branch_target) == get_repo_root(source_url)
is going to cause an extra round-trip to the server.  I don't *think* it
will, assuming that branch_target is a working copy.  Can anyone
confirm?

Dustin

-- 
        Dustin J. Mitchell
        Storage Software Engineer, Zmanda, Inc.
        http://www.zmanda.com/
-------------- next part --------------
analyze_source_revs() gets the latest revision of the *branch*
repository, then proceeds to use that value against the *source*
repository; it should get the latest revision of the *source*

(does not depend on other patches in this collection)

Index: svnmerge.py
===================================================================
--- svnmerge.py.orig	2007-08-04 12:37:10.940768026 -0500
+++ svnmerge.py	2007-08-09 10:39:01.409145094 -0500
@@ -377,7 +377,7 @@
             # If end is not provided, we do not know which is the latest
             # revision in the repository. So we set 'end' to the latest
             # known revision.
-            self.end = log.revs[-1]
+            self.end = self.revs[-1]
         else:
             self.end = int(end)
 
@@ -1049,29 +1049,17 @@
     branches), and which revisions initialize merge tracking against other
     branches.  Return a tuple of four RevisionSet's:
         (real_revs, phantom_revs, reflected_revs, initialized_revs).
-
-    NOTE: To maximize speed, if "end" is not provided, the function is
-    not able to find phantom revisions following the last real
-    revision in the URL.
     """
 
     begin = str(begin)
-    if end is None:
-        end = "HEAD"
-    else:
-        end = str(end)
-        if long(begin) > long(end):
-            return RevisionSet(""), RevisionSet(""), RevisionSet(""), RevisionSet("")
+    assert end is not None, "end must be specified"
+    end = str(end)
+    if long(begin) > long(end):
+        return RevisionSet(""), RevisionSet(""), RevisionSet(""), RevisionSet("")
 
     logs[url] = RevisionLog(url, begin, end, find_reflected)
     revs = RevisionSet(logs[url].revs)
 
-    if end == "HEAD":
-        # If end is not provided, we do not know which is the latest revision
-        # in the repository. So return the phantom revision set only up to
-        # the latest known revision.
-        end = str(list(revs)[-1])
-
     phantom_revs = RevisionSet("%s-%s" % (begin, end)) - revs
 
     if find_reflected:
@@ -1089,13 +1077,8 @@
 def analyze_source_revs(branch_target, source_url, **kwargs):
     """For the given branch and source, extract the real and phantom
     source revisions."""
-    branch_url = target_to_url(branch_target)
     branch_pathid = target_to_pathid(branch_target)
 
-    # Extract the latest repository revision from the URL of the branch
-    # directory (which is already cached at this point).
-    end_rev = get_latest_rev(branch_url)
-
     # Calculate the base of analysis. If there is a "1-XX" interval in the
     # merged_revs, we do not need to check those.
     base = 1
@@ -1105,12 +1088,21 @@
 
     # See if the user filtered the revision set. If so, we are not
     # interested in something outside that range.
+    end_rev = None
     if opts["revision"]:
         revs = RevisionSet(opts["revision"]).sorted()
         if base < revs[0]:
             base = revs[0]
-        if end_rev > revs[-1]:
-            end_rev = revs[-1]
+        end_rev = revs[-1]
+
+    # Extract the latest repository revision from the URL of the branch
+    # directory.  If the branch and source both reference the same repository,
+    # then use the source URL instead, since it's cached.
+    if not end_rev:
+        if get_repo_root(branch_target) == get_repo_root(source_url):
+            end_rev = get_latest_rev(source_url)
+        else:
+            end_rev = get_latest_rev(branch_target)
 
     return analyze_revs(branch_pathid, source_url, base, end_rev, **kwargs)