Fix javac --incremental by using jmake for dependency analysis

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 deb90c5de64e03fb28c562146625da3865833df7..5632db19cda042bb74e4ad3dc51c69ac46ac707d 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
 
@@ -122,10 +123,49 @@ 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'
+    return not any(p.endswith(partial_java_path) for p in java_files)
 
   build_utils.ExtractAll(jar_path, path=dest_dir, predicate=extract_predicate)
+  for path in build_utils.FindInDirectory(dest_dir, '*.class'):
+    shutil.copystat(jar_path, path)
+
+
+def _ConvertToJMakeArgs(javac_cmd, pdb_path):
+  compiler = javac_cmd[0]
+  new_args = ['-C' + arg for arg in javac_cmd]

[Yaron, 2015/09/30 02:19:25]

I feel like these lines would be a little clearer as:
new_args = ['bin/jmake', '-pdb', pdb_path] +  ['-C' + arg for arg in
javac_cmd[1:]]
followed by the conditionals

[agrieve, 2015/10/01 16:26:11]

Done.

+  if compiler != 'javac':
+    new_args.extend(('-jcexec', new_args[0]))
+  new_args[0] = 'bin/jmake'
+  new_args.extend(('-pdb', pdb_path))
+  if md5_check.PRINT_EXPLANATIONS:
+    new_args.append('-Xtiming')
+
+  def revert_arg(index):
+    new_args[index] = javac_cmd[index]
+    new_args[index + 1] = javac_cmd[index + 1]
+
+  # Unprefix classpath args.

[Yaron, 2015/09/30 02:19:25]

Yuck :(

[agrieve, 2015/10/01 16:26:11]

Done.

+  revert_arg(javac_cmd.index('-classpath'))
+  if '-bootclasspath' in javac_cmd:
+    revert_arg(javac_cmd.index('-bootclasspath'))
+  return new_args
+
+
+def _FilterJMakeOutput(stdout):
+  if md5_check.PRINT_EXPLANATIONS:
+    return stdout
+  return re.sub(r'\b(Jmake version|Writing project database).*?\n', '', stdout)
+
+
+def _FixTempPathsInIncrementalMetadata(pdb_path, temp_dir):
+  # The .pdb records absolute paths. Fix up paths witin /tmp (srcjars).

[Yaron, 2015/09/30 02:19:25]

within

[agrieve, 2015/10/01 16:26:11]

Done.

[agrieve, 2015/10/01 16:26:11]

Done.

+  if os.path.exists(pdb_path):
+    with open(pdb_path) as fileobj:

[Yaron, 2015/09/30 02:19:25]

use fileinput.input:
http://stackoverflow.com/questions/5453267/is-it-possible-to-modify-lines-in-...
?

[agrieve, 2015/10/01 16:26:11]

It's a binary file. Added a comment.

+      pdb_data = fileobj.read()
+    with open(pdb_path, 'w') as fileobj:
+      fileobj.write(re.sub(r'/tmp/[^/]*', temp_dir, pdb_data))
 
 
 def _OnStaleMd5(changes, options, javac_cmd, java_files, classpath_inputs,
@@ -140,35 +180,58 @@ def _OnStaleMd5(changes, options, javac_cmd, java_files, classpath_inputs,
     os.makedirs(classes_dir)
 
     changed_paths = None
+    # jmake can handle deleted files, but it's a rare case and it would
+    # complicate this script's logic.
     if options.incremental and changes.AddedOrModifiedOnly():
       changed_paths = set(changes.IterChangedPaths())
       # Do a full compile if classpath has changed.
+      # jmake doesn't seem to do this on its own... Might be that ijars mess up
+      # its change-detection logic.
       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 options.incremental:
+      # jmake is a compiler wrapper that figures out the minimal set of .java
+      # files that need to be rebuilt given a set of .java files that have
+      # changed.
+      # jmake determines what files are stale based on timestamps between .java
+      # and .class files. Since we use .jars, .srcjars, and md5 checks,
+      # timestamp info isn't accurate for this purpose. Rather than use jmake's
+      # programatic interface (like we eventually should), we ensure that all
+      # .class files are newer than their .java files, and convey to jmake which
+      # sources are stale by having their .class files be missing entirely
+      # (by not extracting them).
+      pdb_path = options.jar_path + '.pdb'
+      javac_cmd = _ConvertToJMakeArgs(javac_cmd, pdb_path)
+      if srcjars:
+        _FixTempPathsInIncrementalMetadata(pdb_path, temp_dir)
 
     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))
+      jar_srcs = _FilterJavaFiles(jar_srcs, options.javac_includes)
+      java_files.extend(jar_srcs)
+      if changed_paths:
+        # Set the mtime of all sources to 0 since we use the absense of .class
+        # files to tell jmake which files are stale.

[Yaron, 2015/09/30 02:19:25]

how does this impact non-incremental builds which don't use jmake?

[agrieve, 2015/10/01 16:26:11]

It won't be hit since changed_paths == None for non-incremental.

+        for path in jar_srcs:
+          os.utime(path, (0, 0))
 
     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.
+        changed_java_files = [p for p in java_files if p in changed_paths]
+        if os.path.exists(options.jar_path):
+          _ExtractClassFiles(options.jar_path, classes_dir, changed_java_files)
+        if os.path.exists(excluded_jar_path):
+          _ExtractClassFiles(excluded_jar_path, classes_dir, changed_java_files)
+        # Add the extracted files to the classpath. This is required because
+        # when compiling only a subset of files, classes that haven't changed
+        # need to be findable.
         classpath_idx = javac_cmd.index('-classpath')
         javac_cmd[classpath_idx + 1] += ':' + classes_dir
 
@@ -179,6 +242,7 @@ def _OnStaleMd5(changes, options, javac_cmd, java_files, classpath_inputs,
       build_utils.CheckOutput(
           cmd,
           print_stdout=options.chromium_code,
+          stdout_filter=_FilterJMakeOutput,
           stderr_filter=ColorJavacOutput)
 
     if options.main_class or options.manifest_entry:
@@ -361,6 +425,8 @@ def main(argv):
       options.jar_path,
       options.jar_path.replace('.jar', '.excluded.jar'),
   ]
+  if options.incremental:
+    output_paths.append(options.jar_path + '.pdb')
 
   # An escape hatch to be able to check if incremental compiles are causing
   # problems.