Fix dependencies rules for create_bundle and bundle_data ninja steps.

Record the "bundle_data" target corresponding to a BundleFileRule object
so that the dependencies can be correctly computed when generating the
ninja steps for the "create_bundle" target.

Change the "stamp" step of "bundle_data" target in the generated ninja
to depends on the target dependencies and source files.

Change the "copy_bundle_data" steps of "create_bundle" target in the
generated ninja to depends on the "bundle_data" target that defined
the corresponding source files.

Change the "compile_xcassets" step of "create_bundle" target in the
generated ninja to depends on all the "bundle_data" targets that defined
the corresponding source files.

Change the rule used to determine whether a "bundle_data" source file
is part of an assets catalog (some file where incorrectly omitted causing
them to be incorrectly copied to the final bundle).

All those changes will allow the "copy_bundle_data" and "compile_xcassets"
steps to only depends on the targets that generate them (if any) and will
reduce the number of file copied into the final bundle during incremental
builds.

Fix unit tests with new expectations.

BUG=623501

Review-Url: https://codereview.chromium.org/2105613003
Cr-Commit-Position: refs/heads/master@{#403304}

tools/gn/BUILD.gn

@@ -297,6 +297,7 @@ test("gn_unittests") {
     "ninja_action_target_writer_unittest.cc",
     "ninja_binary_target_writer_unittest.cc",
     "ninja_build_writer_unittest.cc",
+    "ninja_bundle_data_target_writer_unittest.cc",
     "ninja_copy_target_writer_unittest.cc",
     "ninja_create_bundle_target_writer_unittest.cc",
     "ninja_group_target_writer_unittest.cc",

tools/gn/bundle_data.cc

@@ -14,33 +14,38 @@
 namespace {
 
 // Return directory of |path| without the trailing directory separator.
-base::StringPiece FindDirNoTrailingSeparator(const base::StringPiece& path) {
+base::StringPiece FindDirNoTrailingSeparator(base::StringPiece path) {
   base::StringPiece::size_type pos = path.find_last_of("/\\");
   if (pos == base::StringPiece::npos)
     return base::StringPiece();
   return base::StringPiece(path.data(), pos);
 }
 
-}  // namespace
-
-bool IsSourceFileFromAssetCatalog(const SourceFile& source,
-                                  SourceFile* asset_catalog) {
-  // Check that the file matches the following pattern:
+bool IsSourceFileFromAssetsCatalog(base::StringPiece source,
+                                   SourceFile* asset_catalog) {
+  // Check whether |source| matches one of the following pattern:
+  //    .*\.xcassets/Contents.json
+  //    .*\.xcassets/[^/]*\.appiconset/[^/]*
   //    .*\.xcassets/[^/]*\.imageset/[^/]*
-  base::StringPiece dir;
-  dir = FindDirNoTrailingSeparator(source.value());
-  if (!dir.ends_with(".imageset"))
-    return false;
-  dir = FindDirNoTrailingSeparator(dir);
-  if (!dir.ends_with(".xcassets"))
-    return false;
-  if (asset_catalog) {
+  //    .*\.xcassets/[^/]*\.launchimage/[^/]*
+  bool is_file_from_asset_catalog = false;
+  base::StringPiece dir = FindDirNoTrailingSeparator(source);
+  if (source.ends_with("/Contents.json") && dir.ends_with(".xcassets")) {
+    is_file_from_asset_catalog = true;
+  } else if (dir.ends_with(".appiconset") || dir.ends_with(".imageset") ||
+             dir.ends_with(".launchimage")) {
+    dir = FindDirNoTrailingSeparator(dir);
+    is_file_from_asset_catalog = dir.ends_with(".xcassets");
+  }
+  if (is_file_from_asset_catalog && asset_catalog) {
     std::string asset_catalog_path = dir.as_string();
     *asset_catalog = SourceFile(SourceFile::SWAP_IN, &asset_catalog_path);
   }
-  return true;
+  return is_file_from_asset_catalog;
 }
 
+}  // namespace
+
 BundleData::BundleData() {}
 
 BundleData::~BundleData() {}
@@ -51,16 +56,21 @@ void BundleData::AddBundleData(const Target* target) {
 }
 
 void BundleData::OnTargetResolved(Target* owning_target) {
-  // Only initialize file_rules_ and asset_catalog_sources for "create_bundle"
+  // Only initialize file_rules_ and assets_catalog_sources for "create_bundle"
   // target (properties are only used by those targets).
   if (owning_target->output_type() != Target::CREATE_BUNDLE)
     return;
 
+  UniqueVector<const Target*> assets_catalog_deps;
+  UniqueVector<SourceFile> assets_catalog_sources;
+
   for (const Target* target : bundle_deps_) {
     SourceFiles file_rule_sources;
     for (const SourceFile& source_file : target->sources()) {
-      if (IsSourceFileFromAssetCatalog(source_file, nullptr)) {
-        asset_catalog_sources_.push_back(source_file);
+      SourceFile assets_catalog;
+      if (IsSourceFileFromAssetsCatalog(source_file.value(), &assets_catalog)) {
+        assets_catalog_sources.push_back(assets_catalog);
+        assets_catalog_deps.push_back(target);
       } else {
         file_rule_sources.push_back(source_file);
       }
@@ -68,11 +78,19 @@ void BundleData::OnTargetResolved(Target* owning_target) {
 
     if (!file_rule_sources.empty()) {
       DCHECK_EQ(target->action_values().outputs().list().size(), 1u);
-      file_rules_.push_back(BundleFileRule(
-          file_rule_sources, target->action_values().outputs().list()[0]));
+      file_rules_.push_back(
+          BundleFileRule(target, file_rule_sources,
+                         target->action_values().outputs().list()[0]));
     }
   }
 
+  assets_catalog_deps_.insert(assets_catalog_deps_.end(),
+                              assets_catalog_deps.begin(),
+                              assets_catalog_deps.end());
+  assets_catalog_sources_.insert(assets_catalog_sources_.end(),
+                                 assets_catalog_sources.begin(),
+                                 assets_catalog_sources.end());
+
   GetSourceFiles(&owning_target->sources());
 }
 
@@ -81,8 +99,8 @@ void BundleData::GetSourceFiles(SourceFiles* sources) const {
     sources->insert(sources->end(), file_rule.sources().begin(),
                     file_rule.sources().end());
   }
-  sources->insert(sources->end(), asset_catalog_sources_.begin(),
-                  asset_catalog_sources_.end());
+  sources->insert(sources->end(), assets_catalog_sources_.begin(),
+                  assets_catalog_sources_.end());
   if (!code_signing_script_.is_null()) {
     sources->insert(sources->end(), code_signing_sources_.begin(),
                     code_signing_sources_.end());
@@ -107,7 +125,7 @@ void BundleData::GetOutputsAsSourceFiles(
     }
   }
 
-  if (!asset_catalog_sources_.empty())
+  if (!assets_catalog_sources_.empty())
     outputs_as_source->push_back(GetCompiledAssetCatalogPath());
 
   if (!code_signing_script_.is_null()) {
@@ -124,7 +142,7 @@ void BundleData::GetOutputsAsSourceFiles(
 }
 
 SourceFile BundleData::GetCompiledAssetCatalogPath() const {
-  DCHECK(!asset_catalog_sources_.empty());
+  DCHECK(!assets_catalog_sources_.empty());
   std::string assets_car_path = resources_dir_.value() + "/Assets.car";
   return SourceFile(SourceFile::SWAP_IN, &assets_car_path);
 }

tools/gn/bundle_data.h

@@ -19,19 +19,6 @@ class OutputFile;
 class Settings;
 class Target;
 
-// Returns true if |source| correspond to the path of a file in an asset
-// catalog. If defined |asset_catalog| is set to its path.
-//
-// An asset catalog is an OS X bundle with the ".xcassets" extension. It
-// contains one directory per assets each of them with the ".imageset"
-// extension.
-//
-// All asset catalogs are compiled by Xcode into single Assets.car file as
-// part of the creation of an application or framework bundle. BundleData
-// emulates this with the "compile_xcassets" tool.
-bool IsSourceFileFromAssetCatalog(const SourceFile& source,
-                                  SourceFile* asset_catalog);
-
 // BundleData holds the information required by "create_bundle" target.
 class BundleData {
  public:
@@ -64,7 +51,7 @@ class BundleData {
       SourceFiles* outputs_as_source) const;
 
   // Returns the path to the compiled asset catalog. Only valid if
-  // asset_catalog_sources() is not empty.
+  // assets_catalog_sources() is not empty.
   SourceFile GetCompiledAssetCatalogPath() const;
 
   // Returns the path to the top-level directory of the bundle. This is
@@ -83,9 +70,14 @@ class BundleData {
   SourceDir GetBundleRootDirOutputAsDir(const Settings* settings) const;
 
   // Returns the list of inputs for the compilation of the asset catalog.
-  SourceFiles& asset_catalog_sources() { return asset_catalog_sources_; }
-  const SourceFiles& asset_catalog_sources() const {
-    return asset_catalog_sources_;
+  SourceFiles& assets_catalog_sources() { return assets_catalog_sources_; }
+  const SourceFiles& assets_catalog_sources() const {
+    return assets_catalog_sources_;
+  }
+
+  // Returns the list of dependencies for the compilation of the asset catalog.
+  std::vector<const Target*> assets_catalog_deps() const {
+    return assets_catalog_deps_;
   }
 
   BundleFileRules& file_rules() { return file_rules_; }
@@ -132,7 +124,8 @@ class BundleData {
   const UniqueTargets& bundle_deps() const { return bundle_deps_; }
 
  private:
-  SourceFiles asset_catalog_sources_;
+  SourceFiles assets_catalog_sources_;
+  std::vector<const Target*> assets_catalog_deps_;
   BundleFileRules file_rules_;
   UniqueTargets bundle_deps_;
 

tools/gn/bundle_file_rule.cc

@@ -10,9 +10,13 @@
 #include "tools/gn/substitution_writer.h"
 #include "tools/gn/target.h"
 
-BundleFileRule::BundleFileRule(const std::vector<SourceFile> sources,
+BundleFileRule::BundleFileRule(const Target* bundle_data_target,
+                               const std::vector<SourceFile> sources,
                                const SubstitutionPattern& pattern)
-    : sources_(sources), pattern_(pattern) {}
+    : target_(bundle_data_target), sources_(sources), pattern_(pattern) {
+  // target_ may be null during testing.
+  DCHECK(!target_ || target_->output_type() == Target::BUNDLE_DATA);
+}
 
 BundleFileRule::BundleFileRule(const BundleFileRule& other) = default;
 

tools/gn/bundle_file_rule.h

@@ -13,12 +13,14 @@
 class BundleData;
 class Settings;
 class SourceFile;
+class Target;
 class OutputFile;
 
 // BundleFileRule contains the information found in a "bundle_data" target.
 class BundleFileRule {
  public:
-  BundleFileRule(const std::vector<SourceFile> sources,
+  BundleFileRule(const Target* bundle_data_target,
+                 const std::vector<SourceFile> sources,
                  const SubstitutionPattern& pattern);
   BundleFileRule(const BundleFileRule& other);
   ~BundleFileRule();
@@ -33,10 +35,15 @@ class BundleFileRule {
       const BundleData& bundle_data,
       const SourceFile& source_file) const;
 
+  // Returns the associated target (of type Target::BUNDLE_DATA). May be
+  // null during testing.
+  const Target* target() const { return target_; }
+
   // Returns the list of SourceFiles.
   const std::vector<SourceFile>& sources() const { return sources_; }
 
  private:
+  const Target* target_;
   std::vector<SourceFile> sources_;
   SubstitutionPattern pattern_;
 };

tools/gn/gn.gyp

@@ -264,6 +264,7 @@
         'loader_unittest.cc',
         'ninja_action_target_writer_unittest.cc',
         'ninja_binary_target_writer_unittest.cc',
+        'ninja_bundle_data_target_writer_unittest.cc',
         'ninja_copy_target_writer_unittest.cc',
         'ninja_create_bundle_target_writer_unittest.cc',
         'ninja_group_target_writer_unittest.cc',

tools/gn/ninja_bundle_data_target_writer.cc

@@ -5,6 +5,8 @@
 #include "tools/gn/ninja_bundle_data_target_writer.h"
 
 #include "tools/gn/output_file.h"
+#include "tools/gn/settings.h"
+#include "tools/gn/target.h"
 
 NinjaBundleDataTargetWriter::NinjaBundleDataTargetWriter(const Target* target,
                                                          std::ostream& out)
@@ -13,7 +15,20 @@ NinjaBundleDataTargetWriter::NinjaBundleDataTargetWriter(const Target* target,
 NinjaBundleDataTargetWriter::~NinjaBundleDataTargetWriter() {}
 
 void NinjaBundleDataTargetWriter::Run() {
-  std::vector<OutputFile> files;
-  files.push_back(WriteInputDepsStampAndGetDep(std::vector<const Target*>()));
-  WriteStampForTarget(files, std::vector<OutputFile>());
+  std::vector<OutputFile> output_files;
+  for (const SourceFile& source_file : target_->sources()) {
+    output_files.push_back(
+        OutputFile(settings_->build_settings(), source_file));
+  }
+
+  std::vector<const Target*> extra_hard_deps;
+  OutputFile input_dep = WriteInputDepsStampAndGetDep(extra_hard_deps);
+  if (!input_dep.value().empty())
+    output_files.push_back(input_dep);
+
+  std::vector<OutputFile> order_only_deps;
+  for (const auto& pair : target_->data_deps())
+    order_only_deps.push_back(pair.ptr->dependency_output_file());
+
+  WriteStampForTarget(output_files, order_only_deps);
 }

tools/gn/ninja_bundle_data_target_writer_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tools/gn/ninja_bundle_data_target_writer.h"
+
+#include <algorithm>
+#include <sstream>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "tools/gn/target.h"
+#include "tools/gn/test_with_scope.h"
+
+TEST(NinjaBundleDataTargetWriter, Run) {
+  Err err;
+
+  TestWithScope setup;
+  setup.build_settings()->SetBuildDir(SourceDir("//out/Debug/"));
+
+  Target bundle_data(setup.settings(), Label(SourceDir("//foo/"), "data"));
+  bundle_data.set_output_type(Target::BUNDLE_DATA);
+  bundle_data.sources().push_back(SourceFile("//foo/input1.txt"));
+  bundle_data.sources().push_back(SourceFile("//foo/input2.txt"));
+  bundle_data.sources().push_back(
+      SourceFile("//foo/Foo.xcassets/Contents.json"));
+  bundle_data.sources().push_back(
+      SourceFile("//foo/Foo.xcassets/foo.imageset/Contents.json"));
+  bundle_data.sources().push_back(
+      SourceFile("//foo/Foo.xcassets/foo.imageset/FooIcon-29.png"));
+  bundle_data.sources().push_back(
+      SourceFile("//foo/Foo.xcassets/foo.imageset/FooIcon-29@2x.png"));
+  bundle_data.sources().push_back(
+      SourceFile("//foo/Foo.xcassets/foo.imageset/FooIcon-29@3x.png"));
+  bundle_data.action_values().outputs() = SubstitutionList::MakeForTest(
+      "{{bundle_resources_dir}}/{{source_file_part}}");
+  bundle_data.SetToolchain(setup.toolchain());
+  bundle_data.visibility().SetPublic();
+  ASSERT_TRUE(bundle_data.OnResolved(&err));
+
+  std::ostringstream out;
+  NinjaBundleDataTargetWriter writer(&bundle_data, out);
+  writer.Run();
+
+  const char expected[] =
+      "build obj/foo/data.stamp: stamp "
+          "../../foo/input1.txt "
+          "../../foo/input2.txt "
+          "../../foo/Foo.xcassets/Contents.json "
+          "../../foo/Foo.xcassets/foo.imageset/Contents.json "
+          "../../foo/Foo.xcassets/foo.imageset/FooIcon-29.png "
+          "../../foo/Foo.xcassets/foo.imageset/FooIcon-29@2x.png "
+          "../../foo/Foo.xcassets/foo.imageset/FooIcon-29@3x.png\n";
+  std::string out_str = out.str();
+  EXPECT_EQ(expected, out_str);
+}

tools/gn/ninja_create_bundle_target_writer.cc

@@ -57,13 +57,10 @@ void NinjaCreateBundleTargetWriter::Run() {
 
   std::string code_signing_rule_name = WriteCodeSigningRuleDefinition();
 
-  OutputFile input_dep =
-      WriteInputDepsStampAndGetDep(std::vector<const Target*>());
-
   std::vector<OutputFile> output_files;
-  WriteCopyBundleDataRules(input_dep, &output_files);
-  WriteCompileAssetsCatalogRule(input_dep, &output_files);
-  WriteCodeSigningRules(code_signing_rule_name, input_dep, &output_files);
+  WriteCopyBundleDataSteps(&output_files);
+  WriteCompileAssetsCatalogStep(&output_files);
+  WriteCodeSigningStep(code_signing_rule_name, &output_files);
 
   std::vector<OutputFile> order_only_deps;
   for (const auto& pair : target_->data_deps())
@@ -113,37 +110,41 @@ std::string NinjaCreateBundleTargetWriter::WriteCodeSigningRuleDefinition() {
   return custom_rule_name;
 }
 
-void NinjaCreateBundleTargetWriter::WriteCopyBundleDataRules(
-    const OutputFile& input_dep,
+void NinjaCreateBundleTargetWriter::WriteCopyBundleDataSteps(
     std::vector<OutputFile>* output_files) {
-  for (const BundleFileRule& file_rule : target_->bundle_data().file_rules()) {
-    for (const SourceFile& source_file : file_rule.sources()) {
-      OutputFile output_file = file_rule.ApplyPatternToSourceAsOutputFile(
-          settings_, target_->bundle_data(), source_file);
-      output_files->push_back(output_file);
-
-      out_ << "build ";
-      path_output_.WriteFile(out_, output_file);
-      out_ << ": "
-           << GetNinjaRulePrefixForToolchain(settings_)
-           << Toolchain::ToolTypeToName(Toolchain::TYPE_COPY_BUNDLE_DATA)
-           << " ";
-      path_output_.WriteFile(out_, source_file);
-      if (!input_dep.value().empty()) {
-        out_ << " | ";
-        path_output_.WriteFile(out_, input_dep);
-      }
-      out_ << std::endl;
-    }
+  for (const BundleFileRule& file_rule : target_->bundle_data().file_rules())
+    WriteCopyBundleFileRuleSteps(file_rule, output_files);
+}
+
+void NinjaCreateBundleTargetWriter::WriteCopyBundleFileRuleSteps(
+    const BundleFileRule& file_rule,
+    std::vector<OutputFile>* output_files) {
+  // Note that we don't write implicit deps for copy steps. "copy_bundle_data"
+  // steps as this is most likely implemented using hardlink in the common case.
+  // See NinjaCopyTargetWriter::WriteCopyRules() for a detailed explanation.
+  for (const SourceFile& source_file : file_rule.sources()) {
+    OutputFile output_file = file_rule.ApplyPatternToSourceAsOutputFile(
+        settings_, target_->bundle_data(), source_file);
+    output_files->push_back(output_file);
+
+    out_ << "build ";
+    path_output_.WriteFile(out_, output_file);
+    out_ << ": " << GetNinjaRulePrefixForToolchain(settings_)
+         << Toolchain::ToolTypeToName(Toolchain::TYPE_COPY_BUNDLE_DATA) << " ";
+    path_output_.WriteFile(out_, source_file);
+    out_ << std::endl;
   }
 }
 
-void NinjaCreateBundleTargetWriter::WriteCompileAssetsCatalogRule(
-    const OutputFile& input_dep,
+void NinjaCreateBundleTargetWriter::WriteCompileAssetsCatalogStep(
     std::vector<OutputFile>* output_files) {
-  if (target_->bundle_data().asset_catalog_sources().empty())
+  if (target_->bundle_data().assets_catalog_sources().empty())
     return;
 
+  OutputFile input_dep = WriteCompileAssetsCatalogInputDepsStamp(
+      target_->bundle_data().assets_catalog_deps());
+  DCHECK(!input_dep.value().empty());
+
   OutputFile output_file(settings_->build_settings(),
                          target_->bundle_data().GetCompiledAssetCatalogPath());
   output_files->push_back(output_file);
@@ -154,40 +155,53 @@ void NinjaCreateBundleTargetWriter::WriteCompileAssetsCatalogRule(
        << Toolchain::ToolTypeToName(Toolchain::TYPE_COMPILE_XCASSETS);
 
   std::set<SourceFile> asset_catalog_bundles;
-  for (const auto& source : target_->bundle_data().asset_catalog_sources()) {
-    SourceFile asset_catalog_bundle;
-    CHECK(IsSourceFileFromAssetCatalog(source, &asset_catalog_bundle));
-    if (asset_catalog_bundles.find(asset_catalog_bundle) !=
-        asset_catalog_bundles.end())
-      continue;
+  for (const auto& source : target_->bundle_data().assets_catalog_sources()) {
     out_ << " ";
-    path_output_.WriteFile(out_, asset_catalog_bundle);
-    asset_catalog_bundles.insert(asset_catalog_bundle);
+    path_output_.WriteFile(out_, source);
+    asset_catalog_bundles.insert(source);
   }
 
-  out_ << " |";
-  for (const auto& source : target_->bundle_data().asset_catalog_sources()) {
-    out_ << " ";
-    path_output_.WriteFile(
-        out_, OutputFile(settings_->build_settings(), source));
-  }
+  out_ << " | ";
+  path_output_.WriteFile(out_, input_dep);
+  out_ << std::endl;
+}
 
-  if (!input_dep.value().empty()) {
+OutputFile
+NinjaCreateBundleTargetWriter::WriteCompileAssetsCatalogInputDepsStamp(
+    const std::vector<const Target*>& dependencies) {
+  DCHECK(!dependencies.empty());
+  if (dependencies.size() == 1)
+    return dependencies[0]->dependency_output_file();
+
+  OutputFile xcassets_input_stamp_file =
+      OutputFile(RebasePath(GetTargetOutputDir(target_).value(),
+                            settings_->build_settings()->build_dir(),
+                            settings_->build_settings()->root_path_utf8()));
+  xcassets_input_stamp_file.value().append(target_->label().name());
+  xcassets_input_stamp_file.value().append(".xcassets.inputdeps.stamp");
+
+  out_ << "build ";
+  path_output_.WriteFile(out_, xcassets_input_stamp_file);
+  out_ << ": " << GetNinjaRulePrefixForToolchain(settings_)
+       << Toolchain::ToolTypeToName(Toolchain::TYPE_STAMP);
+
+  for (const Target* target : dependencies) {
     out_ << " ";
-    path_output_.WriteFile(out_, input_dep);
+    path_output_.WriteFile(out_, target->dependency_output_file());
   }
   out_ << std::endl;
+  return xcassets_input_stamp_file;
 }
 
-void NinjaCreateBundleTargetWriter::WriteCodeSigningRules(
+void NinjaCreateBundleTargetWriter::WriteCodeSigningStep(
     const std::string& code_signing_rule_name,
-    const OutputFile& input_dep,
     std::vector<OutputFile>* output_files) {
   if (code_signing_rule_name.empty())
     return;
 
   OutputFile code_signing_input_stamp_file =
-      WriteCodeSigningInputDepsStamp(input_dep, output_files);
+      WriteCodeSigningInputDepsStamp(output_files);
+  DCHECK(!code_signing_input_stamp_file.value().empty());
 
   out_ << "build";
   std::vector<OutputFile> code_signing_output_files;
@@ -197,19 +211,17 @@ void NinjaCreateBundleTargetWriter::WriteCodeSigningRules(
   path_output_.WriteFiles(out_, code_signing_output_files);
 
   // Since the code signature step depends on all the files from the bundle,
-  // the create_bundle stamp can just depends on the output of the signature.
+  // the create_bundle stamp can just depends on the output of the signature
+  // script (dependencies are transitive).
   output_files->swap(code_signing_output_files);
 
   out_ << ": " << code_signing_rule_name;
-  if (!code_signing_input_stamp_file.value().empty()) {
-    out_ << " | ";
-    path_output_.WriteFile(out_, code_signing_input_stamp_file);
-  }
+  out_ << " | ";
+  path_output_.WriteFile(out_, code_signing_input_stamp_file);
   out_ << std::endl;
 }
 
 OutputFile NinjaCreateBundleTargetWriter::WriteCodeSigningInputDepsStamp(
-    const OutputFile& input_dep,
     std::vector<OutputFile>* output_files) {
   std::vector<SourceFile> code_signing_input_files;
   code_signing_input_files.push_back(
@@ -222,15 +234,32 @@ OutputFile NinjaCreateBundleTargetWriter::WriteCodeSigningInputDepsStamp(
     code_signing_input_files.push_back(
         output_file.AsSourceFile(settings_->build_settings()));
   }
-  if (!input_dep.value().empty()) {
-    code_signing_input_files.push_back(
-        input_dep.AsSourceFile(settings_->build_settings()));
+
+  std::vector<const Target*> dependencies;
+  for (const auto& label_target_pair : target_->private_deps()) {
+    if (label_target_pair.ptr->output_type() == Target::BUNDLE_DATA)
+      continue;
+    dependencies.push_back(label_target_pair.ptr);
+  }
+  for (const auto& label_target_pair : target_->public_deps()) {
+    if (label_target_pair.ptr->output_type() == Target::BUNDLE_DATA)
+      continue;
+    dependencies.push_back(label_target_pair.ptr);
   }
 
   DCHECK(!code_signing_input_files.empty());
-  if (code_signing_input_files.size() == 1)
+  if (code_signing_input_files.size() == 1 && dependencies.empty())
     return OutputFile(settings_->build_settings(), code_signing_input_files[0]);
 
+  // Remove possible duplicates (if a target is listed in both deps and
+  // public_deps.
+  std::sort(dependencies.begin(), dependencies.end(),
+            [](const Target* lhs, const Target* rhs) -> bool {
+              return lhs->label() < rhs->label();
+            });
+  dependencies.erase(std::unique(dependencies.begin(), dependencies.end()),
+                     dependencies.end());
+
   OutputFile code_signing_input_stamp_file =
       OutputFile(RebasePath(GetTargetOutputDir(target_).value(),
                             settings_->build_settings()->build_dir(),
@@ -247,6 +276,10 @@ OutputFile NinjaCreateBundleTargetWriter::WriteCodeSigningInputDepsStamp(
     out_ << " ";
     path_output_.WriteFile(out_, source);
   }
+  for (const Target* target : dependencies) {
+    out_ << " ";
+    path_output_.WriteFile(out_, target->dependency_output_file());
+  }
   out_ << std::endl;
   return code_signing_input_stamp_file;
 }

tools/gn/ninja_create_bundle_target_writer.h

@@ -8,6 +8,8 @@
 #include "base/macros.h"
 #include "tools/gn/ninja_target_writer.h"
 
+class BundleFileRule;
+
 // Writes a .ninja file for a bundle_data target type.
 class NinjaCreateBundleTargetWriter : public NinjaTargetWriter {
  public:
@@ -23,33 +25,37 @@ class NinjaCreateBundleTargetWriter : public NinjaTargetWriter {
   // defined, otherwise returns an empty string.
   std::string WriteCodeSigningRuleDefinition();
 
-  // Writes the rule to copy files into the bundle.
+  // Writes the steps to copy files into the bundle.
+  //
+  // The list of newly created files will be added to |output_files|.
+  void WriteCopyBundleDataSteps(std::vector<OutputFile>* output_files);
+
+  // Writes the step to copy files BundleFileRule into the bundle.
   //
-  // input_dep is a file expressing the shared dependencies. It will be a
-  // stamp file if there is more than one.
-  void WriteCopyBundleDataRules(const OutputFile& input_dep,
-                                std::vector<OutputFile>* output_files);
+  // The list of newly created files will be added to |output_files|.
+  void WriteCopyBundleFileRuleSteps(const BundleFileRule& file_rule,
+                                    std::vector<OutputFile>* output_files);
 
-  // Writes the rule to compile assets catalogs.
+  // Writes the step to compile assets catalogs.
   //
-  // input_dep is a file expressing the shared dependencies. It will be a
-  // stamp file if there is more than one.
-  void WriteCompileAssetsCatalogRule(const OutputFile& input_dep,
-                                     std::vector<OutputFile>* output_files);
+  // The list of newly created files will be added to |output_files|.
+  void WriteCompileAssetsCatalogStep(std::vector<OutputFile>* output_files);
+
+  // Writes the stamp file for the assets catalog compilation input
+  // dependencies.
+  OutputFile WriteCompileAssetsCatalogInputDepsStamp(
+      const std::vector<const Target*>& dependencies);
 
-  // Writes the code signing rule (if a script is defined).
+  // Writes the code signing step (if a script is defined).
   //
-  // input_dep is a file expressing the shared dependencies. It will be a
-  // stamp file if there is more than one. As the code signing may include
-  // a manifest of the file, this will depends on all files in output_files
-  // too.
-  void WriteCodeSigningRules(const std::string& code_signing_rule_name,
-                             const OutputFile& input_dep,
-                             std::vector<OutputFile>* output_files);
+  // The list of newly created files will be added to |output_files|. As the
+  // code signing may depends on the full bundle structure, this step will
+  // depends on all files generated via other rules.
+  void WriteCodeSigningStep(const std::string& code_signing_rule_name,
+                            std::vector<OutputFile>* output_files);
 
   // Writes the stamp file for the code signing input dependencies.
   OutputFile WriteCodeSigningInputDepsStamp(
-      const OutputFile& input_dep,
       std::vector<OutputFile>* output_files);
 
   DISALLOW_COPY_AND_ASSIGN(NinjaCreateBundleTargetWriter);

tools/gn/runtime_deps_unittest.cc

@@ -284,6 +284,7 @@ TEST(RuntimeDeps, CreateBundle) {
   InitTargetWithType(setup, &module_data, Target::BUNDLE_DATA);
   module_data.private_deps().push_back(LabelTargetPair(&loadable_module));
   module_data.bundle_data().file_rules().push_back(BundleFileRule(
+      nullptr,
       std::vector<SourceFile>{SourceFile(build_dir + "loadable_module.so")},
       SubstitutionPattern::MakeForTest("{{bundle_resources_dir}}")));
   ASSERT_TRUE(module_data.OnResolved(&err));
@@ -305,7 +306,7 @@ TEST(RuntimeDeps, CreateBundle) {
   InitTargetWithType(setup, &dylib_data, Target::BUNDLE_DATA);
   dylib_data.private_deps().push_back(LabelTargetPair(&dylib));
   dylib_data.bundle_data().file_rules().push_back(BundleFileRule(
-      std::vector<SourceFile>{SourceFile(build_dir + "dylib")},
+      nullptr, std::vector<SourceFile>{SourceFile(build_dir + "dylib")},
       SubstitutionPattern::MakeForTest("{{bundle_executable_dir}}")));
   ASSERT_TRUE(dylib_data.OnResolved(&err));
 

tools/gn/target_unittest.cc

@@ -907,7 +907,7 @@ TEST(Target, PullRecursiveBundleData) {
   ASSERT_EQ(a.bundle_data().file_rules().size(), 2u);
   ASSERT_EQ(a.bundle_data().file_rules()[0].sources().size(), 2u);
   ASSERT_EQ(a.bundle_data().file_rules()[1].sources().size(), 3u);
-  ASSERT_EQ(a.bundle_data().asset_catalog_sources().size(), 4u);
+  ASSERT_EQ(a.bundle_data().assets_catalog_sources().size(), 1u);
   ASSERT_EQ(a.bundle_data().bundle_deps().size(), 2u);
 
   // C gets its data from D.
@@ -918,6 +918,6 @@ TEST(Target, PullRecursiveBundleData) {
   // E does not have any bundle_data information but gets a list of
   // bundle_deps to propagate them during target resolution.
   ASSERT_TRUE(e.bundle_data().file_rules().empty());
-  ASSERT_TRUE(e.bundle_data().asset_catalog_sources().empty());
+  ASSERT_TRUE(e.bundle_data().assets_catalog_sources().empty());
   ASSERT_EQ(e.bundle_data().bundle_deps().size(), 2u);
 }