Fix GN action target writing with no sources list.

Properly write outputs of actions that have no sources list. Previously this would lead to empty output lists in a build rule because the writer code was trying to apply the sources list to the output substitutions list. However, for actions, the output is the literal file list and should have no substitutions.

R=jamesr@chromium.org

Review URL: https://codereview.chromium.org/461153002

Cr-Commit-Position: refs/heads/master@{#289109}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289109 0039d316-1c4b-4281-b951-d872f2087c98

tools/gn/function_get_target_outputs.cc

@@ -20,13 +20,11 @@ void GetOutputsForTarget(const Settings* settings,
                          std::vector<SourceFile>* ret) {
   switch (target->output_type()) {
     case Target::ACTION: {
-      // Actions just use the output list with no substitution. To keep things
-      // simple, pass an empty "source file" in to use the same code path for
-      // computing substitution outputs.
+      // Actions just use the output list with no substitution.
       std::vector<SourceFile> sources;
       sources.push_back(SourceFile());
-      SubstitutionWriter::ApplyListToSources(
-          settings, target->action_values().outputs(), sources, ret);
+      SubstitutionWriter::GetListAsSourceFiles(
+          settings, target->action_values().outputs(), ret);
       break;
     }
 

tools/gn/ninja_action_target_writer.cc

@@ -53,11 +53,10 @@ void NinjaActionTargetWriter::Run() {
     DCHECK(target_->output_type() == Target::ACTION);
 
     // Write a rule that invokes the script once with the outputs as outputs,
-    // and the data as inputs.
+    // and the data as inputs. It does not depend on the sources.
     out_ << "build";
-    SubstitutionWriter::ApplyListToSourcesAsOutputFile(
-        settings_, target_->action_values().outputs(), target_->sources(),
-        &output_files);
+    SubstitutionWriter::GetListAsOutputFiles(
+        settings_, target_->action_values().outputs(), &output_files);
     for (size_t i = 0; i < output_files.size(); i++) {
       out_ << " ";
       path_output_.WriteFile(out_, output_files[i]);

tools/gn/ninja_action_target_writer_unittest.cc

@@ -28,6 +28,41 @@ TEST(NinjaActionTargetWriter, WriteOutputFilesForBuildLine) {
   EXPECT_EQ(" gen/a$ bbar.h gen/bar.cc", out.str());
 }
 
+// Tests an action with no sources.
+TEST(NinjaActionTargetWriter, ActionNoSources) {
+  TestWithScope setup;
+  setup.build_settings()->SetBuildDir(SourceDir("//out/Debug/"));
+  Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+  target.set_output_type(Target::ACTION);
+
+  target.action_values().set_script(SourceFile("//foo/script.py"));
+  target.inputs().push_back(SourceFile("//foo/included.txt"));
+
+  target.action_values().outputs() =
+      SubstitutionList::MakeForTest("//out/Debug/foo.out");
+
+  setup.settings()->set_target_os(Settings::LINUX);
+  setup.build_settings()->set_python_path(base::FilePath(FILE_PATH_LITERAL(
+      "/usr/bin/python")));
+
+  std::ostringstream out;
+  NinjaActionTargetWriter writer(&target, setup.toolchain(), out);
+  writer.Run();
+
+  const char expected[] =
+      "rule __foo_bar___rule\n"
+      "  command = /usr/bin/python ../../foo/script.py\n"
+      "  description = ACTION //foo:bar()\n"
+      "  restat = 1\n"
+      "build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
+          "../../foo/included.txt\n"
+      "\n"
+      "build foo.out: __foo_bar___rule | obj/foo/bar.inputdeps.stamp\n"
+      "\n"
+      "build obj/foo/bar.stamp: stamp foo.out\n";
+  EXPECT_EQ(expected, out.str());
+}
+
 // Makes sure that we write sources as input dependencies for actions with
 // both sources and inputs (ACTION_FOREACH treats the sources differently).
 TEST(NinjaActionTargetWriter, ActionWithSources) {

tools/gn/substitution_writer.cc

@@ -124,6 +124,40 @@ SubstitutionWriter::SubstitutionWriter() {
 SubstitutionWriter::~SubstitutionWriter() {
 }
 
+// static
+void SubstitutionWriter::GetListAsSourceFiles(
+    const Settings* settings,
+    const SubstitutionList& list,
+    std::vector<SourceFile>* output) {
+  for (size_t i = 0; i < list.list().size(); i++) {
+    const SubstitutionPattern& pattern = list.list()[i];
+    CHECK(pattern.ranges().size() == 1 &&
+          pattern.ranges()[0].type == SUBSTITUTION_LITERAL)
+        << "The substitution patterm \""
+        << pattern.AsString()
+        << "\" was expected to be a literal with no {{substitutions}}.";
+    const std::string& literal = pattern.ranges()[0].literal;
+    CHECK(literal.size() >= 1 && literal[0] == '/')
+        << "The result of the pattern \""
+        << pattern.AsString()
+        << "\" was not an absolute path.";
+    output->push_back(SourceFile(literal));
+  }
+}
+
+void SubstitutionWriter::GetListAsOutputFiles(
+    const Settings* settings,
+    const SubstitutionList& list,
+    std::vector<OutputFile>* output) {
+  std::vector<SourceFile> output_as_sources;
+  GetListAsSourceFiles(settings, list, &output_as_sources);
+  for (size_t i = 0; i < output_as_sources.size(); i++) {
+    output->push_back(OutputFile(
+        RebaseSourceAbsolutePath(output_as_sources[i].value(),
+                                 settings->build_settings()->build_dir())));
+  }
+}
+
 // static
 SourceFile SubstitutionWriter::ApplyPatternToSource(
       const Settings* settings,

tools/gn/substitution_writer.h

@@ -33,6 +33,19 @@ class SubstitutionWriter {
   SubstitutionWriter();
   ~SubstitutionWriter();
 
+  // Converts the given SubstitutionList to OutputFiles assuming there are
+  // no substitutions (it will assert if there are). This is used for cases
+  // like actions where the outputs are explicit, but the list is stored as
+  // a SubstitutionList.
+  static void GetListAsSourceFiles(
+      const Settings* settings,
+      const SubstitutionList& list,
+      std::vector<SourceFile>* output);
+  static void GetListAsOutputFiles(
+      const Settings* settings,
+      const SubstitutionList& list,
+      std::vector<OutputFile>* output);
+
   // Applies the substitution pattern to a source file, returning the result
   // as either a SourceFile or OutputFile.
   static SourceFile ApplyPatternToSource(