[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_; }