tequilaOS/platform_build
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

releasetools: Remove the unconditional fallback to bsdiff.

Browse source 
This CL uses the 'incomplete' tag to skip applying imgdiff to files with
incomplete block list. It's not the ideal fix to address the holes in
ext4 images, but would unhide other imgdiff issues covered by the
unconditional fallback.

Bug: 68016761
Test: Generate an incremental OTA package from images with incomplete
      block list. Check the imgdiff stats report.
Test: `python -m unittest test_blockimgdiff`
Change-Id: Ice77686414e70f5e42de35c1757fb31cf02e4fd4
This commit is contained in:
Tao Bao 2018-02-06 15:16:41 -08:00
parent 4a066a2cd2
commit 4ccea8549e
2 changed files with 20 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

53
tools/releasetools/blockimgdiff.py
View file

@ -264,9 +264,6 @@ class ImgdiffStats(object):
reasons. The stats is only meaningful when imgdiff not being disabled by the
caller of BlockImageDiff. In addition, only files with supported types
(BlockImageDiff.FileTypeSupportedByImgdiff()) are allowed to be logged.
TODO: The info could be inaccurate due to the unconditional fallback from
imgdiff to bsdiff on errors. The fallbacks will be removed.
"""
USED_IMGDIFF = "APK files diff'd with imgdiff"
@ -275,6 +272,7 @@ class ImgdiffStats(object):
# Reasons for not applying imgdiff on APKs.
SKIPPED_TRIMMED = "Not used imgdiff due to trimmed RangeSet"
SKIPPED_NONMONOTONIC = "Not used imgdiff due to having non-monotonic ranges"
SKIPPED_INCOMPLETE = "Not used imgdiff due to incomplete RangeSet"
# The list of valid reasons, which will also be the dumped order in a report.
REASONS = (
@ -282,6 +280,7 @@ class ImgdiffStats(object):
USED_IMGDIFF_LARGE_APK,
SKIPPED_TRIMMED,
SKIPPED_NONMONOTONIC,
SKIPPED_INCOMPLETE,
)
def __init__(self):
@ -415,6 +414,7 @@ class BlockImageDiff(object):
- The file type is supported by imgdiff;
- The source and target blocks are monotonic (i.e. the data is stored with
blocks in increasing order);
- Both files have complete lists of blocks;
- We haven't removed any blocks from the source set.
If all these conditions are satisfied, concatenating all the blocks in the
@ -437,6 +437,10 @@ class BlockImageDiff(object):
self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_NONMONOTONIC)
return False
if tgt_ranges.extra.get('incomplete') or src_ranges.extra.get('incomplete'):
self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_INCOMPLETE)
return False
if tgt_ranges.extra.get('trimmed') or src_ranges.extra.get('trimmed'):
self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_TRIMMED)
return False
@ -851,7 +855,6 @@ class BlockImageDiff(object):
diff_total = len(diff_queue)
patches = [None] * diff_total
error_messages = []
warning_messages = []
# Using multiprocessing doesn't give additional benefits, due to the
# pattern of the code. The diffing work is done by subprocess.call, which
@ -899,23 +902,6 @@ class BlockImageDiff(object):
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))
# TODO(b/68016761): Better handle the holes in mke2fs created
# images.
if imgdiff:
try:
patch = compute_patch(src_file, tgt_file, imgdiff=False)
message.append(
"Fell back and generated with bsdiff instead for %s" % (
xf.tgt_name,))
xf.style = "bsdiff"
with lock:
warning_messages.extend(message)
del message[:]
except ValueError as e:
message.append(
"Also failed to generate with bsdiff for %s:\n%s" % (
xf.tgt_name, e.message))
if message:
with lock:
error_messages.extend(message)
@ -933,11 +919,6 @@ class BlockImageDiff(object):
if sys.stdout.isatty():
print('\n')
if warning_messages:
print('WARNING:')
print('\n'.join(warning_messages))
print('\n\n\n')
if error_messages:
print('ERROR:')
print('\n'.join(error_messages))
@ -1518,11 +1499,6 @@ class BlockImageDiff(object):
be valid because the block ranges of src-X & tgt-X will always stay the
same afterwards; but there's a chance we don't use the patch if we
convert the "diff" command into "new" or "move" later.
The split will be attempted by calling imgdiff, which expects the input
files to be valid zip archives. If imgdiff fails for some reason (i.e.
holes in the APK file), we will fall back to split the failed APKs into
fixed size chunks.
"""
while True:
@ -1544,16 +1520,11 @@ class BlockImageDiff(object):
"--block-limit={}".format(max_blocks_per_transfer),
"--split-info=" + patch_info_file,
src_file, tgt_file, patch_file]
p = common.Run(cmd, stdout=subprocess.PIPE)
p.communicate()
if p.returncode != 0:
print("Failed to create patch between {} and {},"
" falling back to bsdiff".format(src_name, tgt_name))
with transfer_lock:
AddSplitTransfersWithFixedSizeChunks(tgt_name, src_name,
tgt_ranges, src_ranges,
"diff", self.transfers)
continue
p = common.Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
imgdiff_output, _ = p.communicate()
assert p.returncode == 0, \
"Failed to create imgdiff patch between {} and {}:\n{}".format(
src_name, tgt_name, imgdiff_output)
with open(patch_info_file) as patch_info:
lines = patch_info.readlines()

8
tools/releasetools/test_blockimgdiff.py
View file

@ -236,11 +236,19 @@ class BlockImageDiffTest(unittest.TestCase):
block_image_diff.CanUseImgdiff(
"/vendor/app/app3.apk", RangeSet("10-15"), src_ranges))
# At least one of the ranges is incomplete.
src_ranges = RangeSet("0-5")
src_ranges.extra['incomplete'] = True
self.assertFalse(
block_image_diff.CanUseImgdiff(
"/vendor/app/app4.apk", RangeSet("10-15"), src_ranges))
# The stats are correctly logged.
self.assertDictEqual(
{
ImgdiffStats.SKIPPED_NONMONOTONIC : {'/system/app/app2.apk'},
ImgdiffStats.SKIPPED_TRIMMED : {'/vendor/app/app3.apk'},
ImgdiffStats.SKIPPED_INCOMPLETE: {'/vendor/app/app4.apk'},
},
block_image_diff.imgdiff_stats.stats)