Add new check for non-phony targets that have no commands

There are two related, but separate checks here:

1. --warn_real_no_cmds_or_deps: That an action has no commands or
dependencies that could create the output file. In our ninja output,
these will act similarly to _kati_always_build_ and be considered always
dirty (unless something creates the file before ninja runs, but then
there shouldn't be an action created for it, and we should consider it a
source file).

Actually trying to build these now triggers a ninja error, so this Kati
check is here to catch the issue earlier with better diagnostics.

2. --warn_real_no_cmds: That a rule has no commands, but does have a
dependency that may create the output file. Usually this means that
instead of another action, the dependency should be using
.KATI_IMPLICIT_OUTPUTS to define that it created multiple files.

Under normal circumstances, this shouldn't cause many problems, but can
in some edge cases. I'm also looking to improve our build graph so that
it can function in sandbox-like environments (remote builds, etc). One
of the additions is to remove the output file before we run the commands
that should create it, but this is one of the cases where we don't know
when an output should be created.

Change-Id: Ie330ea49bf3617aa04fa17a3c5aa4dbb6b99b983

5 files changed, 161 insertions, 0 deletions

diff --git a/dep.cc b/dep.cc
index 65687cf..a4eb2d2 100644
--- a/dep.cc
+++ b/dep.cc
@@ -790,6 +790,53 @@ class DepBuilder {
       n->order_onlys.push_back({input, c});
     }
 
