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

2 files changed, 113 insertions, 35 deletions

diff --git a/scripts/manifest_fixer.py b/scripts/manifest_fixer.py
index 6af0ca90..25f96cda 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 <uses-library> 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 <uses-sdk> tag with a minSdkVersion.
 
   Args:
@@ -153,16 +155,27 @@ def raise_min_sdk_version(doc, requested):
     # other children of the <manifest> 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'
+    min_attr.value = requested
     element.setAttributeNode(min_attr)
+  else:
+    if compare_version_gt(requested, min_attr.value):
+      min_attr.value = requested
 
-  # Update the value of the minSdkVersion attribute if necessary
-  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 54a3784a..59089971 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'
       '</manifest>\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 '    <uses-sdk android:minSdkVersion="%s"%s/>\n' % (v, extra)
+        attrs += ' ' + extra
+    return '    <uses-sdk%s/>\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 % '    <uses-sdk extra="foo"/>\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):
         '    <uses-sdk android:minSdkVersion="27" extra="foo"/>\n'
         '    <application/>\n')
 
+    # pylint: disable=line-too-long
     expected = self.manifest_tmpl % (
         '    <!-- comment -->\n'
-        '    <uses-sdk android:minSdkVersion="28" extra="foo"/>\n'
+        '    <uses-sdk android:minSdkVersion="28" android:targetSdkVersion="27" extra="foo"/>\n'
         '    <application/>\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 % '  <!-- comment -->\n'
 
+    # pylint: disable=line-too-long
     expected = self.manifest_tmpl % (
-        '  <uses-sdk android:minSdkVersion="28"/>\n'
+        '  <uses-sdk android:minSdkVersion="28" android:targetSdkVersion="28"/>\n'
         '  <!-- comment -->\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)