Refactor the contruction of the manifest check inputs.
This is a no-op change for a majority of cases.
Before this change, the contruction of the manifest check inputs is
confusing. It mutates uses_libs properties in place just for the
manifest check, by replacing module names with library names for
direct dependencies and merging library names from CLC for both direct
denpendencies and transitive denpendencies, and then constructs manifest
check inputs from those mutated uses_libs properties. This is
error-prone and leads to insistency: the goal is to check that the CLC
matches the manifest, but the inputs to the check don't reflect the CLC.
After this change, we no longer mutate uses_libs properties in place.
Instead, we maintain a separate list of missing denpendencies, and then
construct manifest check inputs directly from the CLC for all existing
libraries, no matter they are direct or transtive, and from the separate
list of missing libraries. This change makes the logic more
consistent and straightforward, and it also allows us to easily do the
next change, which is to propagate transtive missing denpendencies.
In fact, this change revealed several bugs around library optionality
and order in CLC construction, and fixed them.
Bug: 331528424
Test: m --no-skip-soong-tests
Ignore-AOSP-First: Depends on internal changes. Will cherry-pick once merged.
Merged-In: I0de82e76c47995b54aba9efd41538d950256a95f
Change-Id: I0de82e76c47995b54aba9efd41538d950256a95f
diff --git a/scripts/manifest_check.py b/scripts/manifest_check.py
index c33b104..b101259 100755
--- a/scripts/manifest_check.py
+++ b/scripts/manifest_check.py
@@ -52,6 +52,14 @@
'required:false'
)
parser.add_argument(
+ '--missing-optional-uses-library',
+ dest='missing_optional_uses_libraries',
+ action='append',
+ help='specify uses-library entries missing from the build system with '
+ 'required:false',
+ default=[]
+ )
+ parser.add_argument(
'--enforce-uses-libraries',
dest='enforce_uses_libraries',
action='store_true',
@@ -91,7 +99,7 @@
C_BOLD = "\033[1m"
-def enforce_uses_libraries(manifest, required, optional, relax, is_apk, path):
+def enforce_uses_libraries(manifest, required, optional, missing_optional, relax, is_apk, path):
"""Verify that the <uses-library> tags in the manifest match those provided
by the build system.
@@ -119,7 +127,12 @@
required = trim_namespace_parts(required)
optional = trim_namespace_parts(optional)
- if manifest_required == required and manifest_optional == optional:
+ existing_manifest_optional = [
+ lib for lib in manifest_optional if lib not in missing_optional]
+
+ # The order of the existing libraries matter, while the order of the missing
+ # ones doesn't.
+ if manifest_required == required and existing_manifest_optional == optional:
return None
#pylint: disable=line-too-long
@@ -129,6 +142,7 @@
'\t- required libraries in build system: %s[%s]%s\n' % (C_RED, ', '.join(required), C_OFF),
'\t vs. in the manifest: %s[%s]%s\n' % (C_RED, ', '.join(manifest_required), C_OFF),
'\t- optional libraries in build system: %s[%s]%s\n' % (C_RED, ', '.join(optional), C_OFF),
+ '\t and missing ones in build system: %s[%s]%s\n' % (C_RED, ', '.join(missing_optional), C_OFF),
'\t vs. in the manifest: %s[%s]%s\n' % (C_RED, ', '.join(manifest_optional), C_OFF),
'\t- tags in the manifest (%s):\n' % path,
'\t\t%s\n' % '\t\t'.join(tags),
@@ -340,11 +354,14 @@
if args.enforce_uses_libraries:
# Load dexpreopt.config files and build a mapping from module
- # names to library names. This is necessary because build system
- # addresses libraries by their module name (`uses_libs`,
- # `optional_uses_libs`, `LOCAL_USES_LIBRARIES`,
- # `LOCAL_OPTIONAL_LIBRARY_NAMES` all contain module names), while
- # the manifest addresses libraries by their name.
+ # names to library names. This is for Make only and it's necessary
+ # because Make passes module names from `LOCAL_USES_LIBRARIES`,
+ # `LOCAL_OPTIONAL_LIBRARY_NAMES`, while the manifest addresses
+ # libraries by their name. Soong doesn't use it and doesn't need it
+ # because it converts the module names to the library names and
+ # passes the library names. There is no need to translate missing
+ # optional libs because they are missing and therefore there is no
+ # mapping for them.
mod_to_lib = load_dexpreopt_configs(args.dexpreopt_configs)
required = translate_libnames(args.uses_libraries, mod_to_lib)
optional = translate_libnames(args.optional_uses_libraries,
@@ -354,8 +371,8 @@
# those 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.input)
+ args.missing_optional_uses_libraries,
+ 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,
diff --git a/scripts/manifest_check_test.py b/scripts/manifest_check_test.py
index 3be7a30..8003b3e 100755
--- a/scripts/manifest_check_test.py
+++ b/scripts/manifest_check_test.py
@@ -44,15 +44,17 @@
class EnforceUsesLibrariesTest(unittest.TestCase):
"""Unit tests for add_extract_native_libs function."""
- def run_test(self, xml, apk, uses_libraries=[], optional_uses_libraries=[]): #pylint: disable=dangerous-default-value
+ def run_test(self, xml, apk, uses_libraries=[], optional_uses_libraries=[],
+ missing_optional_uses_libraries=[]): #pylint: disable=dangerous-default-value
doc = minidom.parseString(xml)
try:
relax = False
manifest_check.enforce_uses_libraries(
- doc, uses_libraries, optional_uses_libraries, relax, False,
- 'path/to/X/AndroidManifest.xml')
+ doc, uses_libraries, optional_uses_libraries, missing_optional_uses_libraries,
+ relax, False, 'path/to/X/AndroidManifest.xml')
manifest_check.enforce_uses_libraries(apk, uses_libraries,
optional_uses_libraries,
+ missing_optional_uses_libraries,
relax, True,
'path/to/X/X.apk')
return True
@@ -102,6 +104,15 @@
matches = self.run_test(xml, apk, optional_uses_libraries=['foo'])
self.assertFalse(matches)
+ def test_expected_missing_optional_uses_library(self):
+ xml = self.xml_tmpl % (
+ uses_library_xml('foo') + uses_library_xml('missing') + uses_library_xml('bar'))
+ apk = self.apk_tmpl % (
+ uses_library_apk('foo') + uses_library_apk('missing') + uses_library_apk('bar'))
+ matches = self.run_test(xml, apk, optional_uses_libraries=['foo', 'bar'],
+ missing_optional_uses_libraries=['missing'])
+ self.assertFalse(matches)
+
def test_missing_uses_library(self):
xml = self.xml_tmpl % ('')
apk = self.apk_tmpl % ('')