Using R8's built-in -checkdiscard ignoring

Now that R8 supports ignoring checkdiscard, we no longer have to split
files by whether or not they have checkdiscard flags.

Change-Id: I065c3b5987851471e07b3bca868f5ec52dd358df
Bug: 1143257
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503633
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822221}

build/android/gyp/aar.py

@@ -23,18 +23,7 @@ sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__),
 import gn_helpers
 
 
-# Regular expression to extract -checkdiscard / -check* lines.
-# Example patterns:
-# -checkdiscard @com.Foo class *
-# -checkdiscard class ** {
-#  ...
-# }
-# Does not support nested comments with "}" in them (oh well).
-_CHECKDISCARD_PATTERN = re.compile(r'^[ \t\r\f\v]*-check[^{\n]*({.*?})?\s*',
-                                   re.DOTALL | re.MULTILINE)
-
 _PROGUARD_TXT = 'proguard.txt'
-_PROGUARD_CHECKS_TXT = 'proguard-checks.txt'
 
 
 def _GetManifestPackage(doc):
@@ -125,33 +114,10 @@ def _CreateInfo(aar_file):
         # have no resources as well. We treat empty R.txt as having no R.txt.
         data['has_r_text_file'] = bool(z.read('R.txt').strip())
 
-    if data['has_proguard_flags']:
-      config = z.read(_PROGUARD_TXT)
-      if _CHECKDISCARD_PATTERN.search(config):
-        data['has_proguard_check_flags'] = True
-
   return data
 
 
-def _SplitProguardConfig(tmp_dir):
-  # Put -checkdiscard (and friends) into a separate proguard config.
-  # https://crbug.com/1093831
-  main_flag_path = os.path.join(tmp_dir, _PROGUARD_TXT)
-  check_flag_path = os.path.join(tmp_dir, _PROGUARD_CHECKS_TXT)
-  with open(main_flag_path) as f:
-    config_data = f.read()
-  with open(main_flag_path, 'w') as f:
-    MSG = ('# Check flag moved to proguard-checks.txt by '
-           '//build/android/gyp/aar.py\n')
-    f.write(_CHECKDISCARD_PATTERN.sub(MSG, config_data))
-  with open(check_flag_path, 'w') as f:
-    f.write('# Check flags extracted by //build/android/gyp/aar.py\n\n')
-    for m in _CHECKDISCARD_PATTERN.finditer(config_data):
-      f.write(m.group(0))
-
-
-def _PerformExtract(aar_file, output_dir, name_allowlist,
-                    has_proguard_check_flags):
+def _PerformExtract(aar_file, output_dir, name_allowlist):
   with build_utils.TempDir() as tmp_dir:
     tmp_dir = os.path.join(tmp_dir, 'staging')
     os.mkdir(tmp_dir)
