Parse build files with Jython to avoid shelling out to Python

Summary: Modify BuildFileToJsonParser to use Jython via JSR-223 rather than creating a new process to parse each python build file. Cuts build file parsing time roughly in half and avoids buck depending on the system python.

Test Plan: ant clean compile test && buck clean && buck build buck && buck test --all

Author: jimpurbrick

.classpath

@@ -19,5 +19,7 @@
 	<classpathentry kind="lib" path="lib/hamcrest-core-1.3.jar"/>
 	<classpathentry kind="lib" path="lib/hamcrest-library-1.3.jar"/>
 	<classpathentry kind="lib" path="lib/sdklib.jar"/>
+	<classpathentry kind="lib" path="lib/jyson-1.0.2.jar"/>
+	<classpathentry kind="lib" path="lib/jython-standalone-2.5.3.jar"/>
 	<classpathentry kind="output" path="build/classes"/>
 </classpath>

.gitignore

@@ -7,6 +7,7 @@ local.properties
 
 # Compiled Python
 *.pyc
+*py.class
 
 # IntelliJ, based on http://devnet.jetbrains.net/docs/DOC-1186
 .idea/dictionaries

.idea/libraries/buck_lib.xml

@@ -132,5 +132,7 @@ ${BUCK_DIRECTORY}/lib/jackson-core-2.0.5.jar:\
 ${BUCK_DIRECTORY}/lib/jackson-databind-2.0.5.jar:\
 ${BUCK_DIRECTORY}/lib/jsr305.jar:\
 ${BUCK_DIRECTORY}/lib/sdklib.jar:\
-${BUCK_DIRECTORY}/lib/ddmlib-r21.jar \
+${BUCK_DIRECTORY}/lib/ddmlib-r21.jar:\
+${BUCK_DIRECTORY}/lib/jython-standalone-2.5.3.jar:\
+${BUCK_DIRECTORY}/lib/jyson-1.0.2.jar \
 com.facebook.buck.cli.Main "$@"

bin/buck

@@ -101,6 +101,8 @@
       <include name="hamcrest-core-1.3.jar" />
       <include name="hamcrest-library-1.3.jar" />
       <include name="objenesis-1.2.jar" />
+      <include name="jython-standalone-2.5.3.jar" />
+      <include name="jyson-1.0.2.jar" />
     </fileset>
     <pathelement location="${testclasses.dir}" />
     <pathelement location="${test.dir}" />

build.xml

@@ -145,3 +145,21 @@ prebuilt_jar(
     '//src/com/facebook/buck/shell:shell',
   ]
 )
+
+jython_visibility = [
+  '//src/com/facebook/buck/shell:shell',
+  '//test/com/facebook/buck/cli:testutil',
+  '//test/com/facebook/buck/parser:parser',
+]
+
+prebuilt_jar(
+  name = 'jython',
+  binary_jar = 'jython-standalone-2.5.3.jar',
+  visibility = jython_visibility,
+)
+
+prebuilt_jar(
+  name = 'jyson',
+  binary_jar = 'jyson-1.0.2.jar',
+    visibility = jython_visibility,
+)

lib/BUCK

@@ -17,23 +17,33 @@
 package com.facebook.buck.json;
 
 import com.facebook.buck.util.Ansi;
-import com.facebook.buck.util.InputStreamConsumer;
 import com.fasterxml.jackson.core.JsonFactory;
 import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonToken;
+import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.common.io.Closer;
 
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.PipedReader;
+import java.io.PipedWriter;
 import java.io.Reader;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import javax.script.ScriptEngine;
+import javax.script.ScriptEngineManager;
+import javax.script.ScriptException;
 
 /**
  * This is a special JSON parser that is customized to consume the JSON output of buck.py. In
@@ -50,6 +60,16 @@ public class BuildFileToJsonParser {
   private static final String PATH_TO_BUCK_PY = System.getProperty("buck.path_to_buck_py",
       "src/com/facebook/buck/parser/buck.py");
 
+  private static final ExecutorService executor = Executors.newSingleThreadExecutor();
+  private static final ScriptEngine engine = new ScriptEngineManager().getEngineByName("python");
+  private static final String python = Joiner.on(System.getProperty("line.separator")).join(
+      "import sys",
+      "import os.path",
+      "sys.path.append(os.path.dirname(\"%s\"))",
+      "import buck",
+      "sys.argv=[\"%s\"]",
+      "buck.main()");
+
   private final JsonParser parser;
 
   public BuildFileToJsonParser(String json) throws JsonParseException, IOException {
@@ -143,6 +163,40 @@ public static List<Map<String, Object>> getAllRulesInProject(
     return getAllRules(rootPath.getAbsolutePath(), Optional.<String>absent(), includes, ansi);
   }
 
+  private static class BuildFileRunner implements Runnable {
+
+    private final ImmutableList<String> args;
+    private PipedWriter outputWriter;
+
+    public BuildFileRunner(ImmutableList<String> args, PipedReader outputReader) throws IOException {
+      this.args = args;
+      this.outputWriter = new PipedWriter();
+      outputWriter.connect(outputReader);
+    }
+
+    @Override
+    public void run() {
+      Closer closer = Closer.create();
+      try {
+        closer.register(outputWriter);
+
+        // TODO(user): call buck.py directly rather than emulating the old command line interface?
+        // TODO(user): escape args? (they are currently file names, which shouldn't contain quotes)
+        engine.getContext().setWriter(outputWriter);
+        engine.eval(String.format(python, PATH_TO_BUCK_PY, Joiner.on("\",\"").join(args)));
+
+      } catch (ScriptException e) {
+        throw Throwables.propagate(e);
+      } finally {
+        try {
+          closer.close();
+        } catch (IOException e) {
+          Throwables.propagate(e);
+        }
+      }
+    }
+  }
+
   /**
    * @param rootPath Absolute path to the root of the project. buck.py uses this to determine the
    *     base path of the targets in the build file that it is parsing.
@@ -154,11 +208,27 @@ public static List<Map<String, Object>> getAllRules(
       Optional<String> buildFile,
       Iterable<String> includes,
       Ansi ansi) throws IOException {
-    // A list of the build rules parsed from the build file.
+
+    // Run the build file in a background thread.
+    final ImmutableList<String> args = buildArgs(rootPath, buildFile, includes);
+    final PipedReader outputReader = new PipedReader();
+    BuildFileRunner runner = new BuildFileRunner(args, outputReader);
+    executor.execute(runner);
+
+    // Stream build rules from python.
+    BuildFileToJsonParser parser = new BuildFileToJsonParser(outputReader);
     List<Map<String, Object>> rules = Lists.newArrayList();
+    Map<String, Object> value;
+    while ((value = parser.next()) != null) {
+      rules.add(value);
+    }
+
+    return rules;
+  }
 
+  private static ImmutableList<String> buildArgs(String rootPath, Optional<String> buildFile, Iterable<String> includes) {
     // Create a process to run buck.py and read its stdout.
-    List<String> args = Lists.newArrayList("python", PATH_TO_BUCK_PY, "--project_root", rootPath);
+    List<String> args = Lists.newArrayList("buck.py", "--project_root", rootPath);
 
     // Add the --include flags.
     for (String include : includes) {
@@ -170,42 +240,6 @@ public static List<Map<String, Object>> getAllRules(
     if (buildFile.isPresent()) {
       args.add(buildFile.get());
     }
-
-    ProcessBuilder processBuilder = new ProcessBuilder(args);
-    Process process = processBuilder.start();
-    BuildFileToJsonParser parser = new BuildFileToJsonParser(process.getInputStream());
-
-    // If the build file parses cleanly, then nothing should be written to standard error. We make
-    // sure to consume stderr, just as we do in ShellCommand, to avoid potential deadlock.
-    InputStreamConsumer stdErr = new InputStreamConsumer(
-        process.getErrorStream(),
-        System.err,
-        true /* shouldPrintStdErr */,
-        ansi);
-    Thread stdErrConsumer = new Thread(stdErr);
-    stdErrConsumer.start();
-
-    // Read parsed JSON objects from the stream as they become available.
-    Map<String, Object> value = null;
-    while ((value = parser.next()) != null) {
-      rules.add(value);
-    }
-
-    // Make sure the process exits with zero. If not, throw an exception. The error was likely
-    // printed to stderr by the InputStreamConsumer.
-    try {
-      int exitCode = process.waitFor();
-      if (exitCode != 0) {
-        if (buildFile.isPresent()) {
-          throw new RuntimeException("Parsing " + buildFile.get() + " did not exit cleanly");
-        } else {
-          throw new RuntimeException("Error parsing build files");
-        }
-      }
-    } catch (InterruptedException e) {
-      throw Throwables.propagate(e);
-    }
-
-    return rules;
+    return ImmutableList.copyOf(args);
   }
 }

lib/jyson-1.0.2.jar

@@ -1,11 +1,35 @@
-import argparse
+from __future__ import with_statement
+
+import optparse
 import functools
-import json
 import re
 import os
 import os.path
 import posixpath
 
+try:
+  from com.xhaus.jyson import JysonCodec as json # jython embedded in buck
+except ImportError:
+  import json # python test case
+
+
+# TODO(user): upgrade to a jython including os.relpath
+def relpath(path, start=posixpath.curdir):
+  """
+  Return a relative filepath to path from the current directory or an optional start point.
+  """
+  if not path:
+    raise ValueError("no path specified")
+  start_list = posixpath.abspath(start).split(posixpath.sep)
+  path_list = posixpath.abspath(path).split(posixpath.sep)
+  # Work out how much of the filepath is shared by start and path.
+  common = len(posixpath.commonprefix([start_list, path_list]))
+  rel_list = [posixpath.pardir] * (len(start_list) - common) + path_list[common:]
+  if not rel_list:
+    return posixpath.curdir
+  return posixpath.join(*rel_list)
+
+
 # When BUILD files are executed, the functions in this file tagged with
 # @provide_for_build will be provided in the BUILD file's local symbol table.
 #
@@ -103,7 +127,7 @@ def check_path(path):
   for root, dirs, files in os.walk(search_base):
     if len(files) == 0:
       continue
-    relative_root = os.path.relpath(root, search_base)
+    relative_root = relpath(root, search_base)
     # The regexes generated by glob_pattern_to_regex_string don't
     # expect a leading './'
     if relative_root == '.':
@@ -588,19 +612,18 @@ def parse_git_ignore(gitignore_data):
 # parser. That means that printing out other information for debugging purposes will likely break
 # the JSON parsing, so be careful!
 def main():
-  parser = argparse.ArgumentParser()
-  parser.add_argument('--project_root')
-  parser.add_argument('--include', action='append')
-  parser.add_argument('build_files', nargs='*')
-  args = parser.parse_args()
+  parser = optparse.OptionParser()
+  parser.add_option('--project_root', action='store', type='string', dest='project_root')
+  parser.add_option('--include', action='append', dest='include')
+  (options, args) = parser.parse_args()
 
-  project_root = args.project_root
+  project_root = options.project_root
   len_suffix = -len('/' + BUILD_RULES_FILE_NAME)
 
   build_files = None
-  if args.build_files:
+  if args:
     # The user has specified which build files to parse.
-    build_files = args.build_files
+    build_files = args
   else:
     # Find all of the build files in the project root. Symlinks will not be traversed.
     # Search must be done top-down so that directory filtering works as desired.
@@ -633,14 +656,14 @@ def main():
     # or the files in includes through include_defs() don't pollute the namespace for
     # subsequent build files.
     build_env = {}
-    relative_path_to_build_file = os.path.relpath(build_file, project_root)
+    relative_path_to_build_file = relpath(build_file, project_root)
     build_env['BASE'] = relative_path_to_build_file[:len_suffix]
     build_env['BUILD_FILE_DIRECTORY'] = os.path.dirname(build_file)
     build_env['PROJECT_ROOT'] = project_root
     build_env['BUILD_FILE_SYMBOL_TABLE'] = make_build_file_symbol_table(build_env)
 
     # If there are any default includes, evaluate those first to populate the build_env.
-    includes = args.include or []
+    includes = options.include or []
     for include in includes:
       include_defs(include, build_env)
     execfile(os.path.join(project_root, build_file), build_env['BUILD_FILE_SYMBOL_TABLE'])