Remove Client __del__ in favor of context management

[bup.git] / lib / bup / client.py

diff --git a/lib/bup/client.py b/lib/bup/client.py
index 9bb464c0ff60f4810ef85a640bb2e2875f6652c2..178eda563241a438f56202c0427c625d68b9d516 100644 (file)
--- a/lib/bup/client.py
+++ b/lib/bup/client.py
@@ -3,15 +3,16 @@ from __future__ import print_function
 
 from __future__ import absolute_import
 from binascii import hexlify, unhexlify
-import errno, os, re, struct, sys, time, zlib
+import os, re, struct, time, zlib
+import socket
 
 from bup import git, ssh, vfs
-from bup.compat import environ, range, reraise
+from bup.compat import environ, pending_raise, range, reraise
 from bup.helpers import (Conn, atomically_replaced_file, chunkyreader, debug1,
                          debug2, linereader, lines_until_sentinel,
-                         mkdirp, progress, qprogress)
+                         mkdirp, progress, qprogress, DemuxConn)
 from bup.io import path_msg
-from bup.vint import read_bvec, read_vuint, write_bvec
+from bup.vint import write_bvec
 
 
 bwlimit = None
@@ -75,13 +76,15 @@ class Client:
             assert(not remote)
             remote = b'%s:' % is_reverse
         (self.protocol, self.host, self.port, self.dir) = parse_remote(remote)
+        # The b'None' here matches python2's behavior of b'%s' % None == 'None',
+        # python3 will (as of version 3.7.5) do the same for str ('%s' % None),
+        # but crashes instead when doing b'%s' % None.
+        cachehost = b'None' if self.host is None else self.host
+        cachedir = b'None' if self.dir is None else self.dir
         self.cachedir = git.repo(b'index-cache/%s'
                                  % re.sub(br'[^@\w]',
                                           b'_',
-                                          # FIXME: the Nones just
-                                          # match python 2's behavior
-                                          b'%s:%s' % (self.host or b'None',
-                                                      self.dir or b'None')))
+                                          b'%s:%s' % (cachehost, cachedir)))
         if is_reverse:
             self.pout = os.fdopen(3, 'rb')
             self.pin = os.fdopen(4, 'wb')
@@ -98,7 +101,8 @@ class Client:
                     reraise(ClientError('connect: %s' % e))
             elif self.protocol == b'bup':
                 self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-                self.sock.connect((self.host, atoi(self.port) or 1982))
+                self.sock.connect((self.host,
+                                   1982 if self.port is None else int(self.port)))
                 self.sockw = self.sock.makefile('wb')
                 self.conn = DemuxConn(self.sock.fileno(), self.sockw)
         self._available_commands = self._get_available_commands()
@@ -113,14 +117,12 @@ class Client:
             self.check_ok()
         self.sync_indexes()
 
-    def __del__(self):
-        try:
+    def __enter__(self):
+        return self
+
+    def __exit__(self, type, value, traceback):
+        with pending_raise(value, rethrow=False):
             self.close()
-        except IOError as e:
-            if e.errno == errno.EPIPE:
-                pass
-            else:
-                raise
 
     def close(self):
         if self.conn and not self._busy:
@@ -154,15 +156,17 @@ class Client:
             return self.conn.check_ok()
         except Exception as e:
             reraise(ClientError(e))
+            # reraise doesn't return
+            return None
 
     def check_busy(self):
         if self._busy:
             raise ClientError('already busy with command %r' % self._busy)
-        
+
     def ensure_busy(self):
         if not self._busy:
             raise ClientError('expected to be busy, but not busy?!')
-        
+
     def _not_busy(self):
         self._busy = None
 
@@ -404,7 +408,7 @@ class Client:
             raise not_ok
         self._not_busy()
 
-    def rev_list(self, refs, count=None, parse=None, format=None):
+    def rev_list(self, refs, 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
@@ -413,7 +417,6 @@ class Client:
 
         """
         self._require_command(b'rev-list')
-        assert (count is None) or (isinstance(count, Integral))
         if format:
             assert b'\n' not in format
             assert parse
@@ -424,8 +427,6 @@ class Client:
         self._busy = b'rev-list'
         conn = self.conn
         conn.write(b'rev-list\n')
-        if count is not None:
-            conn.write(b'%d' % count)
         conn.write(b'\n')
         if format:
             conn.write(format)
@@ -514,6 +515,7 @@ class PackWriter_Remote(git.PackWriter):
             self.onclose() # Unbusy
             self.objcache = None
             return self.suggest_packs() # Returns last idx received
+        return None
 
     def close(self):
         id = self._end()