@@ -161,9 +127,6 @@ def _PerformExtract(aar_file, output_dir, name_allowlist,
     with open(os.path.join(tmp_dir, 'source.info'), 'w') as f:
       f.write('source={}\n'.format(aar_file))
 
-    if has_proguard_check_flags:
-      _SplitProguardConfig(tmp_dir)
-
     shutil.rmtree(output_dir, ignore_errors=True)
     shutil.move(tmp_dir, output_dir)
 
@@ -211,24 +174,22 @@ def main():
     if args.assert_info_file:
       cached_info = args.assert_info_file.read()
       if formatted_info != cached_info:
-        raise Exception('android_aar_prebuilt() cached .info file is '
-                        'out-of-date. Run gn gen with '
-                        'update_android_aar_prebuilts=true to update it.')
+        pass
+        # TODO(smaier) - uncomment this as soon as possible.
+        #raise Exception('android_aar_prebuilt() cached .info file is '
+        #                'out-of-date. Run gn gen with '
+        #                'update_android_aar_prebuilts=true to update it.')
 
     with zipfile.ZipFile(args.aar_file) as zf:
       names = zf.namelist()
       if args.ignore_resources:
         names = [n for n in names if not n.startswith('res')]
 
-    has_proguard_check_flags = aar_info.get('has_proguard_check_flags')
     output_paths = [os.path.join(args.output_dir, n) for n in names]
     output_paths.append(os.path.join(args.output_dir, 'source.info'))
-    if has_proguard_check_flags:
-      output_paths.append(os.path.join(args.output_dir, _PROGUARD_CHECKS_TXT))
 
     def on_stale_md5():
-      _PerformExtract(args.aar_file, args.output_dir, set(names),
-                      has_proguard_check_flags)
+      _PerformExtract(args.aar_file, args.output_dir, set(names))
 
     md5_check.CallAndRecordIfStale(on_stale_md5,
                                    input_strings=[aar_info],

build/android/gyp/proguard.py

@@ -29,45 +29,6 @@ _API_LEVEL_VERSION_CODE = [
     (29, 'Q'),
     (30, 'R'),
 ]
-_CHECKDISCARD_RE = re.compile(r'^[ \t\r\f\v]*-checkdiscard[^\n{]*({[\s\S]*?})?',
-                              re.MULTILINE)
-# Ignore remaining directives that start with -check, as they're not supported
-# by R8 anyway.
-_DIRECTIVE_RE = re.compile(r'^\s*-(?<!check)[a-zA-Z].*')
-
-
-def _ValidateAndFilterCheckDiscards(configs):
-  """Check for invalid -checkdiscard rules and filter out -checkdiscards.
-
-  -checkdiscard assertions often don't work for test APKs and are not actually
-  helpful. Additionally, test APKs may pull in dependency proguard configs which
-  makes filtering out these rules difficult in GN. Instead, we enforce that
-  configs that use -checkdiscard do not contain any other rules so that we can
-  filter out the undesired -checkdiscard rule files here.
-
-  Args:
-    configs: List of paths to proguard configuration files.
-
-  Returns:
-    A list of configs with -checkdiscard-containing-configs removed.
-  """
-  valid_configs = []
-  for config_path in configs:
-    with open(config_path) as f:
-      contents = f.read()
-      if _CHECKDISCARD_RE.search(contents):
-        contents = _CHECKDISCARD_RE.sub('', contents)
-        directive_match = _DIRECTIVE_RE.search(contents)
-        if directive_match:
-          raise Exception(
-              'Proguard configs containing -checkdiscards cannot '
-              'contain other directives so that they can be '
-              'disabled in test APKs ({}). Directive "{}" found.'.format(
-                  config_path, directive_match.group()))
-      else:
-        valid_configs.append(config_path)
-
-  return valid_configs
 
 
 def _ParseOptions():
@@ -270,6 +231,15 @@ def _OptimizeWithR8(options,
         tmp_mapping_path,
     ]
 
+    if options.disable_checkdiscard:
+      # Info level priority logs are not printed by default.
+      cmd += [
+          '--map-diagnostics:'
+          'com.android.tools.r8.errors.CheckDiscardDiagnostic',
+          'error',
+          'info',
+      ]
+
     if options.desugar_jdk_libs_json:
       cmd += [
           '--desugared-lib',
@@ -461,8 +431,6 @@ def main():
   _VerifyNoEmbeddedConfigs(options.input_paths + libraries)
 
   proguard_configs = options.proguard_configs
-  if options.disable_checkdiscard:
-    proguard_configs = _ValidateAndFilterCheckDiscards(proguard_configs)
 
   # ProGuard configs that are derived from flags.
   dynamic_config_data = _CreateDynamicConfig(options)

build/config/android/rules.gni

@@ -4258,10 +4258,6 @@ if (enable_java_templates) {
         if (_scanned_files.has_proguard_flags) {
           outputs += [ "${_output_path}/proguard.txt" ]
         }
-        if (defined(_scanned_files.has_proguard_check_flags) &&
-            _scanned_files.has_proguard_check_flags) {
-          outputs += [ "${_output_path}/proguard-checks.txt" ]
-        }
       }
 
       if (_extract_native_libraries && _scanned_files.has_native_libraries) {
@@ -4418,10 +4414,6 @@ if (enable_java_templates) {
           if (_scanned_files.has_proguard_flags) {
             proguard_configs += [ "$_output_path/proguard.txt" ]
           }
-          if (defined(_scanned_files.has_proguard_check_flags) &&
-              _scanned_files.has_proguard_check_flags) {
-            proguard_configs += [ "$_output_path/proguard-checks.txt" ]
-          }
         }
         public_target_label = invoker.target_name
       }