Merge remote-tracking branch 'aosp/upstream'

This fixes three bugs and disables the depfile detection. The
depfile detection is not necessary anymore as we use
.KATI_DEPFILE explicitly.

b3d01c4 Merge remote-tracking branch 'aosp/upstream' into merge20
044a51d Fix https://github.com/google/kati/issues/47
c3a4834 Use subshell even when a command starts with '('
f3ad9e0 Detect depfiles only when --detect_depfiles is specified
e6a4d17 Fix https://github.com/google/kati/issues/46

Change-Id: I82252a6d2fd77e095866e7b0820e04268555c78c

9 files changed, 35 insertions, 14 deletions

diff --git a/flags.cc b/flags.cc
index 7623499..7a995a5 100644
--- a/flags.cc
+++ b/flags.cc
@@ -82,6 +82,8 @@ void Flags::Parse(int argc, char** argv) {
       dump_kati_stamp = true;
     } else if (!strcmp(arg, "--detect_android_echo")) {
       detect_android_echo = true;
+    } else if (!strcmp(arg, "--detect_depfiles")) {
+      detect_depfiles = 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 905d5ee..910acbf 100644
--- a/flags.h
+++ b/flags.h
@@ -25,6 +25,7 @@ using namespace std;
 
 struct Flags {
   bool detect_android_echo;
+  bool detect_depfiles;
   bool dump_kati_stamp;
   bool enable_kati_warnings;
   bool enable_stat_logs;
diff --git a/m2n b/m2n
index b6798b8..3fd6661 100755
--- a/m2n
+++ b/m2n
@@ -114,7 +114,7 @@ if [ x"${ninja_suffix}" != x"" ]; then
   ninja_suffix_flag=--ninja_suffix=${ninja_suffix}
 fi
 
-${kati} --ninja ${ninja_suffix_flag} --ignore_optional_include=out/%.P --ignore_dirty=out/% --use_find_emulator --detect_android_echo --gen_all_targets ${goma_flag} ${extra_flags} ${targets}
+${kati} --ninja ${ninja_suffix_flag} --ignore_optional_include=out/%.P --ignore_dirty=out/% --use_find_emulator --detect_android_echo --detect_depfiles --gen_all_targets ${goma_flag} ${extra_flags} ${targets}
 
 echo
 echo ninja${ninja_suffix}.sh and build${ninja_suffix}.ninja were generated, please run ./ninja${ninja_suffix}.sh
diff --git a/ninja.cc b/ninja.cc
index 4a5d014..c95ee64 100644
--- a/ninja.cc
+++ b/ninja.cc
@@ -407,28 +407,19 @@ class NinjaGenerator {
     static bool was_gomacc_found = false;
     bool got_descritpion = false;
     bool use_gomacc = false;
-    bool should_ignore_error = false;
     auto command_count = commands.size();
     for (const Command* c : commands) {
       size_t cmd_begin = cmd_buf->size();
 
       if (!cmd_buf->empty()) {
-        if (should_ignore_error) {
-          *cmd_buf += " ; ";
-        } else {
-          *cmd_buf += " && ";
-        }
+        *cmd_buf += " && ";
       }
-      should_ignore_error = c->ignore_error;
 
       const char* in = c->cmd.c_str();
       while (isspace(*in))
         in++;
 
-      bool needs_subshell = command_count > 1;
-      if (*in == '(') {
-        needs_subshell = false;
-      }
+      bool needs_subshell = (command_count > 1 || c->ignore_error);
 
       if (needs_subshell)
         *cmd_buf += '(';
@@ -458,7 +449,7 @@ class NinjaGenerator {
         was_gomacc_found = true;
       }
 
-      if (c == commands.back() && c->ignore_error) {
+      if (c->ignore_error) {
         *cmd_buf += " ; true";
       }
 
@@ -474,6 +465,8 @@ class NinjaGenerator {
       node->depfile_var->Eval(ev_, depfile);
       return true;
     }
+    if (!g_flags.detect_depfiles)
+      return false;
 
     *cmd_buf += ' ';
     bool result = GetDepfileFromCommand(cmd_buf, depfile);
diff --git a/runtest.rb b/runtest.rb
index bf42bfe..ab1e26b 100755
--- a/runtest.rb
+++ b/runtest.rb
@@ -108,6 +108,10 @@ def normalize_ninja_log(log, mk)
     # This test expects ninja fails. Strip ninja specific error logs.
     log.gsub!(/^FAILED: .*\n/, '')
     log.gsub!(/^ninja: .*\n/, '')
+  elsif mk =~ /\/fail_/
+    # Recipes in these tests fail.
+    log.gsub!(/^FAILED: .*/, '*** [test] Error 1')
+    log.gsub!(/^ninja: .*\n/, '')
   end
   log
 end
diff --git a/testcase/append_self_reference.mk b/testcase/append_self_reference.mk
new file mode 100644
index 0000000..8f3912f
--- /dev/null
+++ b/testcase/append_self_reference.mk
@@ -0,0 +1,8 @@
+x := one
+x += two $(x)
+$(info $(x))
+
+# TODO: shouldn't crash.
+#y = one
+#y += two $(y)
+#$(info $(y))
\ No newline at end of file
diff --git a/testcase/fail_ignore_error.mk b/testcase/fail_ignore_error.mk
new file mode 100644
index 0000000..c4e71a6
--- /dev/null
+++ b/testcase/fail_ignore_error.mk
@@ -0,0 +1,6 @@
+# TODO(go-ninja): Fix
+
+test:
+	false
+	-false
+	echo FAIL
diff --git a/testcase/fail_subshell_in_recipe.mk b/testcase/fail_subshell_in_recipe.mk
new file mode 100644
index 0000000..c2d2b2d
--- /dev/null
+++ b/testcase/fail_subshell_in_recipe.mk
@@ -0,0 +1,5 @@
+# TODO(go-ninja): Fix
+
+test:
+	false
+	(true) ; echo FAIL
diff --git a/var.cc b/var.cc
index 73f542c..4b67c4f 100644
--- a/var.cc
+++ b/var.cc
@@ -60,8 +60,10 @@ void SimpleVar::Eval(Evaluator*, string* s) const {
 }
 
 void SimpleVar::AppendVar(Evaluator* ev, Value* v) {
+  string buf;
+  v->Eval(ev, &buf);
   v_.push_back(' ');
-  v->Eval(ev, &v_);
+  v_ += buf;
 }
 
 StringPiece SimpleVar::String() const {