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

build/android/gyp/javac.py

Index: build/android/gyp/javac.py
diff --git a/build/android/gyp/javac.py b/build/android/gyp/javac.py
index 728c49d5eef513191a56f85993dbf368898ffbfe..61860c9e90538aee115b21c0f985866d71704803 100755
--- a/build/android/gyp/javac.py
+++ b/build/android/gyp/javac.py
@@ -12,6 +12,7 @@ import sys
 import textwrap
 
 from util import build_utils
+from util import md5_check
 
 import jar
 
@@ -108,6 +109,35 @@ def _CreateManifest(manifest_path, classpath, main_class=None,
     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:
@@ -122,8 +152,10 @@ def _ExtractClassFiles(jar_path, dest_dir, 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)
 
@@ -146,22 +178,24 @@ def _OnStaleMd5(changes, options, javac_cmd, java_files, classpath_inputs,
       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