[Svnmerge] [PATCH] fix bugs and tests from r22788

lsuvkne at onemodel.org lsuvkne at onemodel.org
Sat Jun 30 12:24:43 PDT 2007


ps--I'll send later today or Monday another patch that makes the tests run much faster for me--minutes instead of hours. Seems platform-dependent.
-------------- next part --------------
Index: contrib/client-side/svnmerge/svnmerge_test.py
===================================================================
--- contrib/client-side/svnmerge/svnmerge_test.py	(revision 25555)
+++ contrib/client-side/svnmerge/svnmerge_test.py	(working copy)
@@ -499,7 +499,10 @@
     def testBlocked(self):
 
         # Initialize svnmerge
-        self.svnmerge("init")
+        # (Initializing explicitly to versions 1-6; see comments under 
+        # testBasic() for why.)
+        self.svnmerge("init -r 1-6")
+        
         self.launch("svn commit -F svnmerge-commit-message.txt",
                     match=r"Committed revision")
 
@@ -551,7 +554,16 @@
         self.svnmerge("avail -B", match=r"\A$")
 
     def testBasic(self):
-        self.svnmerge("init")
+        # initializing to 1-6 explicitly because this test was written
+        # to verify behavior in the situation where changes on the 
+        # merge source (test-branch) were made after the changes on the merge 
+        # target (trunk); test-branch was created from trunk's r6, and the
+        # changes this tests for came after r6.  An alternative would
+        # be to change the setUp method to create test-branch before
+        # the files are added in trunk, if it is more important to test the
+        # default init behavior here.
+        self.svnmerge("init -r 1-6")
+        
         p = self.getproperty()
         self.assertEqual("/trunk:1-6", p)
 
@@ -695,7 +707,10 @@
 
     def testTrimmedAvailMerge(self):
         """Check that both avail and merge do not search for phantom revs too hard."""
-        self.svnmerge("init")
+        # (Initializing explicitly to versions 1-6; see comments under 
+        # testBasic() for why.)
+        self.svnmerge("init -r 1-6")
+        
         self.svnmerge("avail -vv -r8-9", match=r"svn log.*-r8:9")
         self.svnmerge("merge -F -vv -r8-9", match=r"svn log.*-r8:9")
         self.svnmerge("avail -vv -r2", nonmatch=r"svn log")
@@ -703,7 +718,11 @@
 
     def testMergeRecordOnly(self):
         """Check that flagging revisions as manually merged works."""
-        self.svnmerge("init")
+        """Check that both avail and merge do not search for phantom revs too hard."""
+        # (Initializing explicitly to versions 1-6; see comments under 
+        # testBasic() for why.)
+        self.svnmerge("init -r 1-6")
+        
         self.svnmerge("avail -vv -r9", match=r"svn log.*-r9:9")
         self.svnmerge("merge --record-only -F -vv -r9",
                       nonmatch=r"svn merge -r 8:9")
@@ -737,7 +756,7 @@
         self.launch("svn commit -m \"Change to test1 on trunk\"",
                     match=r"Committed revision 16")
 
-        self.svnmerge("integrated", match=r"^13-14$")
+        self.svnmerge("integrated", match=r"^13$")
 
         os.chdir("..")
         os.chdir("test-branch")
@@ -746,7 +765,7 @@
         self.launch("svn update", match=r"At revision 16")
 
         self.svnmerge("avail -vv --bidirectional", match=r"16$")
-        self.svnmerge("merge -vv --bidirectional", match=r"merge -r 15:16")
+        self.svnmerge("merge -vv --bidirectional", match=r"merge --force -r 15:16")
         p = self.getproperty()
         self.assertEqual("/trunk:1-16", p)
         self.svnmerge("integrated", match=r"^3-16$")
@@ -772,7 +791,7 @@
         # Now check reflected revision is excluded with --bidirectional flag.
         self.svnmerge("avail -vv --bidirectional", match=r"18$")
 
-        self.svnmerge("merge -vv --bidirectional", match=r"merge -r 17:18")
+        self.svnmerge("merge -vv --bidirectional", match=r"merge --force -r 17:18")
         p = self.getproperty()
         self.assertEqual("/branches/test-branch:1-18", p)
 
