Google Git
Sign in
gn/gn/refs/heads/main^!/.
commit
3357c4f51b1a9e676378c695dd9c7e9911c35ee6
[log]
author
Peter Collingbourne <pcc@chromium.org>
Thu May 21 13:16:33 2026 -0700
committer
gn-scoped@luci-project-accounts.iam.gserviceaccount.com <gn-scoped@luci-project-accounts.iam.gserviceaccount.com>
Tue May 26 19:33:58 2026 -0700
tree
bc02f5a877eb4348db872a71fe0d172ca98f0c8b
parent
0fefaeb1b9f7cf0ef8faf03f8e906d1449f0c714 [diff]
Reland "Fix toolchain dependencies not triggering rebuilds in Ninja"

This reverts commit 576a0ebd4cc674fb52681bd1becfd0facec8449e.

Reason for revert: Siso fix has rolled out: b/512275057

Original change's description:
> Revert "Fix toolchain dependencies not triggering rebuilds in Ninja"
>
> This reverts commit 9ece3f5254c273cb46606a6571963f931c3b012d.
>
> Reason for revert: This doesn't pass chromium CQ.
> https://chromium-review.git.corp.google.com/c/chromium/src/+/7810380
>
> Original change's description:
> > Fix toolchain dependencies not triggering rebuilds in Ninja
> >
> > GN previously emitted order-only dependencies on toolchain dependencies
> > (via the .inputdeps.stamp file). This meant that if the toolchain changed,
> > targets compiled by it were not rebuilt.
> >
> > This change modifies WriteInputDepsStampOrPhonyAndGetDep (WIDSOPAGD)
> > to return implicit dependencies separately from order-only
> > dependencies. Toolchain dependencies are now returned as implicit
> > dependencies, which ensures that Ninja triggers a rebuild when the
> > toolchain changes.
> >
> > All target writers have been updated to write the result of WIDSOPAGD as
> > either implicit or order-only dependencies. The action target writer is
> > a special case, because before this change it would write the result of
> > WIDSOPAGD as an implicit dependency, unlike all other target writers,
> > which would write it as an order-only dependency. This means that before
> > this change, all dependencies returned by WIDSOPAGD were implicit
> > dependencies for action targets. Because scripts may depend on this
> > behavior, any phony targets created by WIDSOPAGD for action targets have
> > only implicit dependencies.
> >
> > Bug: 40643484
> > Change-Id: Iae5dbf87cd67ecd5c9d0f6138f17085233c844b3
> > Reviewed-on: https://gn-review.googlesource.com/c/gn/+/22000
> > Reviewed-by: Takuto Ikuta <tikuta@google.com>
> > Commit-Queue: Peter Collingbourne <pcc@google.com>
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 40643484
> Change-Id: I8f38fc48dba7a214a6ef354d2550f91a842ca8c6
> Reviewed-on: https://gn-review.googlesource.com/c/gn/+/22520
> Commit-Queue: Takuto Ikuta <tikuta@google.com>
> Reviewed-by: Junji Watanabe <jwata@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 40643484
Change-Id: I4e5c263c92a1e24abfaaefa7f5f0c62565b233d6
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/22860
Reviewed-by: Junji Watanabe <jwata@google.com>
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Commit-Queue: Peter Collingbourne <pcc@chromium.org>

diff --git a/src/gn/ninja_action_target_writer.cc b/src/gn/ninja_action_target_writer.cc
index 3aa5686..0d01d07 100644
--- a/src/gn/ninja_action_target_writer.cc
+++ b/src/gn/ninja_action_target_writer.cc

@@ -73,8 +73,11 @@
   // and the action will just depend on all the input deps directly.
   size_t num_output_uses =
       target_->output_type() == Target::ACTION ? 1u : target_->sources().size();
-  std::vector<OutputFile> input_deps = WriteInputDepsStampOrPhonyAndGetDep(
+  NinjaTargetWriter::InputDeps stamp_deps = WriteInputDepsStampOrPhonyAndGetDep(
       additional_hard_deps, num_output_uses);
+  std::vector<OutputFile> input_deps = stamp_deps.implicit;
+  input_deps.insert(input_deps.end(), stamp_deps.order_only.begin(),
+                    stamp_deps.order_only.end());
   out_ << std::endl;
 
   // Collects all output files for writing below.

diff --git a/src/gn/ninja_bundle_data_target_writer.cc b/src/gn/ninja_bundle_data_target_writer.cc
index 4ba1733..d6c2e56 100644
--- a/src/gn/ninja_bundle_data_target_writer.cc
+++ b/src/gn/ninja_bundle_data_target_writer.cc

@@ -21,11 +21,14 @@
         OutputFile(settings_->build_settings(), source_file));
   }
 
-  std::vector<OutputFile> input_deps = WriteInputDepsStampOrPhonyAndGetDep(
-      std::vector<const Target*>(), /*num_output_uses=*/1);
-  output_files.insert(output_files.end(), input_deps.begin(), input_deps.end());
+  NinjaTargetWriter::InputDeps stamp_deps =
+      WriteInputDepsStampOrPhonyAndGetDep(std::vector<const Target*>(),
+                                          /*num_output_uses=*/1);
+  std::vector<OutputFile> implicit_deps = stamp_deps.implicit;
+  std::vector<OutputFile> order_only_deps = stamp_deps.order_only;
+  output_files.insert(output_files.end(), implicit_deps.begin(),
+                      implicit_deps.end());
 
