rev_list: handle multiple results/ref from remote for custom formats

Previously a remote rev_list invocation would crash if a custom format
was provided and the remote produced multiple results for any input
ref.  Change the client/server code to use a blank line to indicate
the end of the rev-list results.  Of course, that means that the parse
function provided must be able to handle (consume) any blank lines
that its format string produces, which may preclude the use of some
format strings, but should be sufficient for now.

Adjust test-ls to (trivially) cover this case, and broaden the use of
the commit hash length tests in the code.

Thanks to Alex Roper for reporting the problem, providing an easy way
to reproduce it, and proposing a fix.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>

diff --git a/cmd/server-cmd.py b/cmd/server-cmd.py
index c55c46e3942cd2cafe239363a6d3ff5739d3fe89..6fa9a67a96d8ac68dd7f748315cf6003a4999184 100755 (executable)
--- a/cmd/server-cmd.py
+++ b/cmd/server-cmd.py
@@ -226,6 +226,7 @@ def rev_list(conn, _):
         if not out:
             break
         conn.write(out)
+    conn.write('\n')
     rv = p.wait()  # not fatal
     if rv:
         msg = 'git rev-list returned error %d' % rv

diff --git a/lib/bup/client.py b/lib/bup/client.py
index f02c6b568d1d996fc9cff8e50f437ffd81e6efab..19d52cb968e25145d194a7359ce01a1bb9ce654a 100644 (file)
--- a/lib/bup/client.py
+++ b/lib/bup/client.py
@@ -393,6 +393,13 @@ class Client:
         self._not_busy()
 
     def rev_list(self, refs, count=None, parse=None, format=None):
+        """See git.rev_list for the general semantics, but note that with the
+        current interface, the parse function must be able to handle
+        (consume) any blank lines produced by the format because the
+        first one received that it doesn't consume will be interpreted
+        as a terminator for the entire rev-list result.
+
+        """
         self._require_command('rev-list')
         assert (count is None) or (isinstance(count, Integral))
         if format:
@@ -416,21 +423,17 @@ class Client:
             conn.write('\n')
         conn.write('\n')
         if not format:
-            for _ in range(len(refs)):
-                line = conn.readline()
-                if not line:
-                    raise ClientError('unexpected EOF')
+            for line in lines_until_sentinel(conn, '\n', ClientError):
                 line = line.strip()
                 assert len(line) == 40
                 yield line
         else:
-            for _ in range(len(refs)):
-                line = conn.readline()
-                if not line:
-                    raise ClientError('unexpected EOF')
+            for line in lines_until_sentinel(conn, '\n', ClientError):
                 if not line.startswith('commit '):
                     raise ClientError('unexpected line ' + repr(line))
-                yield line[7:].strip(), parse(conn)
+                cmt_oidx = line[7:].strip()
+                assert len(cmt_oidx) == 40
+                yield cmt_oidx, parse(conn)
         # FIXME: confusing
         not_ok = self.check_ok()
         if not_ok:

diff --git a/lib/bup/git.py b/lib/bup/git.py
index 3c88d74f8b02ad6b6ef1c8e14cd10f9e255835d1..2cf1c911daeb625f450c737cef3ad819f312c4ef 100644 (file)
--- a/lib/bup/git.py
+++ b/lib/bup/git.py
@@ -979,7 +979,9 @@ def rev_list(ref_or_refs, count=None, parse=None, format=None, repo_dir=None):
             s = line.strip()
             if not s.startswith('commit '):
                 raise Exception('unexpected line ' + s)
-            yield s[7:], parse(p.stdout)
+            s = s[7:]
+            assert len(s) == 40
+            yield s, parse(p.stdout)
             line = p.stdout.readline()
 
     rv = p.wait()  # not fatal

diff --git a/t/test-ls b/t/test-ls
index 850b48e04141c1a5c50693a3db7acbf111bad34b..2b919a211958318f416ce8be1f830b8ae20e3e48 100755 (executable)
--- a/t/test-ls
+++ b/t/test-ls
@@ -45,6 +45,8 @@ WVPASS touch -t 200910032348 src/.dotfile src/*
 WVPASS touch -t 200910032348 src
 WVPASS touch -t 200910032348 .
 WVPASS bup index src
+# Include two saves to test multiple results per ref from rev_list.
+WVPASS bup save -n src -d 242312159 --strip src
 WVPASS bup save -n src -d 242312160 --strip src
 WVPASS bup tag some-tag src
 
@@ -82,7 +84,8 @@ src/"
 WVPASSEQ "$(WVPASS bup-ls /.tag)" "some-tag"
 
 WVPASSEQ "$(WVPASS bup-ls /src)" \
-"1977-09-05-125600
+"1977-09-05-125559
+1977-09-05-125600
 latest"
 
 WVPASSEQ "$(WVPASS bup-ls src/latest)" "bad-symlink
@@ -255,6 +258,7 @@ WVPASSEQ "$(bup-ls -ld "src/latest" | tr -s ' ' ' ')" \
 WVSTART "$ls_cmd_desc (backup set - long)"
 WVPASSEQ "$(bup-ls -l --numeric-ids src | cut -d' ' -f 1-2)" \
 "drwx------ $uid/$gid
+drwx------ $uid/$gid
 lrwxr-xr-x 0/0"
 
 WVPASSEQ "$(bup-ls -ds "src/1977-09-05-125600" | tr -s ' ' ' ')" \