Fix javac --incremental by using jmake for dependency analysis

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 90 matching lines @@
   # 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'
+    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,
                 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
+    # 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
 
       # Don't include the output directory in the initial set of args since it
       # being in a temp dir makes it unstable (breaks md5 stamping).
       cmd = javac_cmd + ['-d', classes_dir] + java_files
 
       build_utils.CheckOutput(
           cmd,
           print_stdout=options.chromium_code,
+          stdout_filter=_FilterJMakeOutput,
           stderr_filter=ColorJavacOutput)
 
     if options.main_class or options.manifest_entry:
       entries = []
       if options.manifest_entry:
         entries = [e.split(':') for e in options.manifest_entry]
       manifest_file = os.path.join(temp_dir, 'manifest')
       _CreateManifest(manifest_file, runtime_classpath, options.main_class,
                       entries)
     else:
@@ skipping 162 matching lines @@
       else:
         classpath_inputs.append(path)
 
   # Compute the list of paths that when changed, we need to rebuild.
   input_paths = classpath_inputs + options.java_srcjars + java_files
 
   output_paths = [
       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.
   force = int(os.environ.get('DISABLE_INCREMENTAL_JAVAC', 0))
 
   # List python deps in input_strings rather than input_paths since the contents
   # of them does not change what gets written to the depsfile.
   build_utils.CallAndWriteDepfileIfStale(
       lambda changes: _OnStaleMd5(changes, options, javac_cmd, java_files,
                                   classpath_inputs, runtime_classpath),
       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:]))