+    // Block on werror_writable/werror_phony_looks_real, because otherwise we
+    // can't rely on is_phony being valid for this check.
+    if (!n->is_phony && n->cmds.empty() && g_flags.werror_writable &&
+        g_flags.werror_phony_looks_real) {
+      if (n->deps.empty() && n->order_onlys.empty()) {
+        if (g_flags.werror_real_no_cmds_or_deps) {
+          ERROR_LOC(
+              n->loc,
+              "*** target \"%s\" has no commands or deps that could create it",
+              output.c_str());
+        } else if (g_flags.warn_real_no_cmds_or_deps) {
+          WARN_LOC(n->loc,
+                   "warning: target \"%s\" has no commands or deps that could "
+                   "create it",
+                   output.c_str());
+        }
+      } else {
+        if (n->actual_inputs.size() == 1) {
+          if (g_flags.werror_real_no_cmds) {
+            ERROR_LOC(n->loc,
+                      "*** target \"%s\" has no commands. Should \"%s\" be "
+                      "using .KATI_IMPLICIT_OUTPUTS?",
+                      output.c_str(), n->actual_inputs[0].c_str());
+          } else if (g_flags.warn_real_no_cmds) {
+            WARN_LOC(n->loc,
+                     "warning: target \"%s\" has no commands. Should \"%s\" be "
+                     "using .KATI_IMPLICIT_OUTPUTS?",
+                     output.c_str(), n->actual_inputs[0].c_str());
+          }
+        } else {
+          if (g_flags.werror_real_no_cmds) {
+            ERROR_LOC(
+                n->loc,
+                "*** target \"%s\" has no commands that could create output "
+                "file. Is a dependency missing .KATI_IMPLICIT_OUTPUTS?",
+                output.c_str());
+          } else if (g_flags.warn_real_no_cmds) {
+            WARN_LOC(
+                n->loc,
+                "warning: target \"%s\" has no commands that could create "
+                "output file. Is a dependency missing .KATI_IMPLICIT_OUTPUTS?",
+                output.c_str());
+          }
+        }
+      }
+    }
+
     n->has_rule = true;
     n->is_default_target = first_rule_ == output;
     if (cur_rule_vars_->empty()) {
diff --git a/flags.cc b/flags.cc
index 148d59a..bcc1b4f 100644
--- a/flags.cc
+++ b/flags.cc
@@ -134,6 +134,16 @@ void Flags::Parse(int argc, char** argv) {
       werror_phony_looks_real = true;
     } else if (!strcmp(arg, "--werror_writable")) {
       werror_writable = true;
+    } else if (!strcmp(arg, "--warn_real_no_cmds_or_deps")) {
+      warn_real_no_cmds_or_deps = true;
+    } else if (!strcmp(arg, "--werror_real_no_cmds_or_deps")) {
+      warn_real_no_cmds_or_deps = true;
+      werror_real_no_cmds_or_deps = true;
+    } else if (!strcmp(arg, "--warn_real_no_cmds")) {
+      warn_real_no_cmds = true;
+    } else if (!strcmp(arg, "--werror_real_no_cmds")) {
+      warn_real_no_cmds = true;
+      werror_real_no_cmds = true;
     } else if (ParseCommandLineOptionWithArg("-j", argv, &i, &num_jobs_str)) {
       num_jobs = strtol(num_jobs_str, NULL, 10);
       if (num_jobs <= 0) {
diff --git a/flags.h b/flags.h
index ee16c4f..279e7fe 100644
--- a/flags.h
+++ b/flags.h
@@ -56,6 +56,10 @@ struct Flags {
   bool warn_phony_looks_real;
   bool werror_phony_looks_real;
   bool werror_writable;
+  bool warn_real_no_cmds_or_deps;
+  bool werror_real_no_cmds_or_deps;
+  bool warn_real_no_cmds;
+  bool werror_real_no_cmds;
   const char* default_pool;
   const char* goma_dir;
   const char* ignore_dirty_pattern;
diff --git a/testcase/real_no_cmds.sh b/testcase/real_no_cmds.sh
new file mode 100644
index 0000000..88066d7
--- /dev/null
+++ b/testcase/real_no_cmds.sh
@@ -0,0 +1,53 @@
+#!/bin/bash
+#
+# Copyright 2020 Google Inc. 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.
+
+set -u
+
+mk="$@"
+
+cat <<EOF > Makefile
+test: out/bad.baz out/good.baz
+	@echo "END"
+.PHONY: test
+
+out/bad.bar:
+	@echo bar >out/bad.bar
+	@echo baz >out/bad.baz
+
+out/bad.baz: out/bad.bar
+
+out/good.bar: .KATI_IMPLICIT_OUTPUTS := out/good.baz
+out/good.bar:
+	@echo bar >out/good.bar
+	@echo baz >out/good.baz
+EOF
+
+mkdir -p out
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't support these warnings, so write the expected output.
+  echo 'Makefile:9: warning: target "out/bad.baz" has no commands. Should "out/bad.bar" be using .KATI_IMPLICIT_OUTPUTS?'
+  echo 'END'
+else
+  ${mk} --werror_phony_looks_real --writable=out/ --werror_writable --warn_real_no_cmds 2>&1
+fi
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't support these warnings, so write the expected output.
+  echo 'Makefile:9: *** target "out/bad.baz" has no commands. Should "out/bad.bar" be using .KATI_IMPLICIT_OUTPUTS?'
+else
+  ${mk} --werror_phony_looks_real --writable=out/ --werror_writable --werror_real_no_cmds 2>&1
+fi
diff --git a/testcase/real_no_cmds_or_deps.sh b/testcase/real_no_cmds_or_deps.sh
new file mode 100644
index 0000000..b591803
--- /dev/null
+++ b/testcase/real_no_cmds_or_deps.sh
@@ -0,0 +1,47 @@
+#!/bin/bash
+#
+# Copyright 2020 Google Inc. 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.
+
+set -u
+
+mk="$@"
+
+cat <<EOF > Makefile
+test: out/bad out/good
+	@echo "END"
+.PHONY: test
+
+out/bad:
+
+out/good:
+	@echo bar >out/good
+EOF
+
+mkdir -p out
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't support these warnings, so write the expected output.
+  echo 'Makefile:5: warning: target "out/bad" has no commands or deps that could create it'
+  echo 'END'
+else
+  ${mk} --werror_phony_looks_real --writable=out/ --werror_writable --warn_real_no_cmds_or_deps 2>&1
+fi
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't support these warnings, so write the expected output.
+  echo 'Makefile:5: *** target "out/bad" has no commands or deps that could create it'
+else
+  ${mk} --werror_phony_looks_real --writable=out/ --werror_writable --werror_real_no_cmds_or_deps 2>&1
+fi