[iOS/macOS] consider bundle_data as data-like dependency
Bundle data target's output file doesn't show up as an input
dependency for actions and groups anymore. It's considered as just
order-only dependency. This removes redundant executable linking
step when changes occur in a bundle data.
Change-Id: I79cc4c00af5a4102800d58457b71c81ca6591b68
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/9760
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/ninja_action_target_writer.cc b/src/gn/ninja_action_target_writer.cc
index 28f28c7..c48da06 100644
--- a/src/gn/ninja_action_target_writer.cc
+++ b/src/gn/ninja_action_target_writer.cc
@@ -35,8 +35,14 @@
// operating on the result of that previous step, so we need to be sure to
// serialize these.
std::vector<const Target*> extra_hard_deps;
- for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED))
- extra_hard_deps.push_back(pair.ptr);
+ std::vector<OutputFile> data_outs;
+ for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED)) {
+ if (pair.ptr->IsDataOnly()) {
+ data_outs.push_back(pair.ptr->dependency_output_file());
+ } else {
+ extra_hard_deps.push_back(pair.ptr);
+ }
+ }
// For ACTIONs, the input deps appear only once in the generated ninja
// file, so WriteInputDepsStampAndGetDep() won't create a stamp file
@@ -88,7 +94,6 @@
// done before we run the action.
// TODO(thakis): If the action has just a single output, make things depend
// on that output directly without writing a stamp file.
- std::vector<OutputFile> data_outs;
for (const auto& dep : target_->data_deps())
data_outs.push_back(dep.ptr->dependency_output_file());
WriteStampForTarget(output_files, data_outs);
diff --git a/src/gn/ninja_action_target_writer_unittest.cc b/src/gn/ninja_action_target_writer_unittest.cc
index eb03df1..80b125b 100644
--- a/src/gn/ninja_action_target_writer_unittest.cc
+++ b/src/gn/ninja_action_target_writer_unittest.cc
@@ -171,6 +171,14 @@
dep.SetToolchain(setup.toolchain());
ASSERT_TRUE(dep.OnResolved(&err));
+ Target bundle_data_dep(setup.settings(),
+ Label(SourceDir("//foo/"), "bundle_data_dep"));
+ bundle_data_dep.sources().push_back(SourceFile("//foo/some_data.txt"));
+ bundle_data_dep.set_output_type(Target::BUNDLE_DATA);
+ bundle_data_dep.visibility().SetPublic();
+ bundle_data_dep.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(bundle_data_dep.OnResolved(&err));
+
Target datadep(setup.settings(), Label(SourceDir("//foo/"), "datadep"));
datadep.set_output_type(Target::ACTION);
datadep.visibility().SetPublic();
@@ -180,6 +188,7 @@
Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
target.set_output_type(Target::ACTION_FOREACH);
target.private_deps().push_back(LabelTargetPair(&dep));
+ target.private_deps().push_back(LabelTargetPair(&bundle_data_dep));
target.data_deps().push_back(LabelTargetPair(&datadep));
target.sources().push_back(SourceFile("//foo/input1.txt"));
@@ -226,7 +235,8 @@
" source_name_part = input2\n"
"\n"
"build obj/foo/bar.stamp: "
- "stamp input1.out input2.out || obj/foo/datadep.stamp\n";
+ "stamp input1.out input2.out || obj/foo/bundle_data_dep.stamp "
+ "obj/foo/datadep.stamp\n";
std::string out_str = out.str();
#if defined(OS_WIN)
diff --git a/src/gn/ninja_generated_file_target_writer.cc b/src/gn/ninja_generated_file_target_writer.cc
index 6b0db1b..afe1e08 100644
--- a/src/gn/ninja_generated_file_target_writer.cc
+++ b/src/gn/ninja_generated_file_target_writer.cc
@@ -30,10 +30,15 @@
// on each of the deps and data_deps in the target. The actual collection is
// done at gen time, and so ninja doesn't need to know about it.
std::vector<OutputFile> output_files;
- for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED))
- output_files.push_back(pair.ptr->dependency_output_file());
-
std::vector<OutputFile> data_output_files;
+ for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED)) {
+ if (pair.ptr->IsDataOnly()) {
+ data_output_files.push_back(pair.ptr->dependency_output_file());
+ } else {
+ output_files.push_back(pair.ptr->dependency_output_file());
+ }
+ }
+
const LabelTargetVector& data_deps = target_->data_deps();
for (const auto& pair : data_deps)
data_output_files.push_back(pair.ptr->dependency_output_file());
diff --git a/src/gn/ninja_generated_file_target_writer_unittest.cc b/src/gn/ninja_generated_file_target_writer_unittest.cc
index e88ad49..676ba01 100644
--- a/src/gn/ninja_generated_file_target_writer_unittest.cc
+++ b/src/gn/ninja_generated_file_target_writer_unittest.cc
@@ -36,6 +36,14 @@
dep2.SetToolchain(setup.toolchain());
ASSERT_TRUE(dep2.OnResolved(&err));
+ Target bundle_data_dep(setup.settings(),
+ Label(SourceDir("//foo/"), "bundle_data_dep"));
+ bundle_data_dep.sources().push_back(SourceFile("//foo/some_data.txt"));
+ bundle_data_dep.set_output_type(Target::BUNDLE_DATA);
+ bundle_data_dep.visibility().SetPublic();
+ bundle_data_dep.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(bundle_data_dep.OnResolved(&err));
+
Target datadep(setup.settings(), Label(SourceDir("//foo/"), "datadep"));
datadep.set_output_type(Target::ACTION);
datadep.visibility().SetPublic();
@@ -44,6 +52,7 @@
target.public_deps().push_back(LabelTargetPair(&dep));
target.public_deps().push_back(LabelTargetPair(&dep2));
+ target.public_deps().push_back(LabelTargetPair(&bundle_data_dep));
target.data_deps().push_back(LabelTargetPair(&datadep));
target.SetToolchain(setup.toolchain());
@@ -55,6 +64,6 @@
const char expected[] =
"build obj/foo/bar.stamp: stamp obj/foo/dep.stamp obj/foo/dep2.stamp || "
- "obj/foo/datadep.stamp\n";
+ "obj/foo/bundle_data_dep.stamp obj/foo/datadep.stamp\n";
EXPECT_EQ(expected, out.str());
}
diff --git a/src/gn/ninja_group_target_writer.cc b/src/gn/ninja_group_target_writer.cc
index b518977..beeb54a 100644
--- a/src/gn/ninja_group_target_writer.cc
+++ b/src/gn/ninja_group_target_writer.cc
@@ -20,10 +20,15 @@
// A group rule just generates a stamp file with dependencies on each of
// the deps and data_deps in the group.
std::vector<OutputFile> output_files;
- for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED))
- output_files.push_back(pair.ptr->dependency_output_file());
-
std::vector<OutputFile> data_output_files;
+ for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED)) {
+ if (pair.ptr->IsDataOnly()) {
+ data_output_files.push_back(pair.ptr->dependency_output_file());
+ } else {
+ output_files.push_back(pair.ptr->dependency_output_file());
+ }
+ }
+
const LabelTargetVector& data_deps = target_->data_deps();
for (const auto& pair : data_deps)
data_output_files.push_back(pair.ptr->dependency_output_file());
diff --git a/src/gn/ninja_group_target_writer_unittest.cc b/src/gn/ninja_group_target_writer_unittest.cc
index 2280bb0..1fe51ca 100644
--- a/src/gn/ninja_group_target_writer_unittest.cc
+++ b/src/gn/ninja_group_target_writer_unittest.cc
@@ -27,6 +27,14 @@
dep2.SetToolchain(setup.toolchain());
ASSERT_TRUE(dep2.OnResolved(&err));
+ Target bundle_data_dep(setup.settings(),
+ Label(SourceDir("//foo/"), "bundle_data_dep"));
+ bundle_data_dep.sources().push_back(SourceFile("//foo/some_data.txt"));
+ bundle_data_dep.set_output_type(Target::BUNDLE_DATA);
+ bundle_data_dep.visibility().SetPublic();
+ bundle_data_dep.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(bundle_data_dep.OnResolved(&err));
+
Target datadep(setup.settings(), Label(SourceDir("//foo/"), "datadep"));
datadep.set_output_type(Target::ACTION);
datadep.visibility().SetPublic();
@@ -35,6 +43,7 @@
target.public_deps().push_back(LabelTargetPair(&dep));
target.public_deps().push_back(LabelTargetPair(&dep2));
+ target.public_deps().push_back(LabelTargetPair(&bundle_data_dep));
target.data_deps().push_back(LabelTargetPair(&datadep));
target.SetToolchain(setup.toolchain());
@@ -46,6 +55,6 @@
const char expected[] =
"build obj/foo/bar.stamp: stamp obj/foo/dep.stamp obj/foo/dep2.stamp || "
- "obj/foo/datadep.stamp\n";
+ "obj/foo/bundle_data_dep.stamp obj/foo/datadep.stamp\n";
EXPECT_EQ(expected, out.str());
}
diff --git a/src/gn/ninja_target_writer.cc b/src/gn/ninja_target_writer.cc
index b6bbb9f..7eefc12 100644
--- a/src/gn/ninja_target_writer.cc
+++ b/src/gn/ninja_target_writer.cc
@@ -232,8 +232,14 @@
// Hard dependencies that are direct or indirect dependencies.
// These are large (up to 100s), hence why we check other
const std::set<const Target*>& hard_deps(target_->recursive_hard_deps());
- for (const Target* target : hard_deps)
- input_deps_targets.push_back(target);
+ for (const Target* target : hard_deps) {
+ // BUNDLE_DATA should normally be treated as a data-only dependency
+ // (see Target::IsDataOnly()). Only the CREATE_BUNDLE target, that actually
+ // consumes this data, needs to have the BUNDLE_DATA as an input dependency.
+ if (target->output_type() != Target::BUNDLE_DATA ||
+ target_->output_type() == Target::CREATE_BUNDLE)
+ input_deps_targets.push_back(target);
+ }
// Extra hard dependencies passed in. These are usually empty or small, and
// we don't want to duplicate the explicit hard deps of the target.
diff --git a/src/gn/target.cc b/src/gn/target.cc
index b8a8d05..dd71abb 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -438,6 +438,15 @@
(output_type_ == STATIC_LIBRARY && complete_static_lib_);
}
+bool Target::IsDataOnly() const {
+ // BUNDLE_DATA exists only to declare inputs to subsequent CREATE_BUNDLE
+ // targets. Changing only contents of the bundle data target should not cause
+ // a binary to be re-linked. It should affect only the CREATE_BUNDLE steps
+ // instead. As a result, normal targets should treat this as a data
+ // dependency.
+ return output_type_ == BUNDLE_DATA;
+}
+
DepsIteratorRange Target::GetDeps(DepsIterationType type) const {
if (type == DEPS_LINKED) {
return DepsIteratorRange(
diff --git a/src/gn/target.h b/src/gn/target.h
index dbcfa69..03f456c 100644
--- a/src/gn/target.h
+++ b/src/gn/target.h
@@ -91,6 +91,13 @@
// dependencies, because they also don't propogate libraries up.
bool IsFinal() const;
+ // Set when the target should normally be treated as a data dependency. These
+ // do not need to be treated as inputs or hard dependencies for normal build
+ // steps, but have to be kept in the dependency tree to be properly
+ // propagated. Treating these as data only decreases superfluous rebuilds and
+ // increases parallelism.
+ bool IsDataOnly() const;
+
// Will be the empty string to use the target label as the output name.
// See GetComputedOutputName().
const std::string& output_name() const { return output_name_; }
