Android: compile_resources & create_app_bundle py2->py3

* Adds workaround to print_python_deps.py so that it can import protobuf
* Tweaks manifest expectation logic to sort android:name first
  * Necessary because Python 3.8+ no longer automatically sorts
    attributes.
  * android:name was not sorted first previously because there was no
    way to sort nodes. Having it first is better.
* Updates android build pytests to run under python3.

Bug: 1187279
Change-Id: Icde8563e58a9690b5e9ea6dd69f546da06f43a68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2852040
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#876676}

build/android/PRESUBMIT.py

@@ -105,7 +105,8 @@ def J(*dirs):
               J('pylib', 'utils', 'test_filter_test.py'),
               J('.', 'convert_dex_profile_tests.py'),
           ],
-          env=pylib_test_env))
+          env=pylib_test_env,
+          run_on_python2=False))
 
   return input_api.RunTests(tests)
 

build/android/gyp/compile_resources.py

@@ -1,5 +1,5 @@
-#!/usr/bin/env python
-# encoding: utf-8
+#!/usr/bin/env python3
+#
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
@@ -317,7 +317,7 @@ def _RenameLocaleResourceDirs(resource_dirs, path_info):
     * Modern ISO 639-1 codes will be renamed to their obsolete variant
       for Indonesian, Hebrew and Yiddish (e.g. 'values-in/ -> values-id/).
 
-    * Norwegian macrolanguage strings will be renamed to Bokmål (main
+    * Norwegian macrolanguage strings will be renamed to Bokmal (main
       Norway language). See http://crbug.com/920960. In practice this
       means that 'values-no/ -> values-nb/' unless 'values-nb/' already
       exists.

build/android/gyp/compile_resources.pydeps

@@ -2,6 +2,8 @@
 #   build/print_python_deps.py --root build/android/gyp --output build/android/gyp/compile_resources.pydeps build/android/gyp/compile_resources.py
 ../../../third_party/jinja2/__init__.py
 ../../../third_party/jinja2/_compat.py
+../../../third_party/jinja2/asyncfilters.py
+../../../third_party/jinja2/asyncsupport.py
 ../../../third_party/jinja2/bccache.py
 ../../../third_party/jinja2/compiler.py
 ../../../third_party/jinja2/defaults.py

build/android/gyp/create_app_bundle.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 #
 # Copyright 2018 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
@@ -7,12 +7,10 @@
 """Create an Android application bundle from one or more bundle modules."""
 
 import argparse
-import itertools
 import json
 import os
 import shutil
 import sys
-import tempfile
 import zipfile
 
 sys.path.append(
@@ -376,7 +374,7 @@ def _WriteBundlePathmap(module_pathmap_paths, module_names,
       if not os.path.exists(module_pathmap_path):
         continue
       module_pathmap = _LoadPathmap(module_pathmap_path)
-      for short_path, long_path in module_pathmap.iteritems():
+      for short_path, long_path in module_pathmap.items():
         rebased_long_path = '{}/{}'.format(module_name, long_path)
         rebased_short_path = '{}/{}'.format(module_name, short_path)
         line = '{} -> {}\n'.format(rebased_long_path, rebased_short_path)

build/android/gyp/create_app_bundle.pydeps

@@ -14,6 +14,8 @@
 ../../../third_party/catapult/devil/devil/utils/cmd_helper.py
 ../../../third_party/jinja2/__init__.py
 ../../../third_party/jinja2/_compat.py
+../../../third_party/jinja2/asyncfilters.py
+../../../third_party/jinja2/asyncsupport.py
 ../../../third_party/jinja2/bccache.py
 ../../../third_party/jinja2/compiler.py
 ../../../third_party/jinja2/defaults.py

build/android/gyp/extract_unwind_tables_tests.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # Copyright 2018 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
@@ -63,7 +63,8 @@ def testExtractCfi(self):
 STACK CFI INIT 3b93214 fffff .cfa: sp 0 + .ra: lr
 STACK CFI 3b93218 .cfa: r7 16 + .ra: .cfa -4 + ^
 """.splitlines()
-      extract_unwind_tables._ParseCfiData(test_data_lines, output_file.name)
+      extract_unwind_tables._ParseCfiData(
+          [l.encode('utf8') for l in test_data_lines], output_file.name)
 
       expected_cfi_data = {
         0xe1a1e4 : [0x2, 0x11, 0x4, 0x50],
@@ -109,8 +110,8 @@ def testExtractCfi(self):
 
         func_start = index + 1
         func_end = func_start + unw_data[index] * 2
-        self.assertEquals(
-            len(expected_cfi_data[func_addr]), func_end - func_start)
+        self.assertEqual(len(expected_cfi_data[func_addr]),
+                         func_end - func_start)
         func_cfi = unw_data[func_start : func_end]
         self.assertEqual(expected_cfi_data[func_addr], func_cfi)
 

build/android/gyp/util/manifest_utils.py

@@ -8,6 +8,7 @@
 import os
 import re
 import shlex
+import sys
 import xml.dom.minidom as minidom
 
 from util import build_utils
@@ -123,38 +124,37 @@ def AssertPackage(manifest_node, package):
 
 
 def _SortAndStripElementTree(root):
-  def sort_key(node):
+  # Sort alphabetically with two exceptions:
+  # 1) Put <application> node last (since it's giant).
+  # 2) Put android:name before other attributes.
+  def element_sort_key(node):
+    if node.tag == 'application':
+      return 'z'
     ret = ElementTree.tostring(node)
     # ElementTree.tostring inserts namespace attributes for any that are needed
     # for the node or any of its descendants. Remove them so as to prevent a
     # change to a child that adds/removes a namespace usage from changing sort
     # order.
     return re.sub(r' xmlns:.*?".*?"', '', ret.decode('utf8'))
 
+  name_attr = '{%s}name' % ANDROID_NAMESPACE
+
+  def attribute_sort_key(tup):
+    return ('', '') if tup[0] == name_attr else tup
+
   def helper(node):
     for child in node:
       if child.text and child.text.isspace():
         child.text = None
       helper(child)
-    node[:] = sorted(node, key=sort_key)
 
-  def rename_attrs(node, from_name, to_name):
-    value = node.attrib.get(from_name)
-    if value is not None:
-      node.attrib[to_name] = value
-      del node.attrib[from_name]
-    for child in node:
-      rename_attrs(child, from_name, to_name)
+    # Sort attributes (requires Python 3.8+).
+    node.attrib = dict(sorted(node.attrib.items(), key=attribute_sort_key))
+
+    # Sort nodes
+    node[:] = sorted(node, key=element_sort_key)
 
-  # Sort alphabetically with two exceptions:
-  # 1) Put <application> node last (since it's giant).
-  # 2) Pretend android:name appears before other attributes.
-  app_node = root.find('application')
-  app_node.tag = 'zz'
-  rename_attrs(root, '{%s}name' % ANDROID_NAMESPACE, '__name__')
   helper(root)
-  rename_attrs(root, '__name__', '{%s}name' % ANDROID_NAMESPACE)
-  app_node.tag = 'application'
 
 
 def _SplitElement(line):
@@ -269,7 +269,7 @@ def NormalizeManifest(manifest_contents):
     # Trichrome's static library version number is updated daily. To avoid
     # frequent manifest check failures, we remove the exact version number
     # during normalization.
-    for node in app_node.getchildren():
+    for node in app_node:
       if (node.tag in ['uses-static-library', 'static-library']
           and '{%s}version' % ANDROID_NAMESPACE in node.keys()
           and '{%s}name' % ANDROID_NAMESPACE in node.keys()):
@@ -281,14 +281,14 @@ def blur_package_name(node):
     for key in node.keys():
       node.set(key, node.get(key).replace(package, '$PACKAGE'))
 
-    for child in node.getchildren():
+    for child in node:
       blur_package_name(child)
 
   # We only blur the package names of non-root nodes because they generate a lot
   # of diffs when doing manifest checks for upstream targets. We still want to
   # have 1 piece of package name not blurred just in case the package name is
   # mistakenly changed.
-  for child in root.getchildren():
+  for child in root:
     blur_package_name(child)
 
   _SortAndStripElementTree(root)

build/android/gyp/util/manifest_utils_test.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env vpython
+#!/usr/bin/env python3
 # Copyright 2020 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
@@ -49,18 +49,18 @@
 _TEST_MANIFEST_NORMALIZED = """\
 <?xml version="1.0" ?>
 <manifest
-    package="test.pkg"
-    tools:ignore="MissingVersion"
     xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:tools="http://schemas.android.com/tools">
+    xmlns:tools="http://schemas.android.com/tools"
+    package="test.pkg"
+    tools:ignore="MissingVersion">
   <uses-feature android:name="android.hardware.vr.headtracking" \
 android:required="false" android:version="1"/>
   <uses-sdk android:minSdkVersion="24" android:targetSdkVersion="30"/>
   <application android:name="testname">
     <activity  # DIFF-ANCHOR: {activity_diff_anchor}
+        android:name="to be hashed"
         {extra_activity_attr}android:icon="@drawable/ic_devices_48dp"
         android:label="label with spaces"
-        android:name="to be hashed"
         android:theme="@style/Theme.Chromium.Activity.TranslucentNoAnimations">
       <intent-filter>  # DIFF-ANCHOR: {intent_filter_diff_anchor}
         {extra_intent_filter_elem}\
@@ -69,11 +69,11 @@
         <data android:mimeType="text/plain"/>
       </intent-filter>  # DIFF-ANCHOR: {intent_filter_diff_anchor}
     </activity>  # DIFF-ANCHOR: {activity_diff_anchor}
-    <receiver  # DIFF-ANCHOR: 355000d2
-        android:exported="false"
+    <receiver  # DIFF-ANCHOR: ddab3320
         android:name=\
-"org.chromium.chrome.browser.announcement.AnnouncementNotificationManager$Rcvr">
-    </receiver>  # DIFF-ANCHOR: 355000d2
+"org.chromium.chrome.browser.announcement.AnnouncementNotificationManager$Rcvr"
+        android:exported="false">
+    </receiver>  # DIFF-ANCHOR: ddab3320
   </application>
 </manifest>
 """

build/android/gyp/util/resource_utils.py

@@ -1035,7 +1035,7 @@ def GenerateAndroidResourceStringsXml(names_to_utf8_text, namespaces=None):
     for name, utf8_text in sorted(names_to_utf8_text.items()):
       result += '<string name="%s">"%s"</string>\n' % (name, utf8_text)
   result += '</resources>\n'
-  return result
+  return result.encode('utf8')
 
 
 def FilterAndroidResourceStringsXml(xml_file_path, string_predicate):
@@ -1055,7 +1055,7 @@ def FilterAndroidResourceStringsXml(xml_file_path, string_predicate):
   strings_map, namespaces = ParseAndroidResourceStringsFromXml(xml_data)
 
   string_deletion = False
-  for name in strings_map.keys():
+  for name in list(strings_map.keys()):
     if not string_predicate(name):
       del strings_map[name]
       string_deletion = True

build/android/gyp/util/resource_utils_test.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env vpython
+#!/usr/bin/env python3
 # coding: utf-8
 # Copyright 2018 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
@@ -171,7 +171,7 @@ def test_ToAndroidLocaleName(self):
         'yi': 'ji'
     }
     for chromium_locale, android_locale in \
-        _TEST_CHROMIUM_TO_ANDROID_LOCALE_MAP.iteritems():
+        _TEST_CHROMIUM_TO_ANDROID_LOCALE_MAP.items():
       result = resource_utils.ToAndroidLocaleName(chromium_locale)
       self.assertEqual(result, android_locale)
 
@@ -209,7 +209,7 @@ def test_ToChromiumLocaleName(self):
         'no': 'nb',  # http://crbug.com/920960
     }
     for android_locale, chromium_locale in \
-        _TEST_ANDROID_TO_CHROMIUM_LOCALE_MAP.iteritems():
+        _TEST_ANDROID_TO_CHROMIUM_LOCALE_MAP.items():
       result = resource_utils.ToChromiumLocaleName(android_locale)
       self.assertEqual(result, chromium_locale)
 
@@ -240,18 +240,18 @@ def test_ParseAndroidResourceStringsFromXml(self):
   def test_GenerateAndroidResourceStringsXml(self):
     # Fist, an empty strings map, with no namespaces
     result = resource_utils.GenerateAndroidResourceStringsXml({})
-    self.assertEqual(result, _TEST_XML_OUTPUT_EMPTY)
+    self.assertEqual(result.decode('utf8'), _TEST_XML_OUTPUT_EMPTY)
 
     result = resource_utils.GenerateAndroidResourceStringsXml(
         _TEST_RESOURCES_MAP_1, _TEST_NAMESPACES_1)
-    self.assertEqual(result, _TEST_XML_INPUT_1)
+    self.assertEqual(result.decode('utf8'), _TEST_XML_INPUT_1)
 
   @staticmethod
   def _CreateTestResourceFile(output_dir, locale, string_map, namespaces):
     values_dir = os.path.join(output_dir, 'values-' + locale)
     build_utils.MakeDirectory(values_dir)
     file_path = os.path.join(values_dir, 'strings.xml')
-    with open(file_path, 'w') as f:
+    with open(file_path, 'wb') as f:
       file_data = resource_utils.GenerateAndroidResourceStringsXml(
           string_map, namespaces)
       f.write(file_data)

build/android/pylib/utils/dexdump.py

@@ -50,8 +50,12 @@ def Dump(apk_path):
       BAD_XML_CHARS = re.compile(
           u'[\x00-\x08\x0b-\x0c\x0e-\x1f\x7f-\x84\x86-\x9f' +
           u'\ud800-\udfff\ufdd0-\ufddf\ufffe-\uffff]')
-      decoded_xml = output_xml.decode('utf-8', 'replace')
-      clean_xml = BAD_XML_CHARS.sub(u'\ufffd', decoded_xml)
+      if sys.version_info[0] < 3:
+        decoded_xml = output_xml.decode('utf-8', 'replace')
+        clean_xml = BAD_XML_CHARS.sub(u'\ufffd', decoded_xml)
+      else:
+        # Line duplicated to avoid pylint redefined-variable-type error.
+        clean_xml = BAD_XML_CHARS.sub(u'\ufffd', output_xml)
       parsed_dex_files.append(
           _ParseRootNode(ElementTree.fromstring(clean_xml.encode('utf-8'))))
     return parsed_dex_files

build/config/android/internal_rules.gni

@@ -2590,8 +2590,6 @@ if (enable_java_templates) {
       _expectations_target =
           "${invoker.top_target_name}_validate_android_manifest"
       action_with_pydeps(_expectations_target) {
-        # TODO(crbug.com/1187279): Get this to work under Python3.
-        run_under_python2 = true
         _actual_file = "${invoker.android_manifest}.normalized"
         _failure_file =
             "$expectations_failure_dir/" +
@@ -2644,8 +2642,6 @@ if (enable_java_templates) {
     }
 
     action_with_pydeps(target_name) {
-      # TODO(crbug.com/1187279): Get this to work under Python3.
-      run_under_python2 = true
       script = _script
       depfile = "$target_gen_dir/${target_name}.d"
       inputs = _inputs

build/config/android/rules.gni

@@ -4841,8 +4841,6 @@ if (enable_java_templates) {
 
     _bundle_target_name = "${_target_name}__bundle"
     action_with_pydeps(_bundle_target_name) {
-      # TODO(crbug.com/1187279): Get this to work under Python3.
-      run_under_python2 = true
       script = "//build/android/gyp/create_app_bundle.py"
       inputs = _all_module_zip_paths + _all_module_build_configs
       outputs = [ _bundle_path ]

build/print_python_deps.py

@@ -182,6 +182,12 @@ def main():
     vpython_to_use = {2: 'vpython', 3: 'vpython3'}[target_version]
     os.execvp(vpython_to_use, [vpython_to_use] + sys.argv + ['--did-relaunch'])
 
+  if current_version == 3:
+    # Work-around for protobuf library not being loadable via importlib
+    # This is needed due to compile_resources.py.
+    import importlib._bootstrap_external
+    importlib._bootstrap_external._NamespacePath.sort = lambda self, **_: 0
+
   paths_set = set()
   try:
     for module in modules: