From bb7513d80a4876324c320c0cc96d07de2d8035ca Mon Sep 17 00:00:00 2001 From: Ulya Trafimovich Date: Tue, 30 Mar 2021 17:15:16 +0100 Subject: [PATCH] Emit a better error message in manifest_check.py. Bug: 132357300 Test: lunch aosp_cf_x86_64_phone-userdebug && m Test: temporarily patch Gallery2 to fail the check, observe the error Change-Id: Id67e359188396f68dcecd8b3f4d1bc26271af56d --- scripts/manifest_check.py | 61 +++++++++++++++++++++++----------- scripts/manifest_check_test.py | 4 +-- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/scripts/manifest_check.py b/scripts/manifest_check.py index 907f239cf..8168fbf6a 100755 --- a/scripts/manifest_check.py +++ b/scripts/manifest_check.py @@ -74,7 +74,7 @@ def parse_args(): return parser.parse_args() -def enforce_uses_libraries(manifest, required, optional, relax, is_apk = False): +def enforce_uses_libraries(manifest, required, optional, relax, is_apk, path): """Verify that the tags in the manifest match those provided by the build system. @@ -86,26 +86,36 @@ def enforce_uses_libraries(manifest, required, optional, relax, is_apk = False): is_apk: if the manifest comes from an APK or an XML file """ if is_apk: - manifest_required, manifest_optional = extract_uses_libs_apk(manifest) + manifest_required, manifest_optional, tags = extract_uses_libs_apk(manifest) else: - manifest_required, manifest_optional = extract_uses_libs_xml(manifest) + manifest_required, manifest_optional, tags = extract_uses_libs_xml(manifest) - err = [] - if manifest_required != required: - err.append('Expected required tags "%s", got "%s"' % - (', '.join(required), ', '.join(manifest_required))) + if manifest_required == required and manifest_optional == optional: + return None - if manifest_optional != optional: - err.append('Expected optional tags "%s", got "%s"' % - (', '.join(optional), ', '.join(manifest_optional))) + errmsg = ''.join([ + 'mismatch in the tags between the build system and the ' + 'manifest:\n', + '\t- required libraries in build system: [%s]\n' % ', '.join(required), + '\t vs. in the manifest: [%s]\n' % ', '.join(manifest_required), + '\t- optional libraries in build system: [%s]\n' % ', '.join(optional), + '\t vs. in the manifest: [%s]\n' % ', '.join(manifest_optional), + '\t- tags in the manifest (%s):\n' % path, + '\t\t%s\n' % '\t\t'.join(tags), + 'note: the following options are available:\n', + '\t- to temporarily disable the check on command line, rebuild with ', + 'RELAX_USES_LIBRARY_CHECK=true (this will set compiler filter "verify" ', + 'and disable AOT-compilation in dexpreopt)\n', + '\t- to temporarily disable the check for the whole product, set ', + 'PRODUCT_BROKEN_VERIFY_USES_LIBRARIES := true in the product makefiles\n', + '\t- to fix the check, make build system properties coherent with the ' + 'manifest\n', + '\t- see build/make/Changes.md for details\n']) - if err: - errmsg = '\n'.join(err) - if not relax: - raise ManifestMismatchError(errmsg) - return errmsg + if not relax: + raise ManifestMismatchError(errmsg) - return None + return errmsg def extract_uses_libs_apk(badging): @@ -115,14 +125,19 @@ def extract_uses_libs_apk(badging): required = [] optional = [] + lines = [] for match in re.finditer(pattern, badging): + lines.append(match.group(0)) libname = match.group(2) if match.group(1) == None: required.append(libname) else: optional.append(libname) - return first_unique_elements(required), first_unique_elements(optional) + required = first_unique_elements(required) + optional = first_unique_elements(optional) + tags = first_unique_elements(lines) + return required, optional, tags def extract_uses_libs_xml(xml): @@ -143,7 +158,15 @@ def extract_uses_libs_xml(xml): required = [uses_library_name(x) for x in libs if uses_library_required(x)] optional = [uses_library_name(x) for x in libs if not uses_library_required(x)] - return first_unique_elements(required), first_unique_elements(optional) + # render tags as XML for a pretty error message + tags = [] + for lib in libs: + tags.append(lib.toprettyxml()) + + required = first_unique_elements(required) + optional = first_unique_elements(optional) + tags = first_unique_elements(tags) + return required, optional, tags def first_unique_elements(l): @@ -278,7 +301,7 @@ def main(): # in the manifest. Raise an exception on mismatch, unless the script was # passed a special parameter to suppress exceptions. errmsg = enforce_uses_libraries(manifest, required, optional, - args.enforce_uses_libraries_relax, is_apk) + args.enforce_uses_libraries_relax, is_apk, args.input) # Create a status file that is empty on success, or contains an error # message on failure. When exceptions are suppressed, dexpreopt command diff --git a/scripts/manifest_check_test.py b/scripts/manifest_check_test.py index 635ba9d1d..7159bdd74 100755 --- a/scripts/manifest_check_test.py +++ b/scripts/manifest_check_test.py @@ -49,9 +49,9 @@ class EnforceUsesLibrariesTest(unittest.TestCase): try: relax = False manifest_check.enforce_uses_libraries(doc, uses_libraries, - optional_uses_libraries, relax, is_apk=False) + optional_uses_libraries, relax, False, 'path/to/X/AndroidManifest.xml') manifest_check.enforce_uses_libraries(apk, uses_libraries, - optional_uses_libraries, relax, is_apk=True) + optional_uses_libraries, relax, True, 'path/to/X/X.apk') return True except manifest_check.ManifestMismatchError: return False