-  std::vector<OutputFile> order_only_deps;
   for (const Target* data_dep : resolved().GetDataDeps(target_)) {
     if (data_dep->has_dependency_output())
       order_only_deps.push_back(data_dep->dependency_output());

diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index 4f19645..812803e 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc

@@ -104,8 +104,11 @@
   // that also use PCH files won't have a phony target even though having
   // one would make output ninja file size a bit lower. That's ok, binary
   // targets with a single source are rare.
-  std::vector<OutputFile> order_only_deps = WriteInputDepsStampOrPhonyAndGetDep(
+  NinjaTargetWriter::InputDeps stamp_deps = WriteInputDepsStampOrPhonyAndGetDep(
       std::vector<const Target*>(), num_output_uses);
+  input_deps.insert(input_deps.end(), stamp_deps.implicit.begin(),
+                    stamp_deps.implicit.end());
+  std::vector<OutputFile> order_only_deps = stamp_deps.order_only;
 
   // For GCC builds, the .gch files are not object files, but still need to be
   // added as explicit dependencies below. The .gch output files are placed in

diff --git a/src/gn/ninja_copy_target_writer.cc b/src/gn/ninja_copy_target_writer.cc
index 0952c6f..16fca06 100644
--- a/src/gn/ninja_copy_target_writer.cc
+++ b/src/gn/ninja_copy_target_writer.cc

@@ -72,13 +72,14 @@
       GetNinjaRulePrefixForToolchain(settings_) + GeneralTool::kGeneralToolCopy;
 
   size_t num_output_uses = target_->sources().size();
-  std::vector<OutputFile> input_deps = WriteInputDepsStampOrPhonyAndGetDep(
+  NinjaTargetWriter::InputDeps stamp_deps = WriteInputDepsStampOrPhonyAndGetDep(
       std::vector<const Target*>(), num_output_uses);
+  std::vector<OutputFile> implicit_deps = stamp_deps.implicit;
+  std::vector<OutputFile> order_only_deps = stamp_deps.order_only;
 
-  std::vector<OutputFile> data_outs;
   for (const Target* data_dep : resolved().GetDataDeps(target_)) {
     if (data_dep->has_dependency_output())
-      data_outs.push_back(data_dep->dependency_output());
+      order_only_deps.push_back(data_dep->dependency_output());
   }
 
   // Note that we don't write implicit deps for copy steps. "copy" only
@@ -119,10 +120,13 @@
 
     out_ << ": " << tool_name << " ";
     path_output_.WriteFile(out_, input_file);
-    if (!input_deps.empty() || !data_outs.empty()) {
+    if (!implicit_deps.empty()) {
+      out_ << " |";
+      path_output_.WriteFiles(out_, implicit_deps);
+    }
+    if (!order_only_deps.empty()) {
       out_ << " ||";
-      path_output_.WriteFiles(out_, input_deps);
-      path_output_.WriteFiles(out_, data_outs);
+      path_output_.WriteFiles(out_, order_only_deps);
     }
     WriteValidations();
     out_ << std::endl;

diff --git a/src/gn/ninja_copy_target_writer_unittest.cc b/src/gn/ninja_copy_target_writer_unittest.cc
index da7da59..6fc5e43 100644
--- a/src/gn/ninja_copy_target_writer_unittest.cc
+++ b/src/gn/ninja_copy_target_writer_unittest.cc

@@ -208,10 +208,10 @@
     writer.Run();
 
     const char expected_linux[] =
-        "build phony/foo/bar.inputdeps: phony phony/foo/action1 "
+        "build phony/foo/bar.inputdeps: phony || phony/foo/action1 "
         "phony/foo/action2\n"
-        "build action1.copy: copy action1.out || phony/foo/bar.inputdeps\n"
-        "build action2.copy: copy action2.out || phony/foo/bar.inputdeps\n"
+        "build action1.copy: copy action1.out | phony/foo/bar.inputdeps\n"
+        "build action2.copy: copy action2.out | phony/foo/bar.inputdeps\n"
         "\n"
         "build phony/foo/bar: phony action1.copy action2.copy\n";
     std::string out_str = out.str();

diff --git a/src/gn/ninja_create_bundle_target_writer.cc b/src/gn/ninja_create_bundle_target_writer.cc
index b3a15ba..ca305e6 100644
--- a/src/gn/ninja_create_bundle_target_writer.cc
+++ b/src/gn/ninja_create_bundle_target_writer.cc

@@ -81,16 +81,18 @@
   // Stamp users are CopyBundleData, CompileAssetsCatalog, PostProcessing and
   // StampForTarget.
   size_t num_stamp_uses = 4;
-  std::vector<OutputFile> order_only_deps = WriteInputDepsStampOrPhonyAndGetDep(
+  NinjaTargetWriter::InputDeps stamp_deps = WriteInputDepsStampOrPhonyAndGetDep(
       std::vector<const Target*>(), num_stamp_uses);
+  std::vector<OutputFile> implicit_deps = stamp_deps.implicit;
+  std::vector<OutputFile> order_only_deps = stamp_deps.order_only;
 
   std::string post_processing_rule_name = WritePostProcessingRuleDefinition();
 
   std::vector<OutputFile> output_files;
-  WriteCopyBundleDataSteps(order_only_deps, &output_files);
-  WriteCompileAssetsCatalogStep(order_only_deps, &output_files);
-  WritePostProcessingStep(post_processing_rule_name, order_only_deps,
-                          &output_files);
+  WriteCopyBundleDataSteps(implicit_deps, order_only_deps, &output_files);
+  WriteCompileAssetsCatalogStep(implicit_deps, order_only_deps, &output_files);
+  WritePostProcessingStep(post_processing_rule_name, implicit_deps,
+                          order_only_deps, &output_files);
 
   for (const Target* data_dep : resolved().GetDataDeps(target_)) {
     if (data_dep->has_dependency_output())
@@ -179,14 +181,17 @@
 }
 
 void NinjaCreateBundleTargetWriter::WriteCopyBundleDataSteps(
+    const std::vector<OutputFile>& implicit_deps,
     const std::vector<OutputFile>& order_only_deps,
     std::vector<OutputFile>* output_files) {
   for (const BundleFileRule& file_rule : target_->bundle_data().file_rules())
-    WriteCopyBundleFileRuleSteps(file_rule, order_only_deps, output_files);
+    WriteCopyBundleFileRuleSteps(file_rule, implicit_deps, order_only_deps,
+                                 output_files);
 }
 
 void NinjaCreateBundleTargetWriter::WriteCopyBundleFileRuleSteps(
     const BundleFileRule& file_rule,
+    const std::vector<OutputFile>& implicit_deps,
     const std::vector<OutputFile>& order_only_deps,
     std::vector<OutputFile>* output_files) {
   // Note that we don't write implicit deps for copy steps. "copy_bundle_data"
@@ -209,6 +214,10 @@
          << GeneralTool::kGeneralToolCopyBundleData << " ";
     path_output_.WriteFile(out_, source_file);
 
+    if (!implicit_deps.empty()) {
+      out_ << " |";
+      path_output_.WriteFiles(out_, implicit_deps);
+    }
     if (!order_only_deps.empty()) {
       out_ << " ||";
       path_output_.WriteFiles(out_, order_only_deps);
@@ -219,6 +228,7 @@
 }
 
 void NinjaCreateBundleTargetWriter::WriteCompileAssetsCatalogStep(
+    const std::vector<OutputFile>& implicit_deps,
     const std::vector<OutputFile>& order_only_deps,
     std::vector<OutputFile>* output_files) {
   if (!TargetRequireAssetCatalogCompilation(target_))
@@ -251,6 +261,10 @@
     WriteOutput(partial_info_plist);
     out_ << ": " << GetNinjaRulePrefixForToolchain(settings_)
          << GeneralTool::kGeneralToolStamp;
+    if (!implicit_deps.empty()) {
+      out_ << " |";
+      path_output_.WriteFiles(out_, implicit_deps);
+    }
     if (!order_only_deps.empty()) {
       out_ << " ||";
       path_output_.WriteFiles(out_, order_only_deps);
@@ -286,6 +300,7 @@
 
   out_ << " | ";
   path_output_.WriteFile(out_, input_dep);
+  path_output_.WriteFiles(out_, implicit_deps);
 
   if (!order_only_deps.empty()) {
     out_ << " ||";
@@ -362,13 +377,15 @@
 
 void NinjaCreateBundleTargetWriter::WritePostProcessingStep(
     const std::string& post_processing_rule_name,
+    const std::vector<OutputFile>& implicit_deps,
     const std::vector<OutputFile>& order_only_deps,
     std::vector<OutputFile>* output_files) {
   if (post_processing_rule_name.empty())
     return;
 
   OutputFile post_processing_input_stamp_file =
-      WritePostProcessingInputDepsStampOrPhony(order_only_deps, output_files);
+      WritePostProcessingInputDepsStampOrPhony(implicit_deps, order_only_deps,
+                                               output_files);
   DCHECK(!post_processing_input_stamp_file.value().empty());
 
   out_ << "build";
@@ -391,6 +408,7 @@
 
 OutputFile
 NinjaCreateBundleTargetWriter::WritePostProcessingInputDepsStampOrPhony(
+    const std::vector<OutputFile>& implicit_deps,
     const std::vector<OutputFile>& order_only_deps,
     std::vector<OutputFile>* output_files) {
   std::vector<SourceFile> post_processing_input_files;
@@ -406,7 +424,8 @@
   }
 
   DCHECK(!post_processing_input_files.empty());
-  if (post_processing_input_files.size() == 1 && order_only_deps.empty())
+  if (post_processing_input_files.size() == 1 && implicit_deps.empty() &&
+      order_only_deps.empty())
     return OutputFile(settings_->build_settings(),
                       post_processing_input_files[0]);
 
@@ -438,6 +457,10 @@
     out_ << " ";
     path_output_.WriteFile(out_, source);
   }
+  if (!implicit_deps.empty()) {
+    out_ << " |";
+    path_output_.WriteFiles(out_, implicit_deps);
+  }
   if (!order_only_deps.empty()) {
     out_ << " ||";
     path_output_.WriteFiles(out_, order_only_deps);

diff --git a/src/gn/ninja_create_bundle_target_writer.h b/src/gn/ninja_create_bundle_target_writer.h
index 6b5d6cc..7d603a0 100644
--- a/src/gn/ninja_create_bundle_target_writer.h
+++ b/src/gn/ninja_create_bundle_target_writer.h

@@ -30,7 +30,8 @@
   // Writes the steps to copy files into the bundle.
   //
   // The list of newly created files will be added to |output_files|.
-  void WriteCopyBundleDataSteps(const std::vector<OutputFile>& order_only_deps,
+  void WriteCopyBundleDataSteps(const std::vector<OutputFile>& implicit_deps,
+                                const std::vector<OutputFile>& order_only_deps,
                                 std::vector<OutputFile>* output_files);
 
   // Writes the step to copy files BundleFileRule into the bundle.
@@ -38,6 +39,7 @@
   // The list of newly created files will be added to |output_files|.
   void WriteCopyBundleFileRuleSteps(
       const BundleFileRule& file_rule,
+      const std::vector<OutputFile>& implicit_deps,
       const std::vector<OutputFile>& order_only_deps,
       std::vector<OutputFile>* output_files);
 
@@ -45,6 +47,7 @@
   //
   // The list of newly created files will be added to |output_files|.
   void WriteCompileAssetsCatalogStep(
+      const std::vector<OutputFile>& implicit_deps,
       const std::vector<OutputFile>& order_only_deps,
       std::vector<OutputFile>* output_files);
 
@@ -59,12 +62,14 @@
   // post-processing may depends on the full bundle structure, this step will
   // depends on all files generated via other rules.
   void WritePostProcessingStep(const std::string& post_processing_rule_name,
+                               const std::vector<OutputFile>& implicit_deps,
                                const std::vector<OutputFile>& order_only_deps,
                                std::vector<OutputFile>* output_files);
 
   // Writes the stamp file or phony target for the post-processing input
   // dependencies.
   OutputFile WritePostProcessingInputDepsStampOrPhony(
+      const std::vector<OutputFile>& implicit_deps,
       const std::vector<OutputFile>& order_only_deps,
       std::vector<OutputFile>* output_files);
 

diff --git a/src/gn/ninja_create_bundle_target_writer_unittest.cc b/src/gn/ninja_create_bundle_target_writer_unittest.cc
index 3dfd6a3..7aa1313 100644
--- a/src/gn/ninja_create_bundle_target_writer_unittest.cc
+++ b/src/gn/ninja_create_bundle_target_writer_unittest.cc

@@ -75,16 +75,15 @@
   writer.Run();
 
   const char expected[] =
-      "build phony/baz/bar.inputdeps: phony phony/foo/bar "
+      "build phony/baz/bar.inputdeps: phony || phony/foo/bar "
       "phony/foo/data\n"
       "build bar.bundle/Contents/Resources/input1.txt: copy_bundle_data "
-      "../../foo/input1.txt || phony/baz/bar.inputdeps\n"
+      "../../foo/input1.txt | phony/baz/bar.inputdeps\n"
       "build bar.bundle/Contents/Resources/input2.txt: copy_bundle_data "
-      "../../foo/input2.txt || phony/baz/bar.inputdeps\n"
+      "../../foo/input2.txt | phony/baz/bar.inputdeps\n"
       "build phony/baz/bar: phony "
       "bar.bundle/Contents/Resources/input1.txt "
-      "bar.bundle/Contents/Resources/input2.txt"
-      " || phony/baz/bar.inputdeps\n"
+      "bar.bundle/Contents/Resources/input2.txt\n"
       "build bar.bundle: phony phony/baz/bar\n";
   std::string out_str = out.str();
   EXPECT_EQ(expected, out_str);
@@ -124,16 +123,15 @@
   writer.Run();
 
   const char expected[] =
-      "build phony/baz/bar.inputdeps: phony phony/foo/bar "
+      "build phony/baz/bar.inputdeps: phony || phony/foo/bar "
       "phony/foo/data\n"
       "build gen/bar.bundle/Contents/Resources/input1.txt: copy_bundle_data "
-      "../../foo/input1.txt || phony/baz/bar.inputdeps\n"
+      "../../foo/input1.txt | phony/baz/bar.inputdeps\n"
       "build gen/bar.bundle/Contents/Resources/input2.txt: copy_bundle_data "
-      "../../foo/input2.txt || phony/baz/bar.inputdeps\n"
+      "../../foo/input2.txt | phony/baz/bar.inputdeps\n"
       "build phony/baz/bar: phony "
       "gen/bar.bundle/Contents/Resources/input1.txt "
-      "gen/bar.bundle/Contents/Resources/input2.txt || "
-      "phony/baz/bar.inputdeps\n"
+      "gen/bar.bundle/Contents/Resources/input2.txt\n"
       "build gen/bar.bundle: phony phony/baz/bar\n";
   std::string out_str = out.str();
   EXPECT_EQ(expected, out_str);
@@ -229,16 +227,14 @@
   writer.Run();
 
   const char expected[] =
-      "build phony/baz/bar.inputdeps: phony phony/foo/bar "
+      "build phony/baz/bar.inputdeps: phony || phony/foo/bar "
       "phony/foo/data\n"
       "build bar.bundle/Contents/Resources/Assets.car: compile_xcassets "
-      "../../foo/Foo.xcassets | phony/foo/data || "
-      "phony/baz/bar.inputdeps\n"
+      "../../foo/Foo.xcassets | phony/foo/data phony/baz/bar.inputdeps\n"
       "  product_type = com.apple.product-type\n"
       "  xcasset_compiler_flags = --app-icon foo\n"
       "build phony/baz/bar: phony "
-      "bar.bundle/Contents/Resources/Assets.car || "
-      "phony/baz/bar.inputdeps\n"
+      "bar.bundle/Contents/Resources/Assets.car\n"
       "build bar.bundle: phony phony/baz/bar\n";
   std::string out_str = out.str();
   EXPECT_EQ(expected, out_str);
@@ -393,22 +389,22 @@
   writer.Run();
 
   const char expected[] =
-      "build phony/baz/bar.inputdeps: phony phony/biz/assets "
+      "build phony/baz/bar.inputdeps: phony || phony/biz/assets "
       "phony/foo/assets phony/foo/bar phony/foo/data "
       "phony/qux/info_plist phony/quz/assets\n"
       "build bar.bundle/Contents/Info.plist: copy_bundle_data "
-      "../../qux/qux-Info.plist || phony/baz/bar.inputdeps\n"
+      "../../qux/qux-Info.plist | phony/baz/bar.inputdeps\n"
       "build bar.bundle/Contents/Resources/input1.txt: copy_bundle_data "
-      "../../foo/input1.txt || phony/baz/bar.inputdeps\n"
+      "../../foo/input1.txt | phony/baz/bar.inputdeps\n"
       "build bar.bundle/Contents/Resources/input2.txt: copy_bundle_data "
-      "../../foo/input2.txt || phony/baz/bar.inputdeps\n"
+      "../../foo/input2.txt | phony/baz/bar.inputdeps\n"
       "build phony/baz/bar.xcassets.inputdeps: phony "
       "phony/foo/assets "
       "phony/quz/assets phony/biz/assets\n"
       "build bar.bundle/Contents/Resources/Assets.car | "
       "baz/bar/bar_partial_info.plist: compile_xcassets "
       "../../foo/Foo.xcassets ../../quz/Quz.xcassets "
-      "../../biz/Biz.xcassets | phony/baz/bar.xcassets.inputdeps || "
+      "../../biz/Biz.xcassets | phony/baz/bar.xcassets.inputdeps "
       "phony/baz/bar.inputdeps\n"
       "  product_type = com.apple.product-type\n"
       "  partial_info_plist = baz/bar/bar_partial_info.plist\n"
@@ -417,7 +413,7 @@
       "bar.bundle/Contents/Resources/input1.txt "
       "bar.bundle/Contents/Resources/input2.txt "
       "bar.bundle/Contents/Resources/Assets.car "
-      "baz/bar/bar_partial_info.plist || phony/baz/bar.inputdeps\n"
+      "baz/bar/bar_partial_info.plist\n"
       "build bar.bundle: phony phony/baz/bar\n";
   std::string out_str = out.str();
   EXPECT_EQ(expected, out_str);
@@ -475,7 +471,7 @@
   writer.Run();
 
   const char expected[] =
-      "build phony/baz/bar.inputdeps: phony ./quz phony/foo/bar "
+      "build phony/baz/bar.inputdeps: phony || ./quz phony/foo/bar "
       "phony/foo/data\n"
       "rule __baz_bar___toolchain_default__post_processing_rule\n"
       "  command =  ../../build/codesign.py -b=quz bar.bundle\n"
@@ -483,21 +479,21 @@
       "  restat = 1\n"
       "\n"
       "build bar.bundle/Contents/Resources/input1.txt: copy_bundle_data "
-      "../../foo/input1.txt || phony/baz/bar.inputdeps\n"
+      "../../foo/input1.txt | phony/baz/bar.inputdeps\n"
       "build bar.bundle/Contents/Resources/input2.txt: copy_bundle_data "
-      "../../foo/input2.txt || phony/baz/bar.inputdeps\n"
+      "../../foo/input2.txt | phony/baz/bar.inputdeps\n"
       "build phony/baz/bar.postprocessing.inputdeps: phony "
       "../../build/codesign.py "
       "quz "
       "bar.bundle/Contents/Resources/input1.txt "
-      "bar.bundle/Contents/Resources/input2.txt || "
+      "bar.bundle/Contents/Resources/input2.txt | "
       "phony/baz/bar.inputdeps\n"
       "build bar.bundle/Contents/quz bar.bundle/_CodeSignature/CodeResources: "
       "__baz_bar___toolchain_default__post_processing_rule "
       "| phony/baz/bar.postprocessing.inputdeps\n"
       "build phony/baz/bar: phony "
       "bar.bundle/Contents/quz "
-      "bar.bundle/_CodeSignature/CodeResources || phony/baz/bar.inputdeps\n"
+      "bar.bundle/_CodeSignature/CodeResources\n"
       "build bar.bundle: phony phony/baz/bar\n";
   std::string out_str = out.str();
   EXPECT_EQ(expected, out_str);
@@ -558,7 +554,7 @@
   writer.Run();
 
   const char expected[] =
-      "build toolchain/phony/baz/bar.inputdeps: phony ./quz "
+      "build toolchain/phony/baz/bar.inputdeps: phony || ./quz "
       "toolchain/phony/foo/bar "
       "toolchain/phony/foo/data\n"
       "rule __baz_bar___toolchain_default__post_processing_rule\n"
@@ -568,23 +564,22 @@
       "\n"
       "build bar.bundle/Contents/Resources/input1.txt: "
       "toolchain_copy_bundle_data "
-      "../../foo/input1.txt || toolchain/phony/baz/bar.inputdeps\n"
+      "../../foo/input1.txt | toolchain/phony/baz/bar.inputdeps\n"
       "build bar.bundle/Contents/Resources/input2.txt: "
       "toolchain_copy_bundle_data "
-      "../../foo/input2.txt || toolchain/phony/baz/bar.inputdeps\n"
+      "../../foo/input2.txt | toolchain/phony/baz/bar.inputdeps\n"
       "build toolchain/phony/baz/bar.postprocessing.inputdeps: phony "
       "../../build/codesign.py "
       "quz "
       "bar.bundle/Contents/Resources/input1.txt "
-      "bar.bundle/Contents/Resources/input2.txt || "
+      "bar.bundle/Contents/Resources/input2.txt | "
       "toolchain/phony/baz/bar.inputdeps\n"
       "build bar.bundle/Contents/quz bar.bundle/_CodeSignature/CodeResources: "
       "__baz_bar___toolchain_default__post_processing_rule "
       "| toolchain/phony/baz/bar.postprocessing.inputdeps\n"
       "build toolchain/phony/baz/bar: phony "
       "bar.bundle/Contents/quz "
-      "bar.bundle/_CodeSignature/CodeResources || "
-      "toolchain/phony/baz/bar.inputdeps\n"
+      "bar.bundle/_CodeSignature/CodeResources\n"
       "build bar.bundle: phony toolchain/phony/baz/bar\n";
   std::string out_str = out.str();
   EXPECT_EQ(expected, out_str);

diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc
index e9b44e8..b77157a 100644
--- a/src/gn/ninja_rust_binary_target_writer.cc
+++ b/src/gn/ninja_rust_binary_target_writer.cc

@@ -125,8 +125,9 @@
   // Ninja to make sure the inputs are up to date before compiling this source,
   // but changes in the inputs deps won't cause the file to be recompiled. See
   // the comment on NinjaCBinaryTargetWriter::Run for more detailed explanation.
-  std::vector<OutputFile> order_only_deps = WriteInputDepsStampOrPhonyAndGetDep(
+  NinjaTargetWriter::InputDeps stamp_deps = WriteInputDepsStampOrPhonyAndGetDep(
       std::vector<const Target*>(), num_output_uses);
+  std::vector<OutputFile> order_only_deps = stamp_deps.order_only;
   std::copy(input_deps.begin(), input_deps.end(),
             std::back_inserter(order_only_deps));
 
@@ -135,6 +136,7 @@
   // -Ldependency. Also assemble a list of extra (i.e. implicit) deps
   // for ninja dependency tracking.
   UniqueVector<OutputFile> implicit_deps;
+  implicit_deps.Append(stamp_deps.implicit.begin(), stamp_deps.implicit.end());
   AppendSourcesAndInputsToImplicitDeps(&implicit_deps);
   implicit_deps.Append(classified_deps.extra_object_files.begin(),
                        classified_deps.extra_object_files.end());

diff --git a/src/gn/ninja_target_writer.cc b/src/gn/ninja_target_writer.cc
index 9b45628..affc7d6 100644
--- a/src/gn/ninja_target_writer.cc
+++ b/src/gn/ninja_target_writer.cc

@@ -453,7 +453,8 @@
   }
 }
 
-std::vector<OutputFile> NinjaTargetWriter::WriteInputDepsStampOrPhonyAndGetDep(
+NinjaTargetWriter::InputDeps
+NinjaTargetWriter::WriteInputDepsStampOrPhonyAndGetDep(
     const std::vector<const Target*>& additional_hard_deps,
     size_t num_output_uses) const {
   CHECK(target_->toolchain()) << "Toolchain not set on target "
@@ -519,55 +520,62 @@
   // toolchains often have more than one dependency, we could consider writing
   // a toolchain-specific phony target and only include the phony here.
   // Note that these are usually empty/small.
+  std::vector<const Target*> toolchain_deps_targets;
   const LabelTargetVector& toolchain_deps = target_->toolchain()->deps();
   for (const auto& toolchain_dep : toolchain_deps) {
     // This could theoretically duplicate dependencies already in the list,
     // but it shouldn't happen in practice, is inconvenient to check for,
     // and only results in harmless redundant dependencies listed.
-    input_deps_targets.push_back(toolchain_dep.ptr);
+    toolchain_deps_targets.push_back(toolchain_dep.ptr);
   }
 
   // ---------
   // Write the outputs.
 
-  if (input_deps_sources.size() + input_deps_targets.size() == 0)
-    return std::vector<OutputFile>();  // No input dependencies.
+  if (input_deps_sources.empty() && input_deps_targets.empty() &&
+      toolchain_deps_targets.empty())
+    return InputDeps{};  // No input dependencies.
 
-  // If we're only generating one input dependency, return it directly instead
-  // of writing a phony target for it.
-  if (input_deps_sources.size() == 1 && input_deps_targets.size() == 0)
-    return std::vector<OutputFile>{
-        OutputFile(settings_->build_settings(), *input_deps_sources[0])};
-  if (input_deps_sources.size() == 0 && input_deps_targets.size() == 1) {
-    const auto& dep = *input_deps_targets[0];
-    if (!dep.has_dependency_output())
-      return std::vector<OutputFile>();
-    return std::vector<OutputFile>{dep.dependency_output()};
-  }
-
-  std::vector<OutputFile> outs;
+  InputDeps deps;
   // File input deps.
   for (const SourceFile* source : input_deps_sources)
-    outs.push_back(OutputFile(settings_->build_settings(), *source));
+    deps.order_only.push_back(OutputFile(settings_->build_settings(), *source));
   // Target input deps. Sort by label so the output is deterministic (otherwise
   // some of the targets will have gone through std::sets which will have
   // sorted them by pointer).
-  std::sort(
-      input_deps_targets.begin(), input_deps_targets.end(),
-      [](const Target* a, const Target* b) { return a->label() < b->label(); });
-  for (auto* dep : input_deps_targets) {
-    if (dep->has_dependency_output())
-      outs.push_back(dep->dependency_output());
-  }
+  auto add_target_deps = [](std::vector<OutputFile>& deps,
+                            std::vector<const Target*>& targets) {
+    std::sort(targets.begin(), targets.end(),
+              [](const Target* a, const Target* b) {
+                return a->label() < b->label();
+              });
+    for (auto* dep : targets) {
+      if (dep->has_dependency_output())
+        deps.push_back(dep->dependency_output());
+    }
+  };
+  add_target_deps(deps.order_only, input_deps_targets);
+  add_target_deps(deps.implicit, toolchain_deps_targets);
 
-  // If there are multiple inputs, but the phony target would be referenced only
-  // once, don't write it but depend on the inputs directly.
-  if (num_output_uses == 1u)
-    return outs;
+  // If we're only generating one input dependency, or if there are no
+  // dependencies, return it directly instead of writing a phony target for it.
+  // Also, if there are multiple inputs, but the phony target would be
+  // referenced only once, don't write it but depend on the inputs directly.
+  if (deps.implicit.size() + deps.order_only.size() <= 1 ||
+      num_output_uses == 1u)
+    return deps;
 
-  // If outs is empty, we don't need stamp or phony target for this.
-  if (outs.empty()) {
-    return outs;
+  // Action targets are special because all of their dependencies are implicit
+  // dependencies. This is because prior to
+  // https://gn-review.googlesource.com/c/gn/+/22000 action targets had an
+  // implicit dependency on inputdeps whereas other target types had an
+  // order-only dependency on inputdeps (which at the time only had implicit
+  // dependencies), and scripts may be depending on that.
+  if (target_->output_type() == Target::ACTION ||
+      target_->output_type() == Target::ACTION_FOREACH) {
+    deps.implicit.insert(deps.implicit.end(), deps.order_only.begin(),
+                         deps.order_only.end());
+    deps.order_only.clear();
   }
 
   OutputFile input_stamp_or_phony;
@@ -596,9 +604,16 @@
   out_ << "build ";
   path_output_.WriteFile(out_, input_stamp_or_phony);
   out_ << ": " << tool;
-  path_output_.WriteFiles(out_, outs);
+  path_output_.WriteFiles(out_, deps.implicit);
+  if (!deps.order_only.empty()) {
+    out_ << " ||";
+    path_output_.WriteFiles(out_, deps.order_only);
+  }
   out_ << "\n";
-  return std::vector<OutputFile>{input_stamp_or_phony};
+
+  InputDeps result;
+  result.implicit.push_back(input_stamp_or_phony);
+  return result;
 }
 
 void NinjaTargetWriter::WriteStampOrPhonyForTarget(

diff --git a/src/gn/ninja_target_writer.h b/src/gn/ninja_target_writer.h
index 0936033..2da7e76 100644
--- a/src/gn/ninja_target_writer.h
+++ b/src/gn/ninja_target_writer.h

@@ -82,14 +82,19 @@
                              bool indent,
                              bool always_write);
 
+  struct InputDeps {
+    std::vector<OutputFile> implicit;
+    std::vector<OutputFile> order_only;
+  };
+
   // Writes to the output stream a phony rule for input dependencies, and
   // returns the file to be appended to source rules that encodes the
-  // order-only dependencies for the current target.
+  // dependencies for the current target.
   // If num_output_uses is small, this might return all input dependencies
   // directly, without writing a phony rule.
   // If there are no implicit dependencies and no additional target dependencies
-  // are passed in, this returns an empty vector.
-  std::vector<OutputFile> WriteInputDepsStampOrPhonyAndGetDep(
+  // are passed in, this returns an empty struct.
+  InputDeps WriteInputDepsStampOrPhonyAndGetDep(
       const std::vector<const Target*>& additional_hard_deps,
       size_t num_output_uses) const;
 

diff --git a/src/gn/ninja_target_writer_unittest.cc b/src/gn/ninja_target_writer_unittest.cc
index 317ce2a..cd4a5e4 100644
--- a/src/gn/ninja_target_writer_unittest.cc
+++ b/src/gn/ninja_target_writer_unittest.cc

@@ -22,7 +22,7 @@
   void Run() override {}
 
   // Make this public so the test can call it.
-  std::vector<OutputFile> WriteInputDepsStampOrPhonyAndGetDep(
+  NinjaTargetWriter::InputDeps WriteInputDepsStampOrPhonyAndGetDep(
       const std::vector<const Target*>& additional_hard_deps,
       size_t num_stamp_uses) {
     return NinjaTargetWriter::WriteInputDepsStampOrPhonyAndGetDep(
@@ -119,13 +119,14 @@
   {
     std::ostringstream stream;
     TestingNinjaTargetWriter writer(&base_target, setup.toolchain(), stream);
-    std::vector<OutputFile> dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
+    auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
         std::vector<const Target*>(), 10u);
 
     // Since there is only one dependency, it should just be returned and
     // nothing written to the stream.
-    ASSERT_EQ(1u, dep.size());
-    EXPECT_EQ("../../foo/script.py", dep[0].value());
+    ASSERT_EQ(0u, dep.implicit.size());
+    ASSERT_EQ(1u, dep.order_only.size());
+    EXPECT_EQ("../../foo/script.py", dep.order_only[0].value());
     EXPECT_EQ("", stream.str());
   }
 
@@ -133,13 +134,14 @@
   {
     std::ostringstream stream;
     TestingNinjaTargetWriter writer(&target, setup.toolchain(), stream);
-    std::vector<OutputFile> dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
+    auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
         std::vector<const Target*>(), 10u);
 
     // Since there is only one dependency, a stamp file will be returned
     // directly without writing any additional rules.
-    ASSERT_EQ(1u, dep.size());
-    EXPECT_EQ("phony/foo/base", dep[0].value());
+    ASSERT_EQ(0u, dep.implicit.size());
+    ASSERT_EQ(1u, dep.order_only.size());
+    EXPECT_EQ("phony/foo/base", dep.order_only[0].value());
   }
 
   {
@@ -164,11 +166,12 @@
   {
     std::ostringstream stream;
     TestingNinjaTargetWriter writer(&action, setup.toolchain(), stream);
-    std::vector<OutputFile> dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
+    auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
         std::vector<const Target*>(), 10u);
 
-    ASSERT_EQ(1u, dep.size());
-    EXPECT_EQ("phony/foo/action.inputdeps", dep[0].value());
+    ASSERT_EQ(1u, dep.implicit.size());
+    ASSERT_EQ(0u, dep.order_only.size());
+    EXPECT_EQ("phony/foo/action.inputdeps", dep.implicit[0].value());
     EXPECT_EQ(
         "build phony/foo/action.inputdeps: phony ../../foo/script.py "
         "../../foo/action_source.txt ./target\n",
@@ -216,13 +219,14 @@
   {
     std::ostringstream stream;
     TestingNinjaTargetWriter writer(&base_target, setup.toolchain(), stream);
-    std::vector<OutputFile> dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
+    auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
         std::vector<const Target*>(), 10u);
 
     // Since there is only one dependency, it should just be returned and
     // nothing written to the stream.
-    ASSERT_EQ(1u, dep.size());
-    EXPECT_EQ("../../foo/script.py", dep[0].value());
+    ASSERT_EQ(0u, dep.implicit.size());
+    ASSERT_EQ(1u, dep.order_only.size());
+    EXPECT_EQ("../../foo/script.py", dep.order_only[0].value());
     EXPECT_EQ("", stream.str());
   }
 
@@ -230,13 +234,14 @@
   {
     std::ostringstream stream;
     TestingNinjaTargetWriter writer(&target, setup.toolchain(), stream);
-    std::vector<OutputFile> dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
+    auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
         std::vector<const Target*>(), 10u);
 
     // Since there is only one dependency, a stamp file will be returned
     // directly without writing any additional rules.
-    ASSERT_EQ(1u, dep.size());
-    EXPECT_EQ("obj/foo/base.stamp", dep[0].value());
+    ASSERT_EQ(0u, dep.implicit.size());
+    ASSERT_EQ(1u, dep.order_only.size());
+    EXPECT_EQ("obj/foo/base.stamp", dep.order_only[0].value());
   }
 
   {
@@ -261,11 +266,12 @@
   {
     std::ostringstream stream;
     TestingNinjaTargetWriter writer(&action, setup.toolchain(), stream);
-    std::vector<OutputFile> dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
+    auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
         std::vector<const Target*>(), 10u);
 
-    ASSERT_EQ(1u, dep.size());
-    EXPECT_EQ("obj/foo/action.inputdeps.stamp", dep[0].value());
+    ASSERT_EQ(1u, dep.implicit.size());
+    ASSERT_EQ(0u, dep.order_only.size());
+    EXPECT_EQ("obj/foo/action.inputdeps.stamp", dep.implicit[0].value());
     EXPECT_EQ(
         "build obj/foo/action.inputdeps.stamp: stamp ../../foo/script.py "
         "../../foo/action_source.txt ./target\n",
@@ -297,16 +303,60 @@
 
   std::ostringstream stream;
   TestingNinjaTargetWriter writer(&target, setup.toolchain(), stream);
-  std::vector<OutputFile> dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
+  auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
       std::vector<const Target*>(), 10u);
 
   // Since there is more than one dependency, a stamp file will be returned
   // and the rule for the stamp file will be written to the stream.
-  ASSERT_EQ(1u, dep.size());
-  EXPECT_EQ("phony/foo/setup", dep[0].value());
+  ASSERT_EQ(1u, dep.implicit.size());
+  ASSERT_EQ(0u, dep.order_only.size());
+  EXPECT_EQ("phony/foo/setup", dep.implicit[0].value());
   EXPECT_EQ("", stream.str());
 }
 
+// Tests WriteInputDepsStampOrPhonyAndGetDep when a toolchain dependency and
+// another dependency are present.
+TEST(NinjaTargetWriter,
+     WriteInputDepsStampOrPhonyAndGetDepWithToolchainAndAnotherDep) {
+  TestWithScope setup;
+  Err err;
+
+  Target toolchain_dep_target(setup.settings(),
+                              Label(SourceDir("//foo/"), "setup"));
+  toolchain_dep_target.set_output_type(Target::ACTION);
+  toolchain_dep_target.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(toolchain_dep_target.OnResolved(&err));
+  setup.toolchain()->deps().push_back(LabelTargetPair(&toolchain_dep_target));
+
+  Target base_target(setup.settings(), Label(SourceDir("//foo/"), "base"));
+  base_target.set_output_type(Target::ACTION);
+  base_target.visibility().SetPublic();
+  base_target.SetToolchain(setup.toolchain());
+  base_target.action_values().set_script(SourceFile("//foo/script.py"));
+  ASSERT_TRUE(base_target.OnResolved(&err));
+
+  Target target(setup.settings(), Label(SourceDir("//foo/"), "target"));
+  target.set_output_type(Target::EXECUTABLE);
+  target.SetToolchain(setup.toolchain());
+  target.public_deps().push_back(LabelTargetPair(&base_target));
+  ASSERT_TRUE(target.OnResolved(&err));
+
+  std::ostringstream stream;
+  TestingNinjaTargetWriter writer(&target, setup.toolchain(), stream);
+  auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep(
+      std::vector<const Target*>(), 10u);
+
+  // Since there is a toolchain dependency, it should be returned as an implicit
+  // dependency.
+  ASSERT_EQ(1u, dep.implicit.size());
+  EXPECT_EQ("phony/foo/target.inputdeps", dep.implicit[0].value());
+  ASSERT_EQ(0u, dep.order_only.size());
+  EXPECT_EQ(
+      "build phony/foo/target.inputdeps: phony phony/foo/setup || "
+      "phony/foo/base\n",
+      stream.str());
+}
+
 // Tests that validation dependencies are written to the generated Ninja file
 // with the "|@" syntax for a generic target.
 TEST(NinjaTargetWriter, WriteValidations) {