From 9739514769d1a5af0236e80bf54c57bec82511a8 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 12 Feb 2018 12:08:05 -0800 Subject: [PATCH 1/2] releasetools: Remove the global diff_done in blockimgdiff.py. pylint complains about undefined `diff_done`: W:754, 8: Global variable 'diff_done' undefined at the module level (global-variable-undefined) W:820,14: Global variable 'diff_done' undefined at the module level (global-variable-undefined) It would still warn about using global statement after adding the definition. W:859, 8: Using the global statement (global-statement) W:925,14: Using the global statement (global-statement) This CL computes 'diff_done' via 'len(diff_queue)' instead. It also moves the progress reporting _before_ the diff work. This way it avoids showing 100% progress with still changing filenames (because multiple workers could see an empty queue simultaneously upon finishing their own works). There're possible alternatives, such as using the 'nonlocal' keyword in Python 3 (which we're not there yet), or by using mutable object instead (e.g. 'diff_done = [0]'). This CL looks cleaner, since it just kills the var. Test: Generate a BBOTA incremental. Check the on-screen progress report. Test: `pylint --rcfile=pylintrc blockimgdiff.py` no longer complains about the global diff_done. Change-Id: I339824735527e1f794b5b1dc99ff3fdb2da85744 --- tools/releasetools/blockimgdiff.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tools/releasetools/blockimgdiff.py b/tools/releasetools/blockimgdiff.py index fe37b391d..0535f7be6 100644 --- a/tools/releasetools/blockimgdiff.py +++ b/tools/releasetools/blockimgdiff.py @@ -852,9 +852,6 @@ class BlockImageDiff(object): patches = [None] * diff_total error_messages = [] warning_messages = [] - if sys.stdout.isatty(): - global diff_done - diff_done = 0 # Using multiprocessing doesn't give additional benefits, due to the # pattern of the code. The diffing work is done by subprocess.call, which @@ -870,8 +867,15 @@ class BlockImageDiff(object): if not diff_queue: return xf_index, imgdiff, patch_index = diff_queue.pop() + xf = self.transfers[xf_index] + + if sys.stdout.isatty(): + diff_left = len(diff_queue) + progress = (diff_total - diff_left) * 100 / diff_total + # '\033[K' is to clear to EOL. + print(' [%3d%%] %s\033[K' % (progress, xf.tgt_name), end='\r') + sys.stdout.flush() - xf = self.transfers[xf_index] patch = xf.patch if not patch: src_ranges = xf.src_ranges @@ -918,13 +922,6 @@ class BlockImageDiff(object): with lock: patches[patch_index] = (xf_index, patch) - if sys.stdout.isatty(): - global diff_done - diff_done += 1 - progress = diff_done * 100 / diff_total - # '\033[K' is to clear to EOL. - print(' [%d%%] %s\033[K' % (progress, xf.tgt_name), end='\r') - sys.stdout.flush() threads = [threading.Thread(target=diff_worker) for _ in range(self.threads)] From 508b0879430b55a97e17505aa8fc6a0907bc9e62 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 9 Feb 2018 13:44:43 -0800 Subject: [PATCH 2/2] releasetools: Make blockimgdiff.py pylint-clean. ************* Module blockimgdiff C:433, 0: Unnecessary parens after 'if' keyword (superfluous-parens) C:687, 0: Wrong hanging indentation (add 4 spaces). max_stashed_blocks, self._max_stashed_size, max_allowed, ^ | (bad-continuation) C:688, 0: Wrong hanging indentation (add 4 spaces). self._max_stashed_size * 100.0 / max_allowed)) ^ | (bad-continuation) C:691, 0: Wrong hanging indentation (remove 2 spaces). max_stashed_blocks, self._max_stashed_size)) | ^ (bad-continuation) C:898, 0: Wrong hanging indentation (add 4 spaces). "imgdiff" if imgdiff else "bsdiff", ^ | (bad-continuation) C:899, 0: Wrong hanging indentation (add 4 spaces). xf.tgt_name if xf.tgt_name == xf.src_name else ^ | (bad-continuation) C:901, 0: Wrong hanging indentation (add 4 spaces). xf.tgt_ranges, xf.src_ranges, e.message)) ^ | (bad-continuation) C:909, 0: Wrong hanging indentation (add 4 spaces). xf.tgt_name,)) ^ | (bad-continuation) C:917, 0: Wrong hanging indentation (add 4 spaces). xf.tgt_name, e.message)) ^ | (bad-continuation) C:961, 0: Wrong hanging indentation (remove 2 spaces). xf.patch_len, tgt_size, xf.patch_len * 100.0 / tgt_size, | ^ (bad-continuation) C:962, 0: Wrong hanging indentation (remove 2 spaces). xf.style, | ^ (bad-continuation) C:963, 0: Wrong hanging indentation (remove 2 spaces). xf.tgt_name if xf.tgt_name == xf.src_name else ( | ^ (bad-continuation) C:965, 0: Wrong hanging indentation (remove 2 spaces). xf.tgt_ranges, xf.src_ranges)) | ^ (bad-continuation) C:1422, 0: Wrong continued indentation (add 28 spaces). tgt_skipped.size() * 100.0 / tgt_size, tgt_name)) ^ | (bad-continuation) C:1560, 0: Wrong continued indentation (add 8 spaces). split_src_ranges) in enumerate(split_info_list): ^ | (bad-continuation) R:566, 6: Redefinition of src_str type from list to str (redefined-variable-type) C:1198,25: More than one statement on a single line (multiple-statements) C:1211,25: More than one statement on a single line (multiple-statements) C:1220,16: More than one statement on a single line (multiple-statements) C:1277,38: More than one statement on a single line (multiple-statements) C:1284,19: More than one statement on a single line (multiple-statements) C: 19, 0: standard import "import copy" comes before "import common" (wrong-import-order) C: 20, 0: standard import "import functools" comes before "import common" (wrong-import-order) C: 21, 0: standard import "import heapq" comes before "import common" (wrong-import-order) C: 22, 0: standard import "import itertools" comes before "import common" (wrong-import-order) C: 23, 0: standard import "import multiprocessing" comes before "import common" (wrong-import-order) C: 24, 0: standard import "import os" comes before "import common" (wrong-import-order) C: 25, 0: standard import "import os.path" comes before "import common" (wrong-import-order) C: 26, 0: standard import "import re" comes before "import common" (wrong-import-order) C: 27, 0: standard import "import subprocess" comes before "import common" (wrong-import-order) C: 28, 0: standard import "import sys" comes before "import common" (wrong-import-order) C: 29, 0: standard import "import threading" comes before "import common" (wrong-import-order) C: 31, 0: standard import "from collections import deque, OrderedDict" comes before "import common" (wrong-import-order) C: 32, 0: standard import "from hashlib import sha1" comes before "import common" (wrong-import-order) Test: `pylint --rcfile=pylintrc blockimgdiff.py` Test: Generate an incremental BBOTA package. Change-Id: Ia7ecd0e1fa48daf4e43251bdcdfcd08fb316015d --- tools/releasetools/blockimgdiff.py | 66 ++++++++++++++++-------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/tools/releasetools/blockimgdiff.py b/tools/releasetools/blockimgdiff.py index 0535f7be6..d6ff6a690 100644 --- a/tools/releasetools/blockimgdiff.py +++ b/tools/releasetools/blockimgdiff.py @@ -15,7 +15,6 @@ from __future__ import print_function import array -import common import copy import functools import heapq @@ -27,9 +26,10 @@ import re import subprocess import sys import threading - from collections import deque, OrderedDict from hashlib import sha1 + +import common from rangelib import RangeSet @@ -430,7 +430,7 @@ class BlockImageDiff(object): Returns: A boolean result. """ - if (self.disable_imgdiff or not self.FileTypeSupportedByImgdiff(name)): + if self.disable_imgdiff or not self.FileTypeSupportedByImgdiff(name): return False if not tgt_ranges.monotonic or not src_ranges.monotonic: @@ -535,7 +535,7 @@ class BlockImageDiff(object): # <# blocks> - size = xf.src_ranges.size() - src_str = [str(size)] + src_str_buffer = [str(size)] unstashed_src_ranges = xf.src_ranges mapped_stashes = [] @@ -545,7 +545,7 @@ class BlockImageDiff(object): sr = xf.src_ranges.map_within(sr) mapped_stashes.append(sr) assert sh in stashes - src_str.append("%s:%s" % (sh, sr.to_string_raw())) + src_str_buffer.append("%s:%s" % (sh, sr.to_string_raw())) stashes[sh] -= 1 if stashes[sh] == 0: free_string.append("free %s\n" % (sh,)) @@ -553,17 +553,17 @@ class BlockImageDiff(object): stashes.pop(sh) if unstashed_src_ranges: - src_str.insert(1, unstashed_src_ranges.to_string_raw()) + src_str_buffer.insert(1, unstashed_src_ranges.to_string_raw()) if xf.use_stash: mapped_unstashed = xf.src_ranges.map_within(unstashed_src_ranges) - src_str.insert(2, mapped_unstashed.to_string_raw()) + src_str_buffer.insert(2, mapped_unstashed.to_string_raw()) mapped_stashes.append(mapped_unstashed) self.AssertPartition(RangeSet(data=(0, size)), mapped_stashes) else: - src_str.insert(1, "-") + src_str_buffer.insert(1, "-") self.AssertPartition(RangeSet(data=(0, size)), mapped_stashes) - src_str = " ".join(src_str) + src_str = " ".join(src_str_buffer) # version 3+: # zero @@ -684,11 +684,11 @@ class BlockImageDiff(object): max_allowed = OPTIONS.cache_size * OPTIONS.stash_threshold print("max stashed blocks: %d (%d bytes), " "limit: %d bytes (%.2f%%)\n" % ( - max_stashed_blocks, self._max_stashed_size, max_allowed, - self._max_stashed_size * 100.0 / max_allowed)) + max_stashed_blocks, self._max_stashed_size, max_allowed, + self._max_stashed_size * 100.0 / max_allowed)) else: print("max stashed blocks: %d (%d bytes), limit: \n" % ( - max_stashed_blocks, self._max_stashed_size)) + max_stashed_blocks, self._max_stashed_size)) def ReviseStashSize(self): print("Revising stash size...") @@ -895,10 +895,10 @@ class BlockImageDiff(object): except ValueError as e: message.append( "Failed to generate %s for %s: tgt=%s, src=%s:\n%s" % ( - "imgdiff" if imgdiff else "bsdiff", - xf.tgt_name if xf.tgt_name == xf.src_name else + "imgdiff" if imgdiff else "bsdiff", + xf.tgt_name if xf.tgt_name == xf.src_name else xf.tgt_name + " (from " + xf.src_name + ")", - xf.tgt_ranges, xf.src_ranges, e.message)) + xf.tgt_ranges, xf.src_ranges, e.message)) # TODO(b/68016761): Better handle the holes in mke2fs created # images. if imgdiff: @@ -906,7 +906,7 @@ class BlockImageDiff(object): patch = compute_patch(src_file, tgt_file, imgdiff=False) message.append( "Fell back and generated with bsdiff instead for %s" % ( - xf.tgt_name,)) + xf.tgt_name,)) xf.style = "bsdiff" with lock: warning_messages.extend(message) @@ -914,7 +914,7 @@ class BlockImageDiff(object): except ValueError as e: message.append( "Also failed to generate with bsdiff for %s:\n%s" % ( - xf.tgt_name, e.message)) + xf.tgt_name, e.message)) if message: with lock: @@ -958,11 +958,11 @@ class BlockImageDiff(object): if common.OPTIONS.verbose: tgt_size = xf.tgt_ranges.size() * self.tgt.blocksize print("%10d %10d (%6.2f%%) %7s %s %s %s" % ( - xf.patch_len, tgt_size, xf.patch_len * 100.0 / tgt_size, - xf.style, - xf.tgt_name if xf.tgt_name == xf.src_name else ( - xf.tgt_name + " (from " + xf.src_name + ")"), - xf.tgt_ranges, xf.src_ranges)) + xf.patch_len, tgt_size, xf.patch_len * 100.0 / tgt_size, + xf.style, + xf.tgt_name if xf.tgt_name == xf.src_name else ( + xf.tgt_name + " (from " + xf.src_name + ")"), + xf.tgt_ranges, xf.src_ranges)) def AssertSha1Good(self): """Check the SHA-1 of the src & tgt blocks in the transfer list. @@ -1195,7 +1195,8 @@ class BlockImageDiff(object): while sinks: new_sinks = OrderedDict() for u in sinks: - if u not in G: continue + if u not in G: + continue s2.appendleft(u) del G[u] for iu in u.incoming: @@ -1208,7 +1209,8 @@ class BlockImageDiff(object): while sources: new_sources = OrderedDict() for u in sources: - if u not in G: continue + if u not in G: + continue s1.append(u) del G[u] for iu in u.outgoing: @@ -1217,7 +1219,8 @@ class BlockImageDiff(object): new_sources[iu] = None sources = new_sources - if not G: break + if not G: + break # Find the "best" vertex to put next. "Best" is the one that # maximizes the net difference in source blocks saved we get by @@ -1274,14 +1277,16 @@ class BlockImageDiff(object): intersections = OrderedDict() for s, e in a.tgt_ranges: for i in range(s, e): - if i >= len(source_ranges): break + if i >= len(source_ranges): + break # Add all the Transfers in source_ranges[i] to the (ordered) set. if source_ranges[i] is not None: for j in source_ranges[i]: intersections[j] = None for b in intersections: - if a is b: continue + if a is b: + continue # If the blocks written by A are read by B, then B needs to go before A. i = a.tgt_ranges.intersect(b.src_ranges) @@ -1418,8 +1423,9 @@ class BlockImageDiff(object): if tgt_changed < tgt_size * crop_threshold: assert tgt_changed + tgt_skipped.size() == tgt_size - print('%10d %10d (%6.2f%%) %s' % (tgt_skipped.size(), tgt_size, - tgt_skipped.size() * 100.0 / tgt_size, tgt_name)) + print('%10d %10d (%6.2f%%) %s' % ( + tgt_skipped.size(), tgt_size, + tgt_skipped.size() * 100.0 / tgt_size, tgt_name)) AddSplitTransfers( "%s-skipped" % (tgt_name,), "%s-skipped" % (src_name,), @@ -1557,7 +1563,7 @@ class BlockImageDiff(object): tgt_ranges, src_ranges, lines) for index, (patch_start, patch_length, split_tgt_ranges, - split_src_ranges) in enumerate(split_info_list): + split_src_ranges) in enumerate(split_info_list): with open(patch_file) as f: f.seek(patch_start) patch_content = f.read(patch_length)