From 13144702fca874fe40b08ef630f4b34ebaabc6a5 Mon Sep 17 00:00:00 2001 From: Rob Browning Date: Thu, 30 Sep 2021 21:02:59 -0500 Subject: [PATCH] PackMidx.__del__: replace with context management Signed-off-by: Rob Browning Tested-by: Rob Browning --- lib/bup/cmd/bloom.py | 35 +++++++++--------- lib/bup/cmd/list_idx.py | 24 ++++++------ lib/bup/cmd/midx.py | 81 +++++++++++++++++++---------------------- lib/bup/cmd/server.py | 6 +-- lib/bup/gc.py | 4 +- lib/bup/git.py | 1 - lib/bup/midx.py | 3 -- test/int/test_client.py | 10 ++--- test/int/test_git.py | 20 +++++----- 9 files changed, 88 insertions(+), 96 deletions(-) diff --git a/lib/bup/cmd/bloom.py b/lib/bup/cmd/bloom.py index f995964..9963c43 100755 --- a/lib/bup/cmd/bloom.py +++ b/lib/bup/cmd/bloom.py @@ -50,9 +50,10 @@ def check_bloom(path, bloomfilename, idx): idx = os.path.join(path, idx) log('bloom: bloom file: %s\n' % path_msg(rbloomfilename)) log('bloom: checking %s\n' % path_msg(ridx)) - for objsha in git.open_idx(idx): - if not b.exists(objsha): - add_error('bloom: ERROR: object %s missing' % hexstr(objsha)) + with git.open_idx(idx) as oids: + for oid in oids: + if not b.exists(oid): + add_error('bloom: ERROR: object %s missing' % hexstr(oid)) _first = None @@ -74,14 +75,14 @@ def do_bloom(path, outfilename, k, force): rest_count = 0 for i, name in enumerate(glob.glob(b'%s/*.idx' % path)): progress('bloom: counting: %d\r' % i) - ix = git.open_idx(name) - ixbase = os.path.basename(name) - if b and (ixbase in b.idxnames): - rest.append(name) - rest_count += len(ix) - else: - add.append(name) - add_count += len(ix) + with git.open_idx(name) as ix: + ixbase = os.path.basename(name) + if b and (ixbase in b.idxnames): + rest.append(name) + rest_count += len(ix) + else: + add.append(name) + add_count += len(ix) if not add: debug1("bloom: nothing to do.\n") @@ -129,12 +130,12 @@ def do_bloom(path, outfilename, k, force): count = 0 icount = 0 for name in add: - ix = git.open_idx(name) - qprogress('bloom: writing %.2f%% (%d/%d objects)\r' - % (icount*100.0/add_count, icount, add_count)) - b.add_idx(ix) - count += 1 - icount += len(ix) + with git.open_idx(name) as ix: + qprogress('bloom: writing %.2f%% (%d/%d objects)\r' + % (icount*100.0/add_count, icount, add_count)) + b.add_idx(ix) + count += 1 + icount += len(ix) finally: # This won't handle pending exceptions correctly in py2 # Currently, there's an open file object for tfname inside b. diff --git a/lib/bup/cmd/list_idx.py b/lib/bup/cmd/list_idx.py index 15e8b4c..99384cc 100755 --- a/lib/bup/cmd/list_idx.py +++ b/lib/bup/cmd/list_idx.py @@ -47,18 +47,20 @@ def main(argv): ix = git.open_idx(name) except git.GitError as e: add_error('%r: %s' % (name, e)) + ix.close() continue - if len(opt.find) == 40: - if ix.exists(bin): - out.write(b'%s %s\n' % (name, find)) - else: - # slow, exhaustive search - for _i in ix: - i = hexlify(_i) - if i.startswith(find): - out.write(b'%s %s\n' % (name, i)) - qprogress('Searching: %d\r' % count) - count += 1 + with ix: + if len(opt.find) == 40: + if ix.exists(bin): + out.write(b'%s %s\n' % (name, find)) + else: + # slow, exhaustive search + for _i in ix: + i = hexlify(_i) + if i.startswith(find): + out.write(b'%s %s\n' % (name, i)) + qprogress('Searching: %d\r' % count) + count += 1 if saved_errors: log('WARNING: %d errors encountered while saving.\n' % len(saved_errors)) diff --git a/lib/bup/cmd/midx.py b/lib/bup/cmd/midx.py index 604f7e3..9d3b637 100755 --- a/lib/bup/cmd/midx.py +++ b/lib/bup/cmd/midx.py @@ -4,7 +4,7 @@ from binascii import hexlify import glob, os, math, resource, struct, sys from bup import options, git, midx, _helpers, xstat -from bup.compat import argv_bytes, hexstr, range +from bup.compat import ExitStack, argv_bytes, hexstr, range from bup.helpers import (Sha1, add_error, atomically_replaced_file, debug1, fdatasync, log, mmap_readwrite, qprogress, saved_errors, unlink) @@ -51,32 +51,34 @@ def check_midx(name): except git.GitError as e: add_error('%s: %s' % (path_msg(name), e)) return - for count,subname in enumerate(ix.idxnames): - sub = git.open_idx(os.path.join(os.path.dirname(name), subname)) - for ecount,e in enumerate(sub): + with ix: + for count,subname in enumerate(ix.idxnames): + with git.open_idx(os.path.join(os.path.dirname(name), subname)) \ + as sub: + for ecount,e in enumerate(sub): + if not (ecount % 1234): + qprogress(' %d/%d: %s %d/%d\r' + % (count, len(ix.idxnames), + git.shorten_hash(subname).decode('ascii'), + ecount, len(sub))) + if not sub.exists(e): + add_error("%s: %s: %s missing from idx" + % (path_msg(nicename), + git.shorten_hash(subname).decode('ascii'), + hexstr(e))) + if not ix.exists(e): + add_error("%s: %s: %s missing from midx" + % (path_msg(nicename), + git.shorten_hash(subname).decode('ascii'), + hexstr(e))) + prev = None + for ecount,e in enumerate(ix): if not (ecount % 1234): - qprogress(' %d/%d: %s %d/%d\r' - % (count, len(ix.idxnames), - git.shorten_hash(subname).decode('ascii'), - ecount, len(sub))) - if not sub.exists(e): - add_error("%s: %s: %s missing from idx" - % (path_msg(nicename), - git.shorten_hash(subname).decode('ascii'), - hexstr(e))) - if not ix.exists(e): - add_error("%s: %s: %s missing from midx" - % (path_msg(nicename), - git.shorten_hash(subname).decode('ascii'), - hexstr(e))) - prev = None - for ecount,e in enumerate(ix): - if not (ecount % 1234): - qprogress(' Ordering: %d/%d\r' % (ecount, len(ix))) - if e and prev and not e >= prev: - add_error('%s: ordering error: %s < %s' - % (nicename, hexstr(e), hexstr(prev))) - prev = e + qprogress(' Ordering: %d/%d\r' % (ecount, len(ix))) + if e and prev and not e >= prev: + add_error('%s: ordering error: %s < %s' + % (nicename, hexstr(e), hexstr(prev))) + prev = e _first = None @@ -91,11 +93,10 @@ def _do_midx(outdir, outfilename, infilenames, prefixstr, inp = [] total = 0 allfilenames = [] - midxs = [] - try: + with ExitStack() as contexts: for name in infilenames: ix = git.open_idx(name) - midxs.append(ix) + contexts.enter_context(ix) inp.append(( ix.map, len(ix), @@ -133,18 +134,10 @@ def _do_midx(outdir, outfilename, infilenames, prefixstr, f.flush() fdatasync(f.fileno()) - fmap = mmap_readwrite(f, close=False) - count = merge_into(fmap, bits, total, inp) - del fmap # Assume this calls msync() now. + with mmap_readwrite(f, close=False) as fmap: + count = merge_into(fmap, bits, total, inp) f.seek(0, os.SEEK_END) f.write(b'\0'.join(allfilenames)) - finally: - for ix in midxs: - if isinstance(ix, midx.PackMidx): - ix.close() - midxs = None - inp = None - # This is just for testing (if you enable this, don't clear inp above) # if 0: @@ -178,9 +171,9 @@ def do_midx_dir(path, outfilename, prout, auto=False, force=False, midxs = glob.glob(b'%s/*.midx' % path) contents = {} for mname in midxs: - m = git.open_idx(mname) - contents[mname] = [(b'%s/%s' % (path,i)) for i in m.idxnames] - sizes[mname] = len(m) + with git.open_idx(mname) as m: + contents[mname] = [(b'%s/%s' % (path,i)) for i in m.idxnames] + sizes[mname] = len(m) # sort the biggest+newest midxes first, so that we can eliminate # smaller (or older) redundant ones that come later in the list @@ -201,8 +194,8 @@ def do_midx_dir(path, outfilename, prout, auto=False, force=False, idxs = [k for k in glob.glob(b'%s/*.idx' % path) if not already.get(k)] for iname in idxs: - i = git.open_idx(iname) - sizes[iname] = len(i) + with git.open_idx(iname) as i: + sizes[iname] = len(i) all = [(sizes[n],n) for n in (midxs + idxs)] diff --git a/lib/bup/cmd/server.py b/lib/bup/cmd/server.py index 5e2b6af..3d077dd 100755 --- a/lib/bup/cmd/server.py +++ b/lib/bup/cmd/server.py @@ -73,9 +73,9 @@ def send_index(conn, name): _init_session() assert name.find(b'/') < 0 assert name.endswith(b'.idx') - idx = git.open_idx(git.repo(b'objects/pack/%s' % name)) - conn.write(struct.pack('!I', len(idx.map))) - conn.write(idx.map) + with git.open_idx(git.repo(b'objects/pack/%s' % name)) as idx: + conn.write(struct.pack('!I', len(idx.map))) + conn.write(idx.map) conn.ok() diff --git a/lib/bup/gc.py b/lib/bup/gc.py index 06bd3ba..14e398b 100644 --- a/lib/bup/gc.py +++ b/lib/bup/gc.py @@ -57,8 +57,8 @@ def count_objects(dir, verbosity): log('found %d objects (%d/%d %s)\r' % (object_count, i + 1, len(indexes), path_msg(basename(idx_name)))) - idx = git.open_idx(idx_name) - object_count += len(idx) + with git.open_idx(idx_name) as idx: + object_count += len(idx) return object_count diff --git a/lib/bup/git.py b/lib/bup/git.py index 94f24b7..0dc6e72 100644 --- a/lib/bup/git.py +++ b/lib/bup/git.py @@ -611,7 +611,6 @@ class PackIdxList: broken = True if broken: mx.close() - del mx unlink(full) else: midxl.append(mx) diff --git a/lib/bup/midx.py b/lib/bup/midx.py index 239532e..e245816 100644 --- a/lib/bup/midx.py +++ b/lib/bup/midx.py @@ -58,9 +58,6 @@ class PackMidx: # which len is self.nsha * 4 self.idxnames = self.map[self.which_ofs + 4 * self.nsha:].split(b'\0') - def __del__(self): - self.close() - def __enter__(self): return self diff --git a/test/int/test_client.py b/test/int/test_client.py index 17e3a41..757c3ab 100644 --- a/test/int/test_client.py +++ b/test/int/test_client.py @@ -128,11 +128,11 @@ def test_midx_refreshing(tmpdir): assert len(pi.packs) == 2 assert sorted([os.path.basename(i.name) for i in pi.packs]) == sorted([p1base, p2base]) - p1 = git.open_idx(p1name) - assert p1.exists(s1sha) - p2 = git.open_idx(p2name) - assert not p2.exists(s1sha) - assert p2.exists(s2sha) + with git.open_idx(p1name) as p1, \ + git.open_idx(p2name) as p2: + assert p1.exists(s1sha) + assert not p2.exists(s1sha) + assert p2.exists(s2sha) subprocess.call([path.exe(), b'midx', b'-f']) pi.refresh() diff --git a/test/int/test_git.py b/test/int/test_git.py index ee65177..cae56ca 100644 --- a/test/int/test_git.py +++ b/test/int/test_git.py @@ -133,19 +133,19 @@ def test_packs(tmpdir): WVPASS(os.path.exists(nameprefix + b'.pack')) WVPASS(os.path.exists(nameprefix + b'.idx')) - r = git.open_idx(nameprefix + b'.idx') - print(repr(r.fanout)) + with git.open_idx(nameprefix + b'.idx') as r: + print(repr(r.fanout)) - for i in range(nobj): - WVPASS(r.find_offset(hashes[i]) > 0) - WVPASS(r.exists(hashes[99])) - WVFAIL(r.exists(b'\0'*20)) + for i in range(nobj): + WVPASS(r.find_offset(hashes[i]) > 0) + WVPASS(r.exists(hashes[99])) + WVFAIL(r.exists(b'\0'*20)) - pi = iter(r) - for h in sorted(hashes): - WVPASSEQ(hexlify(next(pi)), hexlify(h)) + pi = iter(r) + for h in sorted(hashes): + WVPASSEQ(hexlify(next(pi)), hexlify(h)) - WVFAIL(r.find_offset(b'\0'*20)) + WVFAIL(r.find_offset(b'\0'*20)) r = git.PackIdxList(bupdir + b'/objects/pack') WVPASS(r.exists(hashes[5])) -- 2.39.2