Improve javac.py --incremental logic by having it recompile more files

build/android/gyp/javac.py

 #!/usr/bin/env python
 #
 # Copyright 2013 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.
 
 import optparse
 import os
 import shutil
 import re
 import sys
 import textwrap
 
 from util import build_utils
+from util import md5_check
 
 import jar
 
 sys.path.append(build_utils.COLORAMA_ROOT)
 import colorama
 
 
 def ColorJavacOutput(output):
   fileline_prefix = r'(?P<fileline>(?P<file>[-.\w/\\]+.java):(?P<line>[0-9]+):)'
   warning_re = re.compile(
@@ skipping 76 matching lines @@
   wrapper = textwrap.TextWrapper(break_long_words=True,
                                  drop_whitespace=False,
                                  subsequent_indent=' ',
                                  width=_MAX_MANIFEST_LINE_LEN - 2)
   output = '\r\n'.join(w for l in output for w in wrapper.wrap(l))
 
   with open(manifest_path, 'w') as f:
     f.write(output)
 
 
+def _DetermineFilesToRecompile(java_files, changed_paths):
+  # Calculate the set of files that must be recompiled as those within
+  # |changed_paths| as well as those that contain a reference to a class within
+  # |changed_paths|.
+  # This approach only mostly works. It's possible for a constant to be inlined
+  # across multiple files like so:
+  #   class A { static final int foo = 1; }
+  #   class B { static final int bar = A.foo; }
+  #   class C { static final int baz = B.bar; }
+  # In this case, when A changes, A and B will be recompiled, but C will not.
+  changed_java_files = [p for p in java_files if p in changed_paths]
+  java_files_to_recompile = list(changed_java_files)
+
+  changed_class_names = []
+  for path in java_files_to_recompile:
+    class_name, _ = os.path.splitext(os.path.basename(path))
+    changed_class_names.append(class_name)
+
+  class_name_pattern = re.compile(r'\b(?:%s)\b' % '|'.join(changed_class_names))

[Yaron, 2015/09/25 18:34:09]

I'm having trouble thinking this through but make sure you've considered inner
classes here? I guess changed_paths would only ever refer to the top-level java
file so should be ok on that end. In terms of the grep, I think it should also
match top-level class or inner classes so seems ok?

[agrieve, 2015/09/25 18:49:44]

You can't reference an inner class without typing the outer-class's name.

However...

Just thought of the case where you do:  obj.getFoo().doThing()

You never reference the type of getFoo(), yet still depend on it :( 

Back to javap...

+  for path in java_files:
+    if path in changed_paths:
+      continue
+    with open(path, 'rb') as fileobj:
+      data = fileobj.read()
+      if class_name_pattern.search(data):
+        java_files_to_recompile.append(path)
+  return java_files_to_recompile
+
+
 def _ExtractClassFiles(jar_path, dest_dir, java_files):
   """Extracts all .class files not corresponding to |java_files|."""
   # Two challenges exist here:
   # 1. |java_files| have prefixes that are not represented in the the jar paths.
   # 2. A single .java file results in multiple .class files when it contains
   #    nested classes.
   # Here's an example:
   #   source path: ../../base/android/java/src/org/chromium/Foo.java
   #   jar paths: org/chromium/Foo.class, org/chromium/Foo$Inner.class
   # To extract only .class files not related to the given .java files, we strip
   # off ".class" and "$*.class" and use a substring match against java_files.
   def extract_predicate(path):
     if not path.endswith('.class'):
       return False
-    path_without_suffix = re.sub(r'(?:\$[^/]+)?\.class$', '', path)
-    return not any(path_without_suffix in p for p in java_files)
+    path_without_suffix = re.sub(r'(?:\$|\.)[^/]+class$', '', path)
+    partial_java_path = path_without_suffix + '.java'
+    ret = not any(p.endswith(partial_java_path) for p in java_files)
+    return ret

[Yaron, 2015/09/25 18:34:09]

merge with line above

[agrieve, 2015/09/25 18:49:44]

Done.

 
   build_utils.ExtractAll(jar_path, path=dest_dir, predicate=extract_predicate)
 
 
 def _OnStaleMd5(changes, options, javac_cmd, java_files, classpath_inputs,
                 runtime_classpath):
   with build_utils.TempDir() as temp_dir:
     srcjars = options.java_srcjars
     # The .excluded.jar contains .class files excluded from the main jar.
     # It is used for incremental compiles.
     excluded_jar_path = options.jar_path.replace('.jar', '.excluded.jar')
 
     classes_dir = os.path.join(temp_dir, 'classes')
     os.makedirs(classes_dir)
 
     changed_paths = None
     if options.incremental and changes.AddedOrModifiedOnly():
       changed_paths = set(changes.IterChangedPaths())
       # Do a full compile if classpath has changed.
       if any(p in changed_paths for p in classpath_inputs):
         changed_paths = None
       else:
-        java_files = [p for p in java_files if p in changed_paths]
         srcjars = [p for p in srcjars if p in changed_paths]
 
     if srcjars:
       java_dir = os.path.join(temp_dir, 'java')
       os.makedirs(java_dir)
       for srcjar in options.java_srcjars:
-        extract_predicate = None
         if changed_paths:
-          changed_subpaths = set(changes.IterChangedSubpaths(srcjar))
-          extract_predicate = lambda p: p in changed_subpaths
-        build_utils.ExtractAll(srcjar, path=java_dir, pattern='*.java',
-                               predicate=extract_predicate)
+          changed_paths.update(changes.IterChangedSubpaths(srcjar))
+        build_utils.ExtractAll(srcjar, path=java_dir, pattern='*.java')
       jar_srcs = build_utils.FindInDirectory(java_dir, '*.java')
       java_files.extend(_FilterJavaFiles(jar_srcs, options.javac_includes))
 
+    if changed_paths:
+      java_files = _DetermineFilesToRecompile(java_files, changed_paths)
+      if md5_check.PRINT_EXPLANATIONS:
+        print 'Files to recompile:'
+        print '  ' + '\n  '.join(java_files)
+
     if java_files:
       if changed_paths:
         # When no files have been removed and the output jar already
         # exists, reuse .class files from the existing jar.
         _ExtractClassFiles(options.jar_path, classes_dir, java_files)
         _ExtractClassFiles(excluded_jar_path, classes_dir, java_files)
         # Add the extracted files to the classpath.
         classpath_idx = javac_cmd.index('-classpath')
         javac_cmd[classpath_idx + 1] += ':' + classes_dir
 
@@ skipping 196 matching lines @@
       options,
       input_paths=input_paths,
       input_strings=javac_cmd,
       output_paths=output_paths,
       force=force,
       pass_changes=True)
 
 
 if __name__ == '__main__':
   sys.exit(main(sys.argv[1:]))