@@ -834,12 +853,24 @@
 
         # Merge into trunk
         self.svnmerge("merge -vv -S branch2",
-                      match=r"merge -r 18:19")
+                      match=r"merge --force -r 15:19")
+        
+        # Before this file was modified in r22788, trunk was initialized 
+        # above to test-branch2 at r17, so it didn't try to merge in r15 of
+        # test-branch2; since now we initialize trunk to an earlier revision
+        # of test-branch2 (see related changes in this patch) and in r15 
+        # test-branch2 had a property change ('/trunk:1-15',  due 
+        # to its own svnmerge init), the merge just performed tried to bring
+        # over that property change; normally a user might have to 
+        # fix that manually, but since we know that that property change is
+        # not fitting here on trunk, we'll get rid of it:
+        self.launch("svn resolved .", match="Resolved conflicted state of '.'")
+
         p = self.getproperty()
-        self.assertEqual("/branches/test-branch:1-16 /branches/test-branch2:1-19", p)
+        self.assertEqual("/branches/test-branch:1-13 /branches/test-branch2:1-19", p)
 
         self.svnmerge("integrated -S branch2", match=r"^14-19$")
-        self.svnmerge("integrated -S ../test-branch", match=r"^13-16$")
+        self.svnmerge("integrated -S ../test-branch", match=r"^13$")
 
         self.launch("svn commit -F svnmerge-commit-message.txt",
                     match=r"Committed revision 20")
@@ -855,7 +886,7 @@
         # should be available for test-branch with --bidirectional flag.
         self.svnmerge("avail -vv --bidirectional", match=r"20$")
 
-        self.svnmerge("merge -vv --bidirectional", match=r"merge -r 17:20")
+        self.svnmerge("merge -vv --bidirectional", match=r"merge --force -r 17:20")
         p = self.getproperty()
         self.assertEqual("/trunk:1-20", p)
 
Index: contrib/client-side/svnmerge/svnmerge.py
===================================================================
--- contrib/client-side/svnmerge/svnmerge.py	(revision 25555)
+++ contrib/client-side/svnmerge/svnmerge.py	(working copy)
@@ -757,22 +757,79 @@
     assert url[:len(root)] == root, "url=%r, root=%r" % (url, root)
     return url[len(root):]
 
+def get_containing_logentry_xml_text(logText, containedPatternMatchObject):
+    # (get the text of the single logentry xml element surrounding the 
+    # pattern that we previously matched in the caller.  
+    # (Not sure why the
+    # non-greedy quantifiers in a regexp "logEntryContainingFoundAddActionPattern"
+    # didn't seem to work for  this--i.e., putting in '.*?' just before and 
+    # after the mention of "actionAddPathPattern" in the section of
+    # get_copyfrom() right after this is called, caused it to get the 
+    # whole regex which contains many log entries--not just the single 
+    # log entry I want. (todo: use an xml parser instead? no matter for now)
+    start = 0
+    while start != -1:
+        # find the latest occurrence of "<logentry" before the 
+        # preceding pattern match (in effect, search from it backwards
+        # to find its containing element).
+        mightBeStart = logText.find("<logentry", start + 1) # " + 1" to prevent looping forever; this string isn't first in the file, so we won't miss the first instance.
+        if mightBeStart == -1 or mightBeStart > containedPatternMatchObject.start():
+            break
+        start = mightBeStart
+    
+    end = logText.find("</logentry>", containedPatternMatchObject.end()) + 11
+    if start == 0 or end == -1:
+        raise Exception, "Did not find " + \
+                         "element as expected: 'A' log entry for " + \
+                         "'" + repos_path + "' should be surrounded by " + \
+                         "'logentry' xml element, in log text  '" + \
+                         + logText + "'"
+    return logText[start:end]
+
 def get_copyfrom(dir):
     """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."""
+    None is returned.  
+    
+    Returns the:
+        - source file or directory from which the copy was made
+        - the revision from which that source was copied
+        - the revision in which the copy was committed
+    """
     repos_path = target_to_repos_relative_path(dir)
-    out = launchsvn('log -v --xml --stop-on-copy "%s"' % dir,
+    actionAddPattern = r'(<path\s[^>]*action="A"[^>]*>%s</path>)'
+    logText = launchsvn('log -v --xml --stop-on-copy "%s"' % dir,
                     split_lines=False)
-    out = out.replace("\n", " ")
+    logText = logText.replace("\n", " ")
+
+    # first get the source; abort if unavailable (not a copy)
     try:
-        m = re.search(r'(<path\s[^>]*action="A"[^>]*>%s</path>)'
-                      % re.escape(repos_path), out)
-        source = re.search(r'copyfrom-path="([^"]*)"', m.group(1)).group(1)
-        rev = re.search(r'copyfrom-rev="([^"]*)"', m.group(1)).group(1)
-        return source, rev
+        actionAddPatternWithPath = actionAddPattern % re.escape(repos_path)
+        addPathPatternMatch = re.search(actionAddPatternWithPath, logText)
+        source = re.search(r'copyfrom-path="([^"]*)"', \
+                           addPathPatternMatch.group(1)).group(1)
     except AttributeError:
-        return None,None
+        return None,None,None
+    
+    # get the revision from which source was copied (if the above re.search
+    # succeeded, yet this throws an exception, we probably want to know).
+    rev = re.search(r'copyfrom-rev="([^"]*)"', 
+        addPathPatternMatch.group(1)).group(1)
+    
+    # Get the revision in which the copy was committed.
+    logentryElementText = get_containing_logentry_xml_text(logText, addPathPatternMatch)
+    logEntryPatternContainingFoundAddAction = \
+        '\<logentry\s+revision\=\"(\d+)\".*' + \
+        actionAddPatternWithPath + \
+        '.*\</logentry\>'
+    match = re.search(logEntryPatternContainingFoundAddAction, logentryElementText)
+    if match == None:
+        raise Exception, "pattern: " + \
+            logEntryPatternContainingFoundAddAction + " ...not found in " + \
+            logentryElementText
+    copy_committed_in_rev = match.group(1)
+    
+    return source, rev, copy_committed_in_rev
 
 def get_latest_rev(url):
     """Get the latest revision of the repository of which URL is part."""
@@ -1013,43 +1070,44 @@
     else:
         assert False, "unhandled display style: %s" % display_style
 
-def action_init(branch_dir, branch_props):
+def action_init(target_dir, target_props):
     """Initialize a branch for merges."""
     # Check branch directory is ready for being modified
-    check_dir_clean(branch_dir)
-
+    check_dir_clean(target_dir)
+    
     # If the user hasn't specified the revisions to use, see if the
     # "source" is a branch from the current tree and if so, we can use
     # the version data obtained from it.
-    if not opts["revision"]:
-        cf_source, cf_rev = get_copyfrom(opts["source-url"])
-        branch_path = target_to_repos_relative_path(branch_dir)
+    revisionRange = opts["revision"]
+    if not revisionRange:
+        cf_source_path, cf_rev, copy_committed_in_rev = get_copyfrom(opts["source-url"])
+        target_path = target_to_repos_relative_path(target_dir)
 
-        # If the branch_path is the source path of "source",
+        # If the target_path is the same path from which "source" was 
+        # copied when it was created, 
         # then "source" was branched from the current working tree
         # and we can use the revisions determined by get_copyfrom
-        if branch_path == cf_source:
+        if target_path == cf_source_path:
             report('the source "%s" is a branch of "%s"' %
-                   (opts["source-url"], branch_dir))
-            opts["revision"] = "1-" + cf_rev
+                   (opts["source-url"], target_dir))
+            revisionRange = "1-" + copy_committed_in_rev
 
     # Get the initial revision set if not explicitly specified.
-    revs = opts["revision"] or "1-" + get_latest_rev(opts["source-url"])
-    revs = RevisionSet(revs)
+    revisionRange = revisionRange or "1-" + get_latest_rev(opts["source-url"])
+    revs = str(RevisionSet(revisionRange))
 
     report('marking "%s" as already containing revisions "%s" of "%s"' %
-           (branch_dir, revs, opts["source-url"]))
-
-    revs = str(revs)
+       (target_dir, revisionRange, opts["source-url"]))
+        
     # If the source-path already has an entry in the svnmerge-integrated
     # property, simply error out.
-    if not opts["force"] and branch_props.has_key(opts["source-path"]):
+    if not opts["force"] and target_props.has_key(opts["source-path"]):
         error('%s has already been initialized at %s\n'
-              'Use --force to re-initialize' % (opts["source-path"], branch_dir))
-    branch_props[opts["source-path"]] = revs
+              'Use --force to re-initialize' % (opts["source-path"], target_dir))
+    target_props[opts["source-path"]] = revs
 
     # Set property
-    set_merge_props(branch_dir, branch_props)
+    set_merge_props(target_dir, target_props)
 
     # Write out commit message if desired
     if opts["commit-file"]:
@@ -1899,11 +1957,11 @@
     report("calculate source path for the branch")
     if not source:
         if str(cmd) == "init":
-            cf_source, cf_rev = get_copyfrom(branch_dir)
-            if not cf_source:
+            cf_source_path, cf_rev, copy_committed_in_rev = get_copyfrom(branch_dir)
+            if not cf_source_path:
                 error('no copyfrom info available. '
                       'Explicit source argument (-S/--source) required.')
-            opts["source-path"] = cf_source
+            opts["source-path"] = cf_source_path
             if not opts["revision"]:
                 opts["revision"] = "1-" + cf_rev
         else:


More information about the Svnmerge mailing list