Split out Java proto compilation logic to a helper script (take 2).

This is just for clarity and should be a no-op.  Maintaining long shell
command lines embedded in genrules is not pretty.

This change is a combination of commit 8ec3866d9e5d28f6c09546269ec82731c45c28ad
and commit ffc891c4efbf498fdb8eaf4d2c922e7e7a58f99d with additional fixes.
In particular I have removed unnecessary properties from all rule
attributes, have made the rule respect any runfiles, and have reinstanted
the explicit dependency on the JDK.  The latter is what fixes the breakage
we observed in #938, though it seems clear that we are missing a
dependency somewhere.

I have verified that this now works by running this change on a clean
Ubuntu 15.10 VM.

--
MOS_MIGRATED_REVID=115081830

3 files changed, 90 insertions, 18 deletions

diff --git a/tools/build_rules/BUILD b/tools/build_rules/BUILD
index 1184bb1895..a00791e4eb 100644
--- a/tools/build_rules/BUILD
+++ b/tools/build_rules/BUILD
@@ -1,5 +1,11 @@
 licenses(["notice"])  # Apache 2.0
 
+sh_binary(
+    name = "gensrcjar",
+    srcs = ["gensrcjar.sh"],
+    visibility = ["//visibility:public"],
+)
+
 filegroup(
     name = "srcs",
     srcs = glob(["**"]),
diff --git a/tools/build_rules/genproto.bzl b/tools/build_rules/genproto.bzl
index 6363f407d9..da0755527c 100644
--- a/tools/build_rules/genproto.bzl
+++ b/tools/build_rules/genproto.bzl
@@ -19,23 +19,22 @@ proto_filetype = FileType([".proto"])
 
 def gensrcjar_impl(ctx):
   out = ctx.outputs.srcjar
-  proto_output = out.path + ".proto_output"
-  proto_compiler = ctx.file._proto_compiler
-  sub_commands = [
-    "rm -rf " + proto_output,
-    "mkdir " + proto_output,
-    ' '.join([proto_compiler.path, "--java_out=" + proto_output,
-              ctx.file.src.path]),
-    "touch -t 198001010000 $(find " + proto_output + ")",
-    ctx.file._jar.path + " cMf " + out.path + " -C " + proto_output + " .",
-  ]
 
   ctx.action(
-    command=" && ".join(sub_commands),
-    inputs=[ctx.file.src, proto_compiler, ctx.file._jar] + ctx.files._jdk,
+    command=' '.join([
+        "JAR='%s'" % ctx.executable._jar.path,
+        "OUTPUT='%s'" % out.path,
+        "PROTO_COMPILER='%s'" % ctx.executable._proto_compiler.path,
+        "SOURCE='%s'" % ctx.file.src.path,
+        ctx.executable._gensrcjar.path,
+    ]),
+    inputs=([ctx.file.src] + ctx.files._gensrcjar + ctx.files._jar +
+            ctx.files._jdk + ctx.files._proto_compiler),
     outputs=[out],
     mnemonic="GenProtoSrcJar",
-    use_default_shell_env = True)
+    use_default_shell_env=True)
+
+  return struct(runfiles=ctx.runfiles(collect_default=True))
 
 gensrcjar = rule(
     gensrcjar_impl,
@@ -44,23 +43,26 @@ gensrcjar = rule(
             allow_files = proto_filetype,
             single_file = True,
         ),
+        "_gensrcjar": attr.label(
+            default = Label("@bazel_tools//tools/build_rules:gensrcjar"),
+            executable = True,
+        ),
         # TODO(bazel-team): this should be a hidden attribute with a default
         # value, but Skylark needs to support select first.
         "_proto_compiler": attr.label(
             default = Label("@bazel_tools//third_party:protoc"),
-            allow_files = True,
             executable = True,
-            single_file = True,
         ),
         "_jar": attr.label(
             default = Label("@bazel_tools//tools/jdk:jar"),
-            allow_files = True,
             executable = True,
-            single_file = True,
         ),
+        # The jdk dependency is required to ensure dependent libraries are found
+        # when we invoke jar (see issue #938).
+        # TODO(bazel-team): Figure out why we need to pull this in explicitly;
+        # the jar dependency above should just do the right thing on its own.
         "_jdk": attr.label(
             default = Label("@bazel_tools//tools/jdk:jdk"),
-            allow_files = True,
         ),
     },
     outputs = {"srcjar": "lib%{name}.srcjar"},
diff --git a/tools/build_rules/gensrcjar.sh b/tools/build_rules/gensrcjar.sh
new file mode 100755
index 0000000000..365aaa86e3
--- /dev/null
+++ b/tools/build_rules/gensrcjar.sh
@@ -0,0 +1,64 @@
+#! /bin/sh
+# Copyright 2016 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# List of environment variables that must be defined as input.
+#
+# We use environment variables instead of positional arguments or flags for
+# clarity in the caller .bzl file and for simplicity of processing.  There is no
+# need to implement a full-blown argument parser for this simple script.
+INPUT_VARS="JAR OUTPUT PROTO_COMPILER SOURCE"
+
+# Basename of the script for error reporting purposes.
+PROGRAM_NAME="${0##*/}"
+
+# A timestamp to mark all generated files with to get deterministic JAR outputs.
+TIMESTAMP=198001010000
+
+# Prints an error and exits.
+#
+# Args:
+#   ...: list(str).  Parts of the message to print; all of them are joined
+#       with a single space in between.
+err() {
+  echo "${PROGRAM_NAME}: ${*}" 1>&2
+  exit 1
+}
+
+# Entry point.
+main() {
+  [ ${#} -eq 0 ] || err "No arguments allowed; set the following environment" \
+      "variables for configuration instead: ${INPUT_VARS}"
+  for var in ${INPUT_VARS}; do
+    local value
+    eval "value=\"\$${var}\""
+    [ -n "${value}" ] || err "Input environment variable ${var} is not set"
+  done
+
+  rm -f "${OUTPUT}"
+
+  local proto_output="${OUTPUT}.proto_output"
+  rm -rf "${proto_output}"
+  mkdir -p "${proto_output}"
+
+  "${PROTO_COMPILER}" --java_out="${proto_output}" "${SOURCE}" \
+      || err "proto_compiler failed"
+  find "${proto_output}" -exec touch -t "${TIMESTAMP}" '{}' \; \
+      || err "Failed to reset timestamps"
+  "${JAR}" cMf "${OUTPUT}.tmp" -C "${proto_output}" . \
+      || err "jar failed"
+  mv "${OUTPUT}.tmp" "${OUTPUT}"
+}
+
+main "${@}"