commit 63b01d6483a30bc11b072b1a051fa2076ceb4948
author Sylvain Defresne <sdefresne@chromium.org> Mon Jun 22 10:23:39 2020 +0200
committer Commit Bot <commit-bot@chromium.org> Mon Jun 22 08:31:05 2020 +0000
tree 45d1d22e563cbecc95530ddff4582396a17a5925
parent eb997b5ab9c3f1ba6a2c52072785884864a84eae

Check that required tool property "command" is defined

The property "command" is required for all tools except "action",
so check that it is defined when initializing the tool (instead
of aborting the execution when generating the ninja file).

Bug: 46
Change-Id: Ie364fd388bea3e7869a2991ba27727ae57665202
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/9022
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>

src/gn/function_toolchain_unittest.cc

diff --git a/src/gn/function_toolchain_unittest.cc b/src/gn/function_toolchain_unittest.cc
index ba4bfb6..74cbef9 100644
--- a/src/gn/function_toolchain_unittest.cc
+++ b/src/gn/function_toolchain_unittest.cc
@@ -43,6 +43,7 @@
     TestParseInput input(
         R"(toolchain("good") {
           tool("link") {
+            command = "link"
             outputs = [ "foo" ]
             runtime_outputs = [ "foo" ]
           }
@@ -116,12 +117,27 @@
   }
 }
 
+TEST_F(FunctionToolchain, Command) {
+  TestWithScope setup;
+
+  TestParseInput input(
+      R"(toolchain("missing_command") {
+        tool("cxx") {}
+      })");
+  ASSERT_FALSE(input.has_error());
+
+  Err err;
+  input.parsed()->Execute(setup.scope(), &err);
+  ASSERT_TRUE(err.has_error()) << err.message();
+}
+
 TEST_F(FunctionToolchain, CommandLauncher) {
   TestWithScope setup;
 
   TestParseInput input(
       R"(toolchain("good") {
         tool("cxx") {
+          command = "cxx"
           command_launcher = "/usr/goma/gomacc"
         }
       })");

src/gn/ninja_toolchain_writer.cc

diff --git a/src/gn/ninja_toolchain_writer.cc b/src/gn/ninja_toolchain_writer.cc
index e38187f..3793ab0 100644
--- a/src/gn/ninja_toolchain_writer.cc
+++ b/src/gn/ninja_toolchain_writer.cc
@@ -132,8 +132,6 @@
     const SubstitutionPattern& command,
     const EscapeOptions& options) {
   CHECK(!command.empty()) << "Command should not be empty";
-  if (command.empty())
-    return;
   out_ << kIndent << name << " = " ;
   if (!launcher.empty())
     out_ << launcher << " ";

src/gn/tool.cc

diff --git a/src/gn/tool.cc b/src/gn/tool.cc
index 3b29775..99b7525 100644
--- a/src/gn/tool.cc
+++ b/src/gn/tool.cc
@@ -205,6 +205,14 @@
       !ReadLabel(scope, "pool", toolchain->label(), &pool_, err)) {
     return false;
   }
+  const bool command_is_required = name_ != GeneralTool::kGeneralToolAction;
+  if (command_.empty() == command_is_required) {
+    *err = Err(defined_from(), "This tool's command is bad.",
+               command_is_required
+                   ? "This tool requires \"command\" to be defined."
+                   : "This tool doesn't support \"command\".");
+    return false;
+  }
   return true;
 }