Hello,

it's been a while I have been thinking of modifying launchsvn() so that it
returns an opened pipe, instead of running the program and catching the whole
output at once. This can have a couple of benefits:

- It provides a way to parallelize operations and thus gaining speed benefits
when the SVN connection time is the bottleneck. For instance, the attached
patch constructs the commit log message by running numerous parallel "svn log"
processes to get the error message, and the gain in speed is enormous.
- It gives a little more feedback to the user. For instance, the actual svn
merge output is shown on the screen while the merge is being done, instead of
all at once at the end.
- I received a bugreport of pipe overflow, which I can't wrap my head against.
Basically, on the GCC repository, when doing merges of old branches, the
initial "svn log --quiet HEAD" call (which is used to find available revisions)
sometimes hit some internal limits (of either Python or kernel, I don't know),
and aborts with an overflow error. Don't ask, but I reproduced it once under
Linux. The suggested fix I was presented changed the os.popen() call with
subprocess (the new Python 2.4 module), and that managed to work around the
problem. I believe that if we read from the pipe incrementally (by also making
good use of the XML format of svn log and thus avoiding parsing errors) the
problem should also be fixed, without having to use the subprocess module.

I have attached a preliminar patch to this mail. Right now, I have just added a
new launchsvn_pipe and used it in a few places. If there's agreement that this
is the way to go, I'll complete the patch.

Comments are welcome!

Giovanni Bajo
Index: svnmerge.py
===================================================================
--- svnmerge.py (revision 19653)
+++ svnmerge.py (working copy)
@@ -201,6 +201,54 @@ class LaunchError(Exception):
     exit code of the process, the original command line, and the output of the
     command."""
 
+def launch_pipe(cmd):
+    if os.name not in ['nt', 'os2']:
+        class Pipe:
+            def __init__(self, cmd):
+                self.p = popen2.Popen4(cmd)
+                self.p.tochild.close()
+            def __iter__(self):
+                return self
+            def next(self):
+                L = self.p.fromchild.readline()
+                if not L:
+                    self._checkError()
+                   raise StopIteration
+                return L
+            def read(self):
+                data = self.p.fromchild.read()
+                self._checkError()
+                return data
+            def _checkError(self):
+                ret = self.p.wait()
+                if ret != 0:
+                    raise LaunchError(ret)
+    else:
+        class Pipe:
+            def __init__(self, cmd):
+                i,k = os.popen4(cmd)
+                i.close()
+                self.k = k
+            def __iter__(self):
+                return self
+            def next(self):
+                L = self.k.readline()
+                if not L:
+                    self._checkError()
+                    raise StopIteration
+                return L
+            def read(self):
+                data = self.k.read()
+                self._checkError()
+                return data
+            def _checkError(self):
+                ret = self.k.close()
+                if ret is not None:
+                    raise LaunchError(ret)
+
+    return Pipe(cmd)
+
+
 def launch(cmd, split_lines=True):
     """Launch a sub-process. Return its output (both stdout and stderr),
     optionally split by lines (if split_lines is True). Raise a LaunchError
@@ -239,13 +287,20 @@ def launchsvn(s, show=False, pretend=Fal
         return None
     return launch(cmd, **kwargs)
 
+def launchsvn_pipe(s, show=False, pretend=False, **kwargs):
+    """Launch SVN and grab its output."""
+    cmd = opts["svn"] + " " + s
+    if show or opts["verbose"] >= 2:
+        print cmd
+    if pretend:
+        return []
+    return launch_pipe(cmd, **kwargs)
+
 def svn_command(s):
     """Do (or pretend to do) an SVN command."""
-    out = launchsvn(s, show=opts["show-changes"] or opts["dry-run"],
-                    pretend=opts["dry-run"],
-                    split_lines=False)
-    if not opts["dry-run"]:
-        print out
+    for L in launchsvn_pipe(s, show=opts["show-changes"] or opts["dry-run"],
+                            pretend=opts["dry-run"]):
+        print L,
 
 def check_dir_clean(dir):
     """Check the current status of dir for local mods."""
@@ -754,12 +809,6 @@ def get_created_rev(url):
         error('unable to determine oldest revision for URL "%s"' % url)
     return oldest_rev
 
-def get_commit_log(url, revnum):
-    """Return the log message for a specific integer revision
-    number."""
-    out = launchsvn("log --incremental -r%d %s" % (revnum, url))
-    return "".join(out[1:])
-
 def construct_merged_log_message(url, revnums):
     """Return a commit log message containing all the commit messages
     in the specified revisions at the given URL.  The separator used
@@ -769,10 +818,22 @@ def construct_merged_log_message(url, re
     log message that is clearer in describing merges that contain
     other merges. Trailing newlines are removed from the embedded
     log messages."""
+    MAX_PIPES = 8
     messages = ['']
+    pipes = []
     longest_sep = ''
-    for r in revnums.sorted():
-        message = get_commit_log(url, r)
+
+    revs = revnums.sorted()
+    revs.reverse()
+    while revs or pipes:
+        if revs and len(pipes) < MAX_PIPES:
+            r = revs.pop()
+            p = launchsvn_pipe('log --incremental -r%d "%s"' % (r, url))
+            pipes.append(p)
+            continue
+
+        p = pipes.pop(0)
+        message = "".join(p.read()[1:])
         if message:
             message = rstrip(message, "\n") + "\n"
             messages.append(prefix_lines(LOG_LINE_PREFIX, message))
@@ -1070,7 +1131,7 @@ def action_merge(branch_dir, branch_prop
 
     blocked_revs = get_blocked_revs(branch_dir, opts["head-path"])
     merged_revs = opts["merged-revs"]
-
+    
     # Show what we're doing
     if opts["verbose"]:  # just to avoid useless calculations
         if merged_revs & revs:
@@ -1082,24 +1143,24 @@ def action_merge(branch_dir, branch_prop
             report('memorizing reflected revision(s): %s' % reflected_revs)
         if blocked_revs & revs:
             report('skipping blocked revisions(s): %s' % (blocked_revs & revs))
-
+    
     # Compute final merge set.
     revs = revs - merged_revs - blocked_revs - reflected_revs - phantom_revs
     if not revs:
         report('no revisions to merge, exiting')
         return
-
+    
     # When manually marking revisions as merged, we only update the
     # integration meta data, and don't perform an actual merge.
     record_only = opts["record-only"]
-
+    
     if record_only:
         report('recording merge of revision(s) %s from "%s"' %
                (revs, opts["head-url"]))
     else:
         report('merging in revision(s) %s from "%s"' %
                (revs, opts["head-url"]))
-
+    
     # Do the merge(s). Note: the starting revision number to 'svn merge'
     # is NOT inclusive so we have to subtract one from start.
     # We try to keep the number of merge operations as low as possible,
@@ -1107,14 +1168,14 @@ def action_merge(branch_dir, branch_prop
     old_merge_props = branch_props
     merge_metadata = logs[opts["head-url"]].merge_metadata()
     for start,end in minimal_merge_intervals(revs, phantom_revs):
-
+    
         # Set merge props appropriately if bidirectional support is enabled
         if opts["bidirectional"]:
           new_merge_props = merge_metadata.get(start-1)
           if new_merge_props != old_merge_props:
               set_merge_props(branch_dir, new_merge_props)
               old_merge_props = new_merge_props
-
+    
         if not record_only:
             # Do the merge
             svn_command("merge -r %d:%d %s %s" % \
_______________________________________________
Svnmerge mailing list
[email protected]
http://www.orcaware.com/mailman/listinfo/svnmerge

Reply via email to