Add --werror_find_emulator, --werror_overriding_commands

For Android builds, we'd like to start removing some of the default
warnings and turn them into errors so that they can't come back.

For find emulator, we could attempt to check for errors, or silence
every find command in the tree, but that doesn't particularly scale,
especially when new code gets added with warnings. We've gone through
and fixed many of these, but they keep coming back, so add
--werror_find_emulator so that when we fix them all we can prevent them
from coming back.

Overriding commands is similar -- we really don't want multiple rules
defining a single output file. In ninja we've turned on -w dupbuild=err,
but if the paths happen to be identical the makefile overriding logic
kicks in first and presents a warning instead of an error. So add
--werror_overriding_commands in order to turn the make warning into an
error.

6 files changed, 124 insertions, 16 deletions

diff --git a/dep.cc b/dep.cc
index 75dff76..e9aa32b 100644
--- a/dep.cc
+++ b/dep.cc
@@ -162,12 +162,18 @@ struct RuleMerger {
 
     if (primary_rule && !r->cmds.empty() &&
         !IsSuffixRule(output) && !r->is_double_colon) {
-      WARN_LOC(r->cmd_loc(),
-               "warning: overriding commands for target `%s'",
-               output.c_str());
-      WARN_LOC(primary_rule->cmd_loc(),
-               "warning: ignoring old commands for target `%s'",
-               output.c_str());
+      if (g_flags.werror_overriding_commands) {
+        ERROR_LOC(r->cmd_loc(),
+                  "*** overriding commands for target `%s', previously defined at %s:%d",
+                  output.c_str(), LOCF(primary_rule->cmd_loc()));
+      } else {
+        WARN_LOC(r->cmd_loc(),
+                 "warning: overriding commands for target `%s'",
+                 output.c_str());
+        WARN_LOC(primary_rule->cmd_loc(),
+                 "warning: ignoring old commands for target `%s'",
+                 output.c_str());
+      }
       primary_rule = r;
     }
     if (!primary_rule && !r->cmds.empty()) {
diff --git a/find.cc b/find.cc
index 6712923..2af242b 100644
--- a/find.cc
+++ b/find.cc
@@ -35,6 +35,14 @@
 #include "strutil.h"
 #include "timeutil.h"
 
+#define FIND_WARN_LOC(...) do {         \
+    if (g_flags.werror_find_emulator) { \
+      ERROR_LOC(__VA_ARGS__);           \
+    } else {                            \
+      WARN_LOC(__VA_ARGS__);            \
+    }                                   \
+  } while (0)
+
 class FindCond {
  public:
   virtual ~FindCond() = default;
@@ -261,9 +269,9 @@ class DirentDirNode : public DirentNode {
                        vector<string>& out) const override {
     ScopedReadDirTracker srdt(this, *path, cur_read_dirs);
     if (!srdt.ok()) {
-      WARN_LOC(loc, "FindEmulator: find: File system loop detected; `%s' "
-               "is part of the same file system loop as `%s'.",
-               path->c_str(), srdt.conflicted().c_str());
+      FIND_WARN_LOC(loc, "FindEmulator: find: File system loop detected; `%s' "
+                    "is part of the same file system loop as `%s'.",
+                    path->c_str(), srdt.conflicted().c_str());
       return true;
     }
 
@@ -368,8 +376,8 @@ class DirentSymlinkNode : public DirentNode {
     if (fc.follows_symlinks && errno_ != ENOENT) {
       if (errno_) {
         if (fc.type != FindCommandType::FINDLEAVES) {
-          WARN_LOC(loc, "FindEmulator: find: `%s': %s",
-                   path->c_str(), strerror(errno_));
+          FIND_WARN_LOC(loc, "FindEmulator: find: `%s': %s",
+                        path->c_str(), strerror(errno_));
         }
         return true;
       }
@@ -687,7 +695,11 @@ class FindCommandParser {
         StringPiece dir= tok.substr(strlen("--dir="));
         fc_->finddirs.push_back(dir.as_string());
       } else if (HasPrefix(tok, "--")) {
-        WARN("Unknown flag in findleaves.py: %.*s", SPF(tok));
+        if (g_flags.werror_find_emulator) {
+          ERROR("Unknown flag in findleaves.py: %.*s", SPF(tok));
+        } else {
+          WARN("Unknown flag in findleaves.py: %.*s", SPF(tok));
+        }
         return false;
       } else {
         findfiles.push_back(tok.as_string());
@@ -826,8 +838,8 @@ class FindEmulatorImpl : public FindEmulator {
         if (should_fallback)
           return false;
         if (!fc.redirect_to_devnull) {
-          WARN_LOC(loc, "FindEmulator: cd: %.*s: No such file or directory",
-                   SPF(fc.chdir));
+          FIND_WARN_LOC(loc, "FindEmulator: cd: %.*s: No such file or directory",
+                        SPF(fc.chdir));
         }
         return true;
       }
@@ -850,8 +862,8 @@ class FindEmulatorImpl : public FindEmulator {
           return false;
         }
         if (!fc.redirect_to_devnull) {
-          WARN_LOC(loc, "FindEmulator: find: `%s': No such file or directory",
-                   ConcatDir(fc.chdir, finddir).c_str());
+          FIND_WARN_LOC(loc, "FindEmulator: find: `%s': No such file or directory",
+                        ConcatDir(fc.chdir, finddir).c_str());
         }
         continue;
       }
diff --git a/flags.cc b/flags.cc
index 5c14af7..5f34835 100644
--- a/flags.cc
+++ b/flags.cc
@@ -99,6 +99,10 @@ void Flags::Parse(int argc, char** argv) {
       detect_depfiles = true;
     } else if (!strcmp(arg, "--color_warnings")) {
       color_warnings = true;
+    } else if (!strcmp(arg, "--werror_find_emulator")) {
+      werror_find_emulator = true;
+    } else if (!strcmp(arg, "--werror_overriding_commands")) {
+      werror_overriding_commands = true;
     } else if (ParseCommandLineOptionWithArg(
         "-j", argv, &i, &num_jobs_str)) {
       num_jobs = strtol(num_jobs_str, NULL, 10);
diff --git a/flags.h b/flags.h
index cd4090a..53e777b 100644
--- a/flags.h
+++ b/flags.h
@@ -40,6 +40,8 @@ struct Flags {
   bool regen_ignoring_kati_binary;
   bool use_find_emulator;
   bool color_warnings;
+  bool werror_find_emulator;
+  bool werror_overriding_commands;
   const char* goma_dir;
   const char* ignore_dirty_pattern;
   const char* no_ignore_dirty_pattern;
diff --git a/testcase/werror_find_emulator.sh b/testcase/werror_find_emulator.sh
new file mode 100644
index 0000000..c18f866
--- /dev/null
+++ b/testcase/werror_find_emulator.sh
@@ -0,0 +1,40 @@
+#!/bin/bash
+#
+# Copyright 2017 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
+FOO := \$(shell find does/not/exist -name '*.txt')
+all:
+EOF
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't use find emulator, or support --werror_find_emulator, so write
+  # expected output.
+  echo 'find: "does/not/exist": No such file or directory'
+  echo 'Nothing to be done for "all".'
+  echo 'Clean exit'
+else
+  ${mk} --use_find_emulator 2>&1 && echo "Clean exit"
+fi
+
+if echo "${mk}" | grep -qv "kati"; then
+  echo 'find: "does/not/exist": No such file or directory'
+else
+  ${mk} --use_find_emulator --werror_find_emulator 2>&1 && echo "Clean exit"
+fi
diff --git a/testcase/werror_overriding_commands.sh b/testcase/werror_overriding_commands.sh
new file mode 100644
index 0000000..e89553d
--- /dev/null
+++ b/testcase/werror_overriding_commands.sh
@@ -0,0 +1,44 @@
+#!/bin/bash
+#
+# Copyright 2017 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: foo
+foo:
+	@echo "FAIL"
+foo:
+	@echo "PASS"
+EOF
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't use find emulator, or support --werror_find_emulator, so write
+  # expected output.
+  echo 'Makefile:5: warning: overriding commands for target "foo"'
+  echo 'Makefile:3: warning: ignoring old commands for target "foo"'
+  echo 'PASS'
+  echo 'Clean exit'
+else
+  ${mk} 2>&1 && echo "Clean exit"
+fi
+
+if echo "${mk}" | grep -qv "kati"; then
+  echo 'Makefile:5: *** overriding commands for target "foo", previously defined at Makefile:3'
+else
+  ${mk} --werror_overriding_commands 2>&1 && echo "Clean exit"
+fi