Create js_library and js_binary templates for closure compiling.

third_party/closure_compiler/js_binary.py

Index: third_party/closure_compiler/js_binary.py
diff --git a/third_party/closure_compiler/js_binary.py b/third_party/closure_compiler/js_binary.py
new file mode 100644
index 0000000000000000000000000000000000000000..0027b2da9479b8a699e8a68f754d6b96033499c5
--- /dev/null
+++ b/third_party/closure_compiler/js_binary.py
@@ -0,0 +1,115 @@
+# Copyright 2017 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.
+"""Used by a js_binary action to compile javascript files.
+
+This script takes in a list of sources and dependencies and compiles them all
+together into a single compiled .js file.  The dependencies are ordered in a
+post-order, left-to-right traversal order.  If multiple instances of the same
+source file are read, only the first is kept. The script can also take in
+optional --defs argument which will add custom flags to the comiler.  Any extern
+files can also be passed in using the --extern flag.
+"""
+
+from argparse import ArgumentParser
+import os
+import subprocess
+import sys
+
+EXIT_SUCCESS = 0
+EXIT_FAILURE = 1
+
+def IsExecutable(path):
+  return os.path.isfile(path) and os.access(path, os.X_OK)
+
+def FindCommand(command):

[mbjorge, 2017/04/07 19:23:18]

add one line doc strings for each of the methods

[damargulis, 2017/04/07 23:02:19]

Done.

+  fpath, _ = os.path.split(command)

[mbjorge, 2017/04/07 19:23:18]

rename: fpath -> filepath or command_name 

[damargulis, 2017/04/07 23:02:19]

Done.

+  if fpath:
+    if IsExecutable(command):
+      return command
+
+  if sys.platform == 'win32':
+    # On Windows, if the command does not have an extension, cmd.exe will
+    # try all extensions from PATHEXT when resolving the full path.
+    command, ext = os.path.splitext(command)
+    if not ext:
+      exts = os.environ['PATHEXT'].split(os.path.pathsep)
+    else:
+      exts = [ext]
+  else:
+    exts = ['']
+
+  for path in os.environ['PATH'].split(os.path.pathsep):
+    for ext in exts:
+      path = os.path.join(path, command) + ext
+      if IsExecutable(path):
+        return path
+
+  return None
+
+
+def RunCompiler(args):
+  java_path = FindCommand('java')
+  if not java_path:
+    sys.stderr.write('java: command not found\n')
+    sys.exit(EXIT_FAILURE)
+  return subprocess.check_call([java_path, '-jar'] + args)
+
+def postOrder(deps, sources):

[mbjorge, 2017/04/07 19:23:18]

nit: PostOrder

[damargulis, 2017/04/07 23:02:19]

Done.

+  for dep in deps:
+    with file(dep, 'r') as dep_list:

[mbjorge, 2017/04/07 19:23:18]

with open(dep, 'r') as dep_list

When opening a file, it’s preferable to use open() instead of invoking [file()]
directly
https://docs.python.org/2/library/functions.html#file

[damargulis, 2017/04/07 23:02:19]

Done.

+      lines = [line.strip() for line in dep_list.readlines()]
+    split = lines.index('deps:')

[mbjorge, 2017/04/07 19:23:18]

Maybe pull this out into a helper method?

new_sources, new_deps = ParseDepList(dep)

def ParseDepList(dep):
  with open(dep, 'r') as dep_list:
    lines = [line.strip() for line in dep_list.readlines()]
  split = lines.index('deps:')
  return lines[1:split], lines[split+1:]


Or maybe just give some nice names to the different parts:
source_lines, dep_lines = lines[1:split], lines[split+1:]

[damargulis, 2017/04/07 23:02:18]

Done.

+    sources = postOrder(lines[split+1:], sources)
+    sources = sources + [source for source in lines[1:split]
+                         if source not in sources]
+  return sources
+
+def main():
+  parser = ArgumentParser()
+  parser.add_argument('-c', '--compiler', required=True,
+                      help='Path to compiler')
+  parser.add_argument('-s', '--sources', nargs='*', default=[],
+                      help='List of js source files')
+  parser.add_argument('-o', '--output', required=True,
+                      help='Compile to output')
+  parser.add_argument('-d', '--deps', nargs='*', default = [],
+                      help='List of js_libarary dependencies')
+  parser.add_argument('-b', '--bootstrap',
+                      help='A file to include before all others')
+  parser.add_argument('-cf', '--config', nargs='*', default = [],
+                      help='A list of files to include after bootstrap and' \
+                      'before all others')
+  parser.add_argument('-df', '--defs', nargs='*', default = [],
+                      help='A list of custom flags to pass to the compiler. ' \
+                      'Do not include leading dashes')
+  parser.add_argument('-e', '--externs', nargs='*', default = [],
+                      help='A list of extern files to pass to the compiler')
+
+  args = parser.parse_args()
+  sources = postOrder(args.deps, args.sources)
+
+  defs = ['--' + flag for flag in args.defs]
+
+  compiler_args = [
+      args.compiler,
+      '--compilation_level',
+      'SIMPLE_OPTIMIZATIONS',

[mbjorge, 2017/04/07 19:23:18]

does it make sense for this to be in the GN template instead?

This is defined in //third_party/clouser_compiler/closure_args.gni ::
checking_clouser_args, maybe you want to just include all of those by default
also?

Possibly with a boolean option (that defaults true) that can be set to false if
someone doesn't want to check their target (the ios version has checked and
unchecked targets, so presumably there is a usecase for not checking?
https://cs.chromium.org/chromium/src/ios/web/js_compile.gni?q=js_comp&l=100)

[damargulis, 2017/04/07 23:02:19]

I think it makes sense to add this in from the GN template, but Im not sure if
we should create an option to leave it unchecked.  In ios, the unchecked target
is only used for legacy code that doesn't pass the checks, and there is
currently a TODO to fix all the code and remove the unchecked target.  I think
it makes more sense to require checking when using this template to ensure that
code always can pass the checks.  Ideally, whenever anyone uses this template
they will not commit the code until it passes all these checks anyway.

Also, checking_closure_args includes polymer_pass, so I will need to include the
polymer externs by default, but this already exists in
closure_compiler/externs/polymer-1.0.js, so this is simple to do.

+  ] + defs
+
+  if(args.externs):
+    compiler_args += ['--externs'] + args.externs
+
+  compiler_args += [
+      '--js_output_file',
+      args.output,
+      '--js',
+  ]
+  if(args.bootstrap):
+    compiler_args += [args.bootstrap]
+  compiler_args += args.config
+  compiler_args += sources
+  RunCompiler(compiler_args)
+
+if __name__ == '__main__':
+  sys.exit(main())