From 1b6a3cfea4ac2a3a82f4b384b1dd526af0f32c12 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 24 Jul 2018 14:51:30 -0700 Subject: [PATCH] Set targetSdkVersion in manifest_fixer If targetSdkVersion is not set in the manifest, set it to the value it was implicitly using before changing minSdkVersion. Requires passing --library to manifest_fixer.py to distinguish between apps, where the implicit value was set by aapt2 to current, or libraries where the implicit value was 1. Fixes cases where the manifest does not specify targetSdkVersion and was inheriting the minSdkVersion value until manifest merger started merging a lower targetSdkVersion value from a library. Bug: 111347801 Test: manifest_fixer_test.py Change-Id: I8fcf0c5f452707565ba1808f6fe552ffed055c47 --- java/aar.go | 9 ++- java/android_manifest.go | 13 +++- scripts/manifest_fixer.py | 29 +++++--- scripts/manifest_fixer_test.py | 121 +++++++++++++++++++++++++-------- 4 files changed, 131 insertions(+), 41 deletions(-) diff --git a/java/aar.go b/java/aar.go index 0cfc585e8..de67da612 100644 --- a/java/aar.go +++ b/java/aar.go @@ -67,6 +67,7 @@ type aapt struct { rroDirs android.Paths rTxt android.Path extraAaptPackagesFile android.Path + isLibrary bool aaptProperties aaptProperties } @@ -167,13 +168,16 @@ func (a *aapt) buildActions(ctx android.ModuleContext, sdkContext sdkContext, ex manifestFile := proptools.StringDefault(a.aaptProperties.Manifest, "AndroidManifest.xml") manifestSrcPath := android.PathForModuleSrc(ctx, manifestFile) - manifestPath := manifestMerger(ctx, manifestSrcPath, sdkContext, staticLibManifests) + manifestPath := manifestMerger(ctx, manifestSrcPath, sdkContext, staticLibManifests, a.isLibrary) linkFlags, linkDeps, resDirs, overlayDirs, rroDirs := a.aapt2Flags(ctx, sdkContext, manifestPath) linkFlags = append(linkFlags, libFlags...) linkDeps = append(linkDeps, libDeps...) linkFlags = append(linkFlags, extraLinkFlags...) + if a.isLibrary { + linkFlags = append(linkFlags, "--static-lib") + } packageRes := android.PathForModuleOut(ctx, "package-res.apk") srcJar := android.PathForModuleGen(ctx, "R.jar") @@ -286,7 +290,8 @@ func (a *AndroidLibrary) DepsMutator(ctx android.BottomUpMutatorContext) { } func (a *AndroidLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) { - a.aapt.buildActions(ctx, sdkContext(a), "--static-lib") + a.isLibrary = true + a.aapt.buildActions(ctx, sdkContext(a)) ctx.CheckbuildFile(a.proguardOptionsFile) ctx.CheckbuildFile(a.exportPackage) diff --git a/java/android_manifest.go b/java/android_manifest.go index 8fcdcbac1..77b6214e1 100644 --- a/java/android_manifest.go +++ b/java/android_manifest.go @@ -16,6 +16,7 @@ package java import ( "android/soong/java/config" + "strings" "github.com/google/blueprint" @@ -24,10 +25,10 @@ import ( var manifestFixerRule = pctx.AndroidStaticRule("manifestFixer", blueprint.RuleParams{ - Command: `${config.ManifestFixerCmd} --minSdkVersion ${minSdkVersion} $usesLibraries $in $out`, + Command: `${config.ManifestFixerCmd} --minSdkVersion ${minSdkVersion} $args $in $out`, CommandDeps: []string{"${config.ManifestFixerCmd}"}, }, - "minSdkVersion", "usesLibraries") + "minSdkVersion", "args") var manifestMergerRule = pctx.AndroidStaticRule("manifestMerger", blueprint.RuleParams{ @@ -38,7 +39,12 @@ var manifestMergerRule = pctx.AndroidStaticRule("manifestMerger", "libs") func manifestMerger(ctx android.ModuleContext, manifest android.Path, sdkContext sdkContext, - staticLibManifests android.Paths) android.Path { + staticLibManifests android.Paths, isLibrary bool) android.Path { + + var args []string + if isLibrary { + args = append(args, "--library") + } // Inject minSdkVersion into the manifest fixedManifest := android.PathForModuleOut(ctx, "manifest_fixer", "AndroidManifest.xml") @@ -48,6 +54,7 @@ func manifestMerger(ctx android.ModuleContext, manifest android.Path, sdkContext Output: fixedManifest, Args: map[string]string{ "minSdkVersion": sdkVersionOrDefault(ctx, sdkContext.minSdkVersion()), + "args": strings.Join(args, " "), }, }) manifest = fixedManifest diff --git a/scripts/manifest_fixer.py b/scripts/manifest_fixer.py index 6af0ca900..25f96cda2 100755 --- a/scripts/manifest_fixer.py +++ b/scripts/manifest_fixer.py @@ -49,6 +49,8 @@ def parse_args(): parser = argparse.ArgumentParser() parser.add_argument('--minSdkVersion', default='', dest='min_sdk_version', help='specify minSdkVersion used by the build system') + parser.add_argument('--library', dest='library', action='store_true', + help='manifest is for a static library') parser.add_argument('--uses-library', dest='uses_libraries', action='append', help='specify additional tag to add') parser.add_argument('input', help='input AndroidManifest.xml file') @@ -126,7 +128,7 @@ def get_indent(element, default_level): return indent -def raise_min_sdk_version(doc, requested): +def raise_min_sdk_version(doc, requested, library): """Ensure the manifest contains a tag with a minSdkVersion. Args: @@ -153,16 +155,27 @@ def raise_min_sdk_version(doc, requested): # other children of the tag. manifest.insertBefore(doc.createTextNode(indent), manifest.firstChild) - # Get or insert the minSdkVersion attribute + # Get or insert the minSdkVersion attribute. If it is already present, make + # sure it as least the requested value. min_attr = element.getAttributeNodeNS(android_ns, 'minSdkVersion') if min_attr is None: min_attr = doc.createAttributeNS(android_ns, 'android:minSdkVersion') - min_attr.value = '1' - element.setAttributeNode(min_attr) - - # Update the value of the minSdkVersion attribute if necessary - if compare_version_gt(requested, min_attr.value): min_attr.value = requested + element.setAttributeNode(min_attr) + else: + if compare_version_gt(requested, min_attr.value): + min_attr.value = requested + + # Insert the targetSdkVersion attribute if it is missing. If it is already + # present leave it as is. + target_attr = element.getAttributeNodeNS(android_ns, 'targetSdkVersion') + if target_attr is None: + target_attr = doc.createAttributeNS(android_ns, 'android:targetSdkVersion') + if library: + target_attr.value = '1' + else: + target_attr.value = requested + element.setAttributeNode(target_attr) def add_uses_libraries(doc, new_uses_libraries): @@ -230,7 +243,7 @@ def main(): ensure_manifest_android_ns(doc) if args.min_sdk_version: - raise_min_sdk_version(doc, args.min_sdk_version) + raise_min_sdk_version(doc, args.min_sdk_version, args.library) if args.uses_libraries: add_uses_libraries(doc, args.uses_libraries) diff --git a/scripts/manifest_fixer_test.py b/scripts/manifest_fixer_test.py index 54a3784a9..590899719 100755 --- a/scripts/manifest_fixer_test.py +++ b/scripts/manifest_fixer_test.py @@ -54,9 +54,9 @@ class CompareVersionGtTest(unittest.TestCase): class RaiseMinSdkVersionTest(unittest.TestCase): """Unit tests for raise_min_sdk_version function.""" - def raise_min_sdk_version_test(self, input_manifest, min_sdk_version): + def raise_min_sdk_version_test(self, input_manifest, min_sdk_version, library): doc = minidom.parseString(input_manifest) - manifest_fixer.raise_min_sdk_version(doc, min_sdk_version) + manifest_fixer.raise_min_sdk_version(doc, min_sdk_version, library) output = StringIO.StringIO() manifest_fixer.write_xml(output, doc) return output.getvalue() @@ -67,67 +67,130 @@ class RaiseMinSdkVersionTest(unittest.TestCase): '%s' '\n') - def uses_sdk(self, v, extra=''): + # pylint: disable=redefined-builtin + def uses_sdk(self, min=None, target=None, extra=''): + attrs = "" + if min: + attrs += ' android:minSdkVersion="%s"' % (min) + if target: + attrs += ' android:targetSdkVersion="%s"' % (target) if extra: - extra = ' ' + extra - return ' \n' % (v, extra) + attrs += ' ' + extra + return ' \n' % (attrs) def test_no_uses_sdk(self): """Tests inserting a uses-sdk element into a manifest.""" manifest_input = self.manifest_tmpl % '' - expected = self.manifest_tmpl % self.uses_sdk('28') - output = self.raise_min_sdk_version_test(manifest_input, '28') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='28') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) self.assertEqual(output, expected) def test_no_min(self): """Tests inserting a minSdkVersion attribute into a uses-sdk element.""" manifest_input = self.manifest_tmpl % ' \n' - expected = self.manifest_tmpl % self.uses_sdk('28', 'extra="foo"') - output = self.raise_min_sdk_version_test(manifest_input, '28') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='28', + extra='extra="foo"') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) self.assertEqual(output, expected) def test_raise_min(self): """Tests inserting a minSdkVersion attribute into a uses-sdk element.""" - manifest_input = self.manifest_tmpl % self.uses_sdk('27') - expected = self.manifest_tmpl % self.uses_sdk('28') - output = self.raise_min_sdk_version_test(manifest_input, '28') + manifest_input = self.manifest_tmpl % self.uses_sdk(min='27') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='27') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) self.assertEqual(output, expected) def test_raise(self): """Tests raising a minSdkVersion attribute.""" - manifest_input = self.manifest_tmpl % self.uses_sdk('27') - expected = self.manifest_tmpl % self.uses_sdk('28') - output = self.raise_min_sdk_version_test(manifest_input, '28') + manifest_input = self.manifest_tmpl % self.uses_sdk(min='27') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='27') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) self.assertEqual(output, expected) def test_no_raise_min(self): """Tests a minSdkVersion that doesn't need raising.""" - manifest_input = self.manifest_tmpl % self.uses_sdk('28') - expected = manifest_input - output = self.raise_min_sdk_version_test(manifest_input, '27') + manifest_input = self.manifest_tmpl % self.uses_sdk(min='28') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='28') + output = self.raise_min_sdk_version_test(manifest_input, '27', False) self.assertEqual(output, expected) def test_raise_codename(self): """Tests raising a minSdkVersion attribute to a codename.""" - manifest_input = self.manifest_tmpl % self.uses_sdk('28') - expected = self.manifest_tmpl % self.uses_sdk('P') - output = self.raise_min_sdk_version_test(manifest_input, 'P') + manifest_input = self.manifest_tmpl % self.uses_sdk(min='28') + expected = self.manifest_tmpl % self.uses_sdk(min='P', target='28') + output = self.raise_min_sdk_version_test(manifest_input, 'P', False) self.assertEqual(output, expected) def test_no_raise_codename(self): """Tests a minSdkVersion codename that doesn't need raising.""" - manifest_input = self.manifest_tmpl % self.uses_sdk('P') - expected = manifest_input - output = self.raise_min_sdk_version_test(manifest_input, '28') + manifest_input = self.manifest_tmpl % self.uses_sdk(min='P') + expected = self.manifest_tmpl % self.uses_sdk(min='P', target='P') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) self.assertEqual(output, expected) + def test_target(self): + """Tests an existing targetSdkVersion is preserved.""" + + manifest_input = self.manifest_tmpl % self.uses_sdk(min='26', target='27') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='27') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) + self.assertEqual(output, expected) + + def test_no_target(self): + """Tests inserting targetSdkVersion when minSdkVersion exists.""" + + manifest_input = self.manifest_tmpl % self.uses_sdk(min='27') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='27') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) + self.assertEqual(output, expected) + + def test_target_no_min(self): + """Tests inserting targetSdkVersion when minSdkVersion exists.""" + + manifest_input = self.manifest_tmpl % self.uses_sdk(target='27') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='27') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) + self.assertEqual(output, expected) + + def test_no_target_no_min(self): + """Tests inserting targetSdkVersion when minSdkVersion does not exist.""" + + manifest_input = self.manifest_tmpl % '' + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='28') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) + self.assertEqual(output, expected) + + def test_library_no_target(self): + """Tests inserting targetSdkVersion when minSdkVersion exists.""" + + manifest_input = self.manifest_tmpl % self.uses_sdk(min='27') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='27') + output = self.raise_min_sdk_version_test(manifest_input, '28', True) + self.assertEqual(output, expected) + + def test_library_target_no_min(self): + """Tests inserting targetSdkVersion when minSdkVersion exists.""" + + manifest_input = self.manifest_tmpl % self.uses_sdk(target='27') + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='27') + output = self.raise_min_sdk_version_test(manifest_input, '28', True) + self.assertEqual(output, expected) + + def test_library_no_target_no_min(self): + """Tests inserting targetSdkVersion when minSdkVersion does not exist.""" + + manifest_input = self.manifest_tmpl % '' + expected = self.manifest_tmpl % self.uses_sdk(min='28', target='1') + output = self.raise_min_sdk_version_test(manifest_input, '28', True) + self.assertEqual(output, expected) + def test_extra(self): """Tests that extra attributes and elements are maintained.""" @@ -136,12 +199,13 @@ class RaiseMinSdkVersionTest(unittest.TestCase): ' \n' ' \n') + # pylint: disable=line-too-long expected = self.manifest_tmpl % ( ' \n' - ' \n' + ' \n' ' \n') - output = self.raise_min_sdk_version_test(manifest_input, '28') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) self.assertEqual(output, expected) @@ -150,11 +214,12 @@ class RaiseMinSdkVersionTest(unittest.TestCase): manifest_input = self.manifest_tmpl % ' \n' + # pylint: disable=line-too-long expected = self.manifest_tmpl % ( - ' \n' + ' \n' ' \n') - output = self.raise_min_sdk_version_test(manifest_input, '28') + output = self.raise_min_sdk_version_test(manifest_input, '28', False) self.assertEqual(output, expected)