Consider bundle_data as public_deps of create_bundle targets. The create_bundle and bundle_data targets works as a pair to copy files into iOS/OS X bundles. The bundle_data target defines the files to copy and the destination in the bundle, the create_bundle target generate the copy rule (as the destination cannot be resolved earlier). This cause a problem with generated files, as gn prevents a target using as input a generated file unless there is a direct dependency or a chain of public dependencies. Since bundle_data/create_bundle are designed to be resolved recursively, update this check to consider all bundle_data as public_deps of create_bundle targets. Convert Target::PullRecursiveBundleData() to O(n) algorithm instead of a O(n^2) by percolating the dependencies up. BUG=297668 Review URL: https://codereview.chromium.org/1804263003 Cr-Original-Commit-Position: refs/heads/master@{#383049} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 1cc4b1ca2085776730f92ab41173c6afd9d51d12
diff --git a/tools/gn/bundle_data.cc b/tools/gn/bundle_data.cc index e22612c..7faccbd 100644 --- a/tools/gn/bundle_data.cc +++ b/tools/gn/bundle_data.cc
@@ -43,26 +43,38 @@ BundleData::~BundleData() {} -void BundleData::AddFileRuleFromTarget(const Target* target) { +void BundleData::AddBundleData(const Target* target) { DCHECK_EQ(target->output_type(), Target::BUNDLE_DATA); + bundle_deps_.push_back(target); +} - std::vector<SourceFile> file_rule_sources; - for (const SourceFile& source_file : target->sources()) { - if (IsSourceFileFromAssetCatalog(source_file, nullptr)) { - asset_catalog_sources_.push_back(source_file); - } else { - file_rule_sources.push_back(source_file); +void BundleData::OnTargetResolved(Target* owning_target) { + // Only initialize file_rules_ and asset_catalog_sources for "create_bundle" + // target (properties are only used by those targets). + if (owning_target->output_type() != Target::CREATE_BUNDLE) + return; + + 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); + } else { + file_rule_sources.push_back(source_file); + } + } + + 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])); } } - 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])); - } + GetSourceFiles(&owning_target->sources()); } -void BundleData::GetSourceFiles(std::vector<SourceFile>* sources) const { +void BundleData::GetSourceFiles(SourceFiles* sources) const { for (const BundleFileRule& file_rule : file_rules_) { sources->insert(sources->end(), file_rule.sources().begin(), file_rule.sources().end()); @@ -72,8 +84,8 @@ } void BundleData::GetOutputFiles(const Settings* settings, - std::vector<OutputFile>* outputs) const { - std::vector<SourceFile> outputs_as_sources; + OutputFiles* outputs) const { + SourceFiles outputs_as_sources; GetOutputsAsSourceFiles(settings, &outputs_as_sources); for (const SourceFile& source_file : outputs_as_sources) outputs->push_back(OutputFile(settings->build_settings(), source_file)); @@ -81,7 +93,7 @@ void BundleData::GetOutputsAsSourceFiles( const Settings* settings, - std::vector<SourceFile>* outputs_as_source) const { + SourceFiles* outputs_as_source) const { for (const BundleFileRule& file_rule : file_rules_) { for (const SourceFile& source : file_rule.sources()) { outputs_as_source->push_back(
diff --git a/tools/gn/bundle_data.h b/tools/gn/bundle_data.h index 9d1df13..e78589e 100644 --- a/tools/gn/bundle_data.h +++ b/tools/gn/bundle_data.h
@@ -9,6 +9,7 @@ #include <vector> #include "tools/gn/bundle_file_rule.h" +#include "tools/gn/unique_vector.h" class OutputFile; class SourceFile; @@ -31,38 +32,46 @@ // BundleData holds the information required by "create_bundle" target. class BundleData { public: + using UniqueTargets = UniqueVector<const Target*>; + using SourceFiles = std::vector<SourceFile>; + using OutputFiles = std::vector<OutputFile>; + using BundleFileRules = std::vector<BundleFileRule>; + BundleData(); ~BundleData(); - // Extracts the information required from a "bundle_data" target. - void AddFileRuleFromTarget(const Target* target); + // Adds a bundle_data target to the recursive collection of all bundle_data + // that the target depends on. + void AddBundleData(const Target* target); + + // Called upon resolution of the target owning this instance of BundleData. + // |owning_target| is the owning target. + void OnTargetResolved(Target* owning_target); // Returns the list of inputs. - void GetSourceFiles(std::vector<SourceFile>* sources) const; + void GetSourceFiles(SourceFiles* sources) const; // Returns the list of outputs. void GetOutputFiles(const Settings* settings, - std::vector<OutputFile>* outputs) const; + OutputFiles* outputs) const; // Returns the list of outputs as SourceFile. void GetOutputsAsSourceFiles( const Settings* settings, - std::vector<SourceFile>* outputs_as_source) const; + SourceFiles* outputs_as_source) const; // Returns the path to the compiled asset catalog. Only valid if // asset_catalog_sources() is not empty. SourceFile GetCompiledAssetCatalogPath() const; // Returns the list of inputs for the compilation of the asset catalog. - std::vector<SourceFile>& asset_catalog_sources() { - return asset_catalog_sources_; - } - const std::vector<SourceFile>& asset_catalog_sources() const { + SourceFiles& asset_catalog_sources() { return asset_catalog_sources_; } + const SourceFiles& asset_catalog_sources() const { return asset_catalog_sources_; } - std::vector<BundleFileRule>& file_rules() { return file_rules_; } - const std::vector<BundleFileRule>& file_rules() const { return file_rules_; } + BundleFileRules& file_rules() { return file_rules_; } + const BundleFileRules& file_rules() const { return file_rules_; } std::string& root_dir() { return root_dir_; } const std::string& root_dir() const { return root_dir_; } @@ -76,9 +85,13 @@ std::string& plugins_dir() { return plugins_dir_; } const std::string& plugins_dir() const { return plugins_dir_; } + // Recursive collection of all bundle_data that the target depends on. + const UniqueTargets& bundle_deps() const { return bundle_deps_; } + private: - std::vector<SourceFile> asset_catalog_sources_; - std::vector<BundleFileRule> file_rules_; + SourceFiles asset_catalog_sources_; + BundleFileRules file_rules_; + UniqueTargets bundle_deps_; // All those values are subdirectories relative to root_build_dir, and apart // from root_dir, they are either equal to root_dir_ or subdirectories of it.
diff --git a/tools/gn/target.cc b/tools/gn/target.cc index 714fd91..2386870 100644 --- a/tools/gn/target.cc +++ b/tools/gn/target.cc
@@ -125,6 +125,14 @@ seen_targets)) return true; // Found a path. } + if (target->output_type() == Target::CREATE_BUNDLE) { + for (const auto& dep : target->bundle_data().bundle_deps()) { + if (EnsureFileIsGeneratedByDependency(dep, file, false, + consider_object_files, + seen_targets)) + return true; // Found a path. + } + } } return false; } @@ -481,38 +489,21 @@ } void Target::PullRecursiveBundleData() { - if (output_type_ != CREATE_BUNDLE) - return; - - std::set<const Target*> visited; - std::vector<const Target*> deps; - deps.push_back(this); - - while (!deps.empty()) { - const Target* current = deps.back(); - deps.pop_back(); - - if (visited.find(current) != visited.end()) + for (const auto& pair : GetDeps(DEPS_LINKED)) { + // Don't propagate bundle_data once they are added to a bundle. + if (pair.ptr->output_type() == CREATE_BUNDLE) continue; - visited.insert(current); - if (current->output_type_ == BUNDLE_DATA) - bundle_data_.AddFileRuleFromTarget(current); + // Direct dependency on a bundle_data target. + if (pair.ptr->output_type() == BUNDLE_DATA) + bundle_data_.AddBundleData(pair.ptr); - for (const LabelTargetPair& pair : current->GetDeps(DEPS_ALL)) { - DCHECK(pair.ptr); - DCHECK(pair.ptr->toolchain_); - if (visited.find(pair.ptr) != visited.end()) - continue; - - if (pair.ptr->output_type() == CREATE_BUNDLE) - continue; - - deps.push_back(pair.ptr); - } + // Recursive bundle_data informations from all dependencies. + for (const auto& target : pair.ptr->bundle_data().bundle_deps()) + bundle_data_.AddBundleData(target); } - bundle_data_.GetSourceFiles(&sources_); + bundle_data_.OnTargetResolved(this); } void Target::FillOutputFiles() {
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc index 8f6684f..876c2e3 100644 --- a/tools/gn/target_unittest.cc +++ b/tools/gn/target_unittest.cc
@@ -821,8 +821,16 @@ 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().bundle_deps().size(), 2u); // C gets its data from D. ASSERT_EQ(c.bundle_data().file_rules().size(), 1u); ASSERT_EQ(c.bundle_data().file_rules()[0].sources().size(), 1u); + ASSERT_EQ(c.bundle_data().bundle_deps().size(), 1u); + + // 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_EQ(e.bundle_data().bundle_deps().size(), 2u); }