commit dbbb724fd1586a4974bcbc6fa69888ea834bebce
author dpranke <dpranke@chromium.org> Tue Sep 20 15:04:08 2016 -0700
committer Commit bot <commit-bot@chromium.org> Tue Sep 20 22:06:24 2016 +0000
tree 4b9f8ec464cd44ac5d7dbccf66f8716cd03c9dd0
parent 3e35519a885a6793f7655d993e7e728a437f3709

Fix an issue in `gn analyze` when building all.

If you ran `gn analyze` on a list of files that changed a build file,
the list of compile targets and output targets should've been returned
unchanged, but if the additional_compile_targets input included "all", the
returned list was left empty.

R=brettw@chromium.org
BUG=648532

Review-Url: https://codereview.chromium.org/2350963006
Cr-Original-Commit-Position: refs/heads/master@{#419866}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 385a310b530199122f9781ead9410afe09b9c8d2

tools/gn/analyzer.cc

diff --git a/tools/gn/analyzer.cc b/tools/gn/analyzer.cc
index f89c93d..011b77b 100644
--- a/tools/gn/analyzer.cc
+++ b/tools/gn/analyzer.cc
@@ -34,7 +34,7 @@
   std::vector<SourceFile> source_vec;
   std::vector<Label> compile_vec;
   std::vector<Label> test_vec;
-  bool compile_included_all;
+  bool compile_included_all = false;
   SourceFileSet source_files;
   LabelSet compile_labels;
   LabelSet test_labels;
@@ -43,6 +43,7 @@
 struct Outputs {
   std::string status;
   std::string error;
+  bool compile_includes_all = false;
   LabelSet compile_labels;
   LabelSet test_labels;
   LabelSet invalid_labels;
@@ -203,8 +204,14 @@
                 outputs.invalid_labels);
   } else {
     WriteString(*value, "status", outputs.status);
-    WriteLabels(default_toolchain, *value, "compile_targets",
-                outputs.compile_labels);
+    if (outputs.compile_includes_all) {
+      auto compile_targets = base::WrapUnique(new base::ListValue());
+      compile_targets->AppendString("all");
+      value->Set("compile_targets", std::move(compile_targets));
+    } else {
+      WriteLabels(default_toolchain, *value, "compile_targets",
+                  outputs.compile_labels);
+    }
     WriteLabels(default_toolchain, *value, "test_targets", outputs.test_labels);
   }
 
@@ -259,7 +266,14 @@
   // or toolchain defined in that file.
   if (AnyBuildFilesWereModified(inputs.source_files)) {
     outputs.status = "Found dependency (all)";
-    outputs.compile_labels = inputs.compile_labels;
+    if (inputs.compile_included_all) {
+      outputs.compile_includes_all = true;
+    } else {
+      outputs.compile_labels.insert(inputs.compile_labels.begin(),
+                                    inputs.compile_labels.end());
+      outputs.compile_labels.insert(inputs.test_labels.begin(),
+                                    inputs.test_labels.end());
+    }
     outputs.test_labels = inputs.test_labels;
     return OutputsToJSON(outputs, default_toolchain_, err);
   }

tools/gn/analyzer_unittest.cc

diff --git a/tools/gn/analyzer_unittest.cc b/tools/gn/analyzer_unittest.cc
index 4d5c8d4..a3c8d5b 100644
--- a/tools/gn/analyzer_unittest.cc
+++ b/tools/gn/analyzer_unittest.cc
@@ -212,7 +212,24 @@
       "  \"test_targets\": [ \"//:a\" ]"
       "}",
       "{"
-      "\"compile_targets\":[],"
+      "\"compile_targets\":[\"//:a\"],"
+      "\"status\":\"Found dependency (all)\","
+      "\"test_targets\":[\"//:a\"]"
+      "}");
+}
+
+TEST_F(AnalyzerTest, BuildFilesWereModifiedAndCompilingAll) {
+  // This tests that if a build file is modified, we bail out early with
+  // "Found dependency (all)" error since we can't handle changes to
+  // build files yet (crbug.com/555273).
+  RunBasicTest(
+      "{"
+      "  \"files\": [ \"//a.cc\", \"//BUILD.gn\" ],"
+      "  \"additional_compile_targets\": [ \"all\" ],"
+      "  \"test_targets\": [ \"//:a\" ]"
+      "}",
+      "{"
+      "\"compile_targets\":[\"all\"],"
       "\"status\":\"Found dependency (all)\","
       "\"test_targets\":[\"//:a\"]"
       "}");