From 738cac100f6e9b2dbb783a5d3bcd3054d12593e0 Mon Sep 17 00:00:00 2001
From: John Millikin <john@john-millikin.com>
Date: Wed, 13 Nov 2019 04:18:47 -0800
Subject: [PATCH] Add tool executables (from FilesToRunProvider) to action
 inputs.

Fixes https://github.com/bazelbuild/bazel/issues/7390

Summary: rules are allowed to set `DefaultInfo(files = depset(), executable = ...)`. If the resulting targets are used as tool inputs to another target, the executable will not be added to the downstream targets' action inputs. This manifests as either an `execvp()` error (from `ctx.actions.run()`) or a Bash error in genrules.

This PR fixes that so the use case of non-default executables works as expected. This is useful for building scripts with complex runfiles, where `bazel run //:something` and `bazel build //:something && bazel-bin/something` cannot be supported by the same output file.

cc @ccate @jmmv

Closes #10110.

PiperOrigin-RevId: 280170524
---
 .../analysis/RuleConfiguredTargetBuilder.java |  3 +
 src/test/shell/bazel/bazel_rules_test.sh      | 69 +++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 50b24761d67211..84cdd2ed557412 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -362,6 +362,9 @@ private NestedSet<Artifact> buildFilesToRun(
       NestedSet<Artifact> runfilesMiddlemen, NestedSet<Artifact> filesToBuild) {
     filesToRunBuilder.addTransitive(filesToBuild);
     filesToRunBuilder.addTransitive(runfilesMiddlemen);
+    if (executable != null && ruleContext.getRule().getRuleClassObject().isSkylark()) {
+      filesToRunBuilder.add(executable);
+    }
     return filesToRunBuilder.build();
   }
 
diff --git a/src/test/shell/bazel/bazel_rules_test.sh b/src/test/shell/bazel/bazel_rules_test.sh
index ca94833b237cb0..ef70918a41992d 100755
--- a/src/test/shell/bazel/bazel_rules_test.sh
+++ b/src/test/shell/bazel/bazel_rules_test.sh
@@ -524,4 +524,73 @@ EOF
   expect_log "Public or private visibility labels (e.g. //visibility:public or //visibility:private) cannot be used in combination with other labels"
 }
 
+function test_executable_without_default_files() {
+  mkdir pkg
+  cat >pkg/BUILD <<'EOF'
+load(":rules.bzl", "bin_rule", "out_rule")
+bin_rule(name = "hello_bin")
+out_rule(name = "hello_out")
+
+genrule(
+    name = "hello_gen",
+    tools = [":hello_bin"],
+    outs = ["hello_gen.txt"],
+    cmd = "$(location :hello_bin) $@",
+)
+EOF
+
+  # On Windows this file needs to be acceptable by CreateProcessW(), rather
+  # than a Bourne script.
+  if "$is_windows"; then
+    cat >pkg/rules.bzl <<'EOF'
+_SCRIPT_EXT = ".bat"
+_SCRIPT_CONTENT = "@ECHO OFF\necho hello world > %1"
+EOF
+  else
+    cat >pkg/rules.bzl <<'EOF'
+_SCRIPT_EXT = ".sh"
+_SCRIPT_CONTENT = "#!/bin/sh\necho 'hello world' > $@"
+EOF
+  fi
+
+  cat >>pkg/rules.bzl <<'EOF'
+def _bin_rule(ctx):
+    out_sh = ctx.actions.declare_file(ctx.attr.name + _SCRIPT_EXT)
+    ctx.actions.write(
+        output = out_sh,
+        content = _SCRIPT_CONTENT,
+        is_executable = True,
+    )
+    return DefaultInfo(
+        files = depset(direct = []),
+        executable = out_sh,
+    )
+
+def _out_rule(ctx):
+    out = ctx.actions.declare_file(ctx.attr.name + ".txt")
+    ctx.actions.run(
+        executable = ctx.executable._hello_bin,
+        outputs = [out],
+        arguments = [out.path],
+        mnemonic = "HelloOut",
+    )
+    return DefaultInfo(
+        files = depset(direct = [out]),
+    )
+
+bin_rule = rule(_bin_rule, executable = True)
+out_rule = rule(_out_rule, attrs = {
+    "_hello_bin": attr.label(
+        default = ":hello_bin",
+        executable = True,
+        cfg = "host",
+    ),
+})
+EOF
+
+  bazel build //pkg:hello_out //pkg:hello_gen >$TEST_log 2>&1 || fail "Should build"
+  assert_contains "hello world" bazel-bin/pkg/hello_out.txt
+  assert_contains "hello world" bazel-bin/pkg/hello_gen.txt
+}
+
 run_suite "rules test"