Fix various issues with `gn analyze`.
I had the name of one of the input fields wrong, and I had
an error case not returning output correctly.
R=brettw@chromium.org
BUG=555273
Review-Url: https://codereview.chromium.org/2341683006
Cr-Original-Commit-Position: refs/heads/master@{#419075}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: ac5096069eaa4a7716f6441a1af04e59b635adb7
diff --git a/tools/gn/analyzer.cc b/tools/gn/analyzer.cc
index 38acb6b..8ee303f 100644
--- a/tools/gn/analyzer.cc
+++ b/tools/gn/analyzer.cc
@@ -157,7 +157,7 @@
inputs->source_vec.push_back(SourceFile(s));
}
- strings = GetStringVector(*dict, "compile_targets", &err);
+ strings = GetStringVector(*dict, "additional_compile_targets", &err);
if (err.has_error())
return err;
@@ -193,7 +193,7 @@
}
std::string OutputsToJSON(const Outputs& outputs,
- const Label& default_toolchain) {
+ const Label& default_toolchain, Err *err) {
std::string output;
auto value = base::MakeUnique<base::DictionaryValue>();
@@ -208,7 +208,8 @@
WriteLabels(default_toolchain, *value, "test_targets", outputs.test_labels);
}
- base::JSONWriter::Write(*value.get(), &output);
+ if (!base::JSONWriter::Write(*value.get(), &output))
+ *err = Err(Location(), "Failed to marshal JSON value for output");
return output;
}
@@ -237,9 +238,7 @@
Err local_err = JSONToInputs(default_toolchain_, input, &inputs);
if (local_err.has_error()) {
outputs.error = local_err.message();
- if (err)
- *err = Err();
- return "";
+ return OutputsToJSON(outputs, default_toolchain_, err);
}
LabelSet invalid_labels;
@@ -250,17 +249,13 @@
if (!invalid_labels.empty()) {
outputs.error = "Invalid targets";
outputs.invalid_labels = invalid_labels;
- if (err)
- *err = Err();
- return OutputsToJSON(outputs, default_toolchain_);
+ return OutputsToJSON(outputs, default_toolchain_, err);
}
TargetSet affected_targets = AllAffectedTargets(inputs.source_files);
if (affected_targets.empty()) {
outputs.status = "No dependency";
- if (err)
- *err = Err();
- return OutputsToJSON(outputs, default_toolchain_);
+ return OutputsToJSON(outputs, default_toolchain_, err);
}
// TODO: We can do smarter things when we detect changes to build files.
@@ -272,9 +267,7 @@
outputs.status = "Found dependency (all)";
outputs.compile_labels = inputs.compile_labels;
outputs.test_labels = inputs.test_labels;
- if (err)
- *err = Err();
- return OutputsToJSON(outputs, default_toolchain_);
+ return OutputsToJSON(outputs, default_toolchain_, err);
}
TargetSet compile_targets = TargetsFor(inputs.compile_labels);
@@ -293,8 +286,7 @@
outputs.status = "No dependency";
else
outputs.status = "Found dependency";
- *err = Err();
- return OutputsToJSON(outputs, default_toolchain_);
+ return OutputsToJSON(outputs, default_toolchain_, err);
}
TargetSet Analyzer::AllAffectedTargets(
diff --git a/tools/gn/analyzer_unittest.cc b/tools/gn/analyzer_unittest.cc
index 6910a67..dba0e77 100644
--- a/tools/gn/analyzer_unittest.cc
+++ b/tools/gn/analyzer_unittest.cc
@@ -120,7 +120,7 @@
RunBasicTest(
"{"
" \"files\": [ \"//d/b.cc\" ],"
- " \"compile_targets\": [ \"all\" ],"
+ " \"additional_compile_targets\": [ \"all\" ],"
" \"test_targets\": [ ]"
"}",
"{"
@@ -134,7 +134,7 @@
RunBasicTest(
"{"
" \"files\":[ \"//missing.cc\" ],"
- " \"compile_targets\": [ \"all\" ],"
+ " \"additional_compile_targets\": [ \"all\" ],"
" \"test_targets\": [ \"//:a\" ]"
"}",
"{"
@@ -148,7 +148,7 @@
RunBasicTest(
"{"
" \"files\": [],"
- " \"compile_targets\": [],"
+ " \"additional_compile_targets\": [],"
" \"test_targets\": []"
"}",
"{"
@@ -162,7 +162,7 @@
RunBasicTest(
"{"
" \"files\": [ \"//a.cc\" ],"
- " \"compile_targets\": [],"
+ " \"additional_compile_targets\": [],"
" \"test_targets\": [ \"//:a\" ]"
"}",
"{"
@@ -171,3 +171,32 @@
"\"test_targets\":[\"//:a\"]"
"}");
}
+
+TEST_F(AnalyzerTest, FilesArentSourceAbsolute) {
+ RunBasicTest(
+ "{"
+ " \"files\": [ \"a.cc\" ],"
+ " \"additional_compile_targets\": [],"
+ " \"test_targets\": [ \"//:a\" ]"
+ "}",
+ "{"
+ "\"error\":"
+ "\"\\\"a.cc\\\" is not a source-absolute or absolute path.\","
+ "\"invalid_targets\":[]"
+ "}");
+}
+
+TEST_F(AnalyzerTest, WrongInputFields) {
+ RunBasicTest(
+ "{"
+ " \"files\": [ \"//a.cc\" ],"
+ " \"compile_targets\": [],"
+ " \"test_targets\": [ \"//:a\" ]"
+ "}",
+ "{"
+ "\"error\":"
+ "\"Input does not have a key named "
+ "\\\"additional_compile_targets\\\" with a list value.\","
+ "\"invalid_targets\":[]"
+ "}");
+}
diff --git a/tools/gn/command_analyze.cc b/tools/gn/command_analyze.cc
index a9c0d9a..03519cb 100644
--- a/tools/gn/command_analyze.cc
+++ b/tools/gn/command_analyze.cc
@@ -34,27 +34,26 @@
"\n"
" - \"files\": A list of the filenames to check.\n"
"\n"
- " - \"compile_targets\": A list of the labels targets that we wish to\n"
- " rebuild, but aren't necessarily needed for testing. The\n"
- " important difference between this field and \"test_targets\"\n"
- " is that if an item in the compile_targets list is a group, then\n"
+ " - \"test_targets\": A list of the labels for targets that\n"
+ " are needed to run the tests we wish to run.\n"
+ "\n"
+ " - \"additional_compile_targets\": A list of the labels for\n"
+ " targets that we wish to rebuild, but aren't necessarily needed\n"
+ " for testing. The important difference between this field and\n"
+ " \"test_targets\" is that if an item in the\n"
+ " additional_compile_targets list refers to a group, then\n"
" any dependencies of that group will be returned if they are out\n"
" of date, but the group itself does not need to be. If the\n"
" dependencies themselves are groups, the same filtering is\n"
" repeated. This filtering can be used to avoid rebuilding\n"
" dependencies of a group that are unaffected by the input files.\n"
- " The list may contain the string \"all\" to refer to a\n"
- " pseudo-group that contains every root target in the build graph.\n"
+ " The list may also contain the string \"all\" to refer to a\n"
+ " pseudo-group that contains every root target in the build\n"
+ " graph.\n"
"\n"
" This filtering behavior is also known as \"pruning\" the list\n"
" of compile targets.\n"
"\n"
- " - \"test_targets\": A list of the labels for targets that\n"
- " are needed to run the tests we wish to run. Unlike\n"
- " \"compile_targets\", this list may not contain the string \"all\",\n"
- " because having a test be dependent on everything in the build\n"
- " would be silly.\n"
- "\n"
" output_path is a path indicating where the results of the command\n"
" are to be written. The results will be a file containing a JSON\n"
" object with one or more of following fields:\n"
diff --git a/tools/gn/docs/reference.md b/tools/gn/docs/reference.md
index 540d3ef..356712f 100644
--- a/tools/gn/docs/reference.md
+++ b/tools/gn/docs/reference.md
@@ -259,27 +259,26 @@
- "files": A list of the filenames to check.
- - "compile_targets": A list of the labels targets that we wish to
- rebuild, but aren't necessarily needed for testing. The
- important difference between this field and "test_targets"
- is that if an item in the compile_targets list is a group, then
+ - "test_targets": A list of the labels for targets that
+ are needed to run the tests we wish to run.
+
+ - "additional_compile_targets": A list of the labels for
+ targets that we wish to rebuild, but aren't necessarily needed
+ for testing. The important difference between this field and
+ "test_targets" is that if an item in the
+ additional_compile_targets list refers to a group, then
any dependencies of that group will be returned if they are out
of date, but the group itself does not need to be. If the
dependencies themselves are groups, the same filtering is
repeated. This filtering can be used to avoid rebuilding
dependencies of a group that are unaffected by the input files.
- The list may contain the string "all" to refer to a
- pseudo-group that contains every root target in the build graph.
+ The list may also contain the string "all" to refer to a
+ pseudo-group that contains every root target in the build
+ graph.
This filtering behavior is also known as "pruning" the list
of compile targets.
- - "test_targets": A list of the labels for targets that
- are needed to run the tests we wish to run. Unlike
- "compile_targets", this list may not contain the string "all",
- because having a test be dependent on everything in the build
- would be silly.
-
output_path is a path indicating where the results of the command
are to be written. The results will be a file containing a JSON
object with one or more of following fields:
