GN: Handle deps always in create_bundle target
Stop to use explicit dependency for indirect dependency in the CL broke
Mac builder.
https://chromium-review.googlesource.com/1041506
This CL uses deps/public_deps in create_bundle target always not to
miss the dependency.
Bug: 843470
Change-Id: Iae66f2a3aec20f67ea2e880bb5fd33d6c2aa2f91
Reviewed-on: https://chromium-review.googlesource.com/1065591
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#560203}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 75dfa0ea1bf5f385a36e4d696bb5afeb9a6637bfdiff --git a/tools/gn/ninja_create_bundle_target_writer.cc b/tools/gn/ninja_create_bundle_target_writer.cc
index 873a190..3602df4 100644
--- a/tools/gn/ninja_create_bundle_target_writer.cc
+++ b/tools/gn/ninja_create_bundle_target_writer.cc
@@ -55,14 +55,19 @@
if (!EnsureAllToolsAvailable(target_))
return;
+ // Stamp users are CopyBundleData, CompileAssetsCatalog, CodeSigning and
+ // StampForTarget.
+ size_t num_stamp_uses = 4;
+ std::vector<OutputFile> order_only_deps = WriteInputDepsStampAndGetDep(
+ std::vector<const Target*>(), num_stamp_uses);
+
std::string code_signing_rule_name = WriteCodeSigningRuleDefinition();
std::vector<OutputFile> output_files;
- WriteCopyBundleDataSteps(&output_files);
- WriteCompileAssetsCatalogStep(&output_files);
- WriteCodeSigningStep(code_signing_rule_name, &output_files);
+ WriteCopyBundleDataSteps(order_only_deps, &output_files);
+ WriteCompileAssetsCatalogStep(order_only_deps, &output_files);
+ WriteCodeSigningStep(code_signing_rule_name, order_only_deps, &output_files);
- 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);
@@ -111,13 +116,15 @@
}
void NinjaCreateBundleTargetWriter::WriteCopyBundleDataSteps(
+ 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, output_files);
+ WriteCopyBundleFileRuleSteps(file_rule, order_only_deps, output_files);
}
void NinjaCreateBundleTargetWriter::WriteCopyBundleFileRuleSteps(
const BundleFileRule& file_rule,
+ 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"
// steps as this is most likely implemented using hardlink in the common case.
@@ -132,11 +139,18 @@
out_ << ": " << GetNinjaRulePrefixForToolchain(settings_)
<< Toolchain::ToolTypeToName(Toolchain::TYPE_COPY_BUNDLE_DATA) << " ";
path_output_.WriteFile(out_, source_file);
+
+ if (!order_only_deps.empty()) {
+ out_ << " ||";
+ path_output_.WriteFiles(out_, order_only_deps);
+ }
+
out_ << std::endl;
}
}
void NinjaCreateBundleTargetWriter::WriteCompileAssetsCatalogStep(
+ const std::vector<OutputFile>& order_only_deps,
std::vector<OutputFile>* output_files) {
if (target_->bundle_data().assets_catalog_sources().empty() &&
target_->bundle_data().partial_info_plist().is_null())
@@ -168,8 +182,12 @@
out_ << "build ";
path_output_.WriteFile(out_, partial_info_plist);
out_ << ": " << GetNinjaRulePrefixForToolchain(settings_)
- << Toolchain::ToolTypeToName(Toolchain::TYPE_STAMP) << std::endl;
-
+ << Toolchain::ToolTypeToName(Toolchain::TYPE_STAMP);
+ if (!order_only_deps.empty()) {
+ out_ << " ||";
+ path_output_.WriteFiles(out_, order_only_deps);
+ }
+ out_ << std::endl;
return;
}
@@ -200,6 +218,12 @@
out_ << " | ";
path_output_.WriteFile(out_, input_dep);
+
+ if (!order_only_deps.empty()) {
+ out_ << " ||";
+ path_output_.WriteFiles(out_, order_only_deps);
+ }
+
out_ << std::endl;
out_ << " product_type = " << target_->bundle_data().product_type()
@@ -239,12 +263,13 @@
void NinjaCreateBundleTargetWriter::WriteCodeSigningStep(
const std::string& code_signing_rule_name,
+ const std::vector<OutputFile>& order_only_deps,
std::vector<OutputFile>* output_files) {
if (code_signing_rule_name.empty())
return;
OutputFile code_signing_input_stamp_file =
- WriteCodeSigningInputDepsStamp(output_files);
+ WriteCodeSigningInputDepsStamp(order_only_deps, output_files);
DCHECK(!code_signing_input_stamp_file.value().empty());
out_ << "build";
@@ -266,6 +291,7 @@
}
OutputFile NinjaCreateBundleTargetWriter::WriteCodeSigningInputDepsStamp(
+ const std::vector<OutputFile>& order_only_deps,
std::vector<OutputFile>* output_files) {
std::vector<SourceFile> code_signing_input_files;
code_signing_input_files.push_back(
@@ -279,31 +305,10 @@
output_file.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 && dependencies.empty())
+ if (code_signing_input_files.size() == 1 && order_only_deps.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 =
GetBuildDirForTargetAsOutputFile(target_, BuildDirType::OBJ);
code_signing_input_stamp_file.value().append(target_->label().name());
@@ -318,9 +323,9 @@
out_ << " ";
path_output_.WriteFile(out_, source);
}
- for (const Target* target : dependencies) {
- out_ << " ";
- path_output_.WriteFile(out_, target->dependency_output_file());
+ if (!order_only_deps.empty()) {
+ out_ << " ||";
+ path_output_.WriteFiles(out_, order_only_deps);
}
out_ << std::endl;
return code_signing_input_stamp_file;
diff --git a/tools/gn/ninja_create_bundle_target_writer.h b/tools/gn/ninja_create_bundle_target_writer.h
index 824e7d9..072e02c 100644
--- a/tools/gn/ninja_create_bundle_target_writer.h
+++ b/tools/gn/ninja_create_bundle_target_writer.h
@@ -28,18 +28,23 @@
// 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);
+ void WriteCopyBundleDataSteps(const std::vector<OutputFile>& order_only_deps,
+ std::vector<OutputFile>* output_files);
// Writes the step to copy files BundleFileRule into the bundle.
//
// The list of newly created files will be added to |output_files|.
- void WriteCopyBundleFileRuleSteps(const BundleFileRule& file_rule,
- std::vector<OutputFile>* output_files);
+ void WriteCopyBundleFileRuleSteps(
+ const BundleFileRule& file_rule,
+ const std::vector<OutputFile>& order_only_deps,
+ std::vector<OutputFile>* output_files);
// Writes the step to compile assets catalogs.
//
// The list of newly created files will be added to |output_files|.
- void WriteCompileAssetsCatalogStep(std::vector<OutputFile>* output_files);
+ void WriteCompileAssetsCatalogStep(
+ const std::vector<OutputFile>& order_only_deps,
+ std::vector<OutputFile>* output_files);
// Writes the stamp file for the assets catalog compilation input
// dependencies.
@@ -52,10 +57,12 @@
// 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,
+ const std::vector<OutputFile>& order_only_deps,
std::vector<OutputFile>* output_files);
// Writes the stamp file for the code signing input dependencies.
OutputFile WriteCodeSigningInputDepsStamp(
+ const std::vector<OutputFile>& order_only_deps,
std::vector<OutputFile>* output_files);
DISALLOW_COPY_AND_ASSIGN(NinjaCreateBundleTargetWriter);
diff --git a/tools/gn/ninja_create_bundle_target_writer_unittest.cc b/tools/gn/ninja_create_bundle_target_writer_unittest.cc
index 06fbd51..ff6feaf 100644
--- a/tools/gn/ninja_create_bundle_target_writer_unittest.cc
+++ b/tools/gn/ninja_create_bundle_target_writer_unittest.cc
@@ -5,6 +5,7 @@
#include "tools/gn/ninja_create_bundle_target_writer.h"
#include <algorithm>
+#include <memory>
#include <sstream>
#include "testing/gtest/include/gtest/gtest.h"
@@ -25,6 +26,21 @@
SourceDir(bundle_data->contents_dir().value() + "/Plug Ins");
}
+std::unique_ptr<Target> NewAction(const TestWithScope& setup) {
+ Err err;
+ auto action = std::make_unique<Target>(setup.settings(),
+ Label(SourceDir("//foo/"), "bar"));
+ action->set_output_type(Target::ACTION);
+ action->visibility().SetPublic();
+ action->action_values().set_script(SourceFile("//foo/script.py"));
+
+ action->action_values().outputs() =
+ SubstitutionList::MakeForTest("//out/Debug/foo.out");
+
+ action->SetToolchain(setup.toolchain());
+ return action;
+}
+
} // namespace
// Tests multiple files with an output pattern.
@@ -32,6 +48,9 @@
Err err;
TestWithScope setup;
+ std::unique_ptr<Target> action = NewAction(setup);
+ ASSERT_TRUE(action->OnResolved(&err)) << err.message();
+
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"));
@@ -49,6 +68,7 @@
SetupBundleDataDir(&create_bundle.bundle_data(), "//out/Debug");
create_bundle.set_output_type(Target::CREATE_BUNDLE);
create_bundle.private_deps().push_back(LabelTargetPair(&bundle_data));
+ create_bundle.private_deps().push_back(LabelTargetPair(action.get()));
create_bundle.SetToolchain(setup.toolchain());
ASSERT_TRUE(create_bundle.OnResolved(&err));
@@ -58,12 +78,13 @@
const char expected[] =
"build bar.bundle/Contents/Resources/input1.txt: copy_bundle_data "
- "../../foo/input1.txt\n"
+ "../../foo/input1.txt || obj/foo/bar.stamp\n"
"build bar.bundle/Contents/Resources/input2.txt: copy_bundle_data "
- "../../foo/input2.txt\n"
+ "../../foo/input2.txt || obj/foo/bar.stamp\n"
"build obj/baz/bar.stamp: stamp "
- "bar.bundle/Contents/Resources/input1.txt "
- "bar.bundle/Contents/Resources/input2.txt\n"
+ "bar.bundle/Contents/Resources/input1.txt "
+ "bar.bundle/Contents/Resources/input2.txt"
+ " || obj/foo/bar.stamp\n"
"build bar.bundle: phony obj/baz/bar.stamp\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str);
@@ -74,6 +95,9 @@
Err err;
TestWithScope setup;
+ std::unique_ptr<Target> action = NewAction(setup);
+ ASSERT_TRUE(action->OnResolved(&err)) << err.message();
+
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"));
@@ -91,6 +115,7 @@
SetupBundleDataDir(&create_bundle.bundle_data(), "//out/Debug/gen");
create_bundle.set_output_type(Target::CREATE_BUNDLE);
create_bundle.private_deps().push_back(LabelTargetPair(&bundle_data));
+ create_bundle.private_deps().push_back(LabelTargetPair(action.get()));
create_bundle.SetToolchain(setup.toolchain());
ASSERT_TRUE(create_bundle.OnResolved(&err));
@@ -100,12 +125,12 @@
const char expected[] =
"build gen/bar.bundle/Contents/Resources/input1.txt: copy_bundle_data "
- "../../foo/input1.txt\n"
+ "../../foo/input1.txt || obj/foo/bar.stamp\n"
"build gen/bar.bundle/Contents/Resources/input2.txt: copy_bundle_data "
- "../../foo/input2.txt\n"
+ "../../foo/input2.txt || obj/foo/bar.stamp\n"
"build obj/baz/bar.stamp: stamp "
- "gen/bar.bundle/Contents/Resources/input1.txt "
- "gen/bar.bundle/Contents/Resources/input2.txt\n"
+ "gen/bar.bundle/Contents/Resources/input1.txt "
+ "gen/bar.bundle/Contents/Resources/input2.txt || obj/foo/bar.stamp\n"
"build gen/bar.bundle: phony obj/baz/bar.stamp\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str);
@@ -116,12 +141,16 @@
Err err;
TestWithScope setup;
+ std::unique_ptr<Target> action = NewAction(setup);
+ ASSERT_TRUE(action->OnResolved(&err)) << err.message();
+
Target create_bundle(
setup.settings(),
Label(SourceDir("//baz/"), "bar", setup.toolchain()->label().dir(),
setup.toolchain()->label().name()));
SetupBundleDataDir(&create_bundle.bundle_data(), "//out/Debug");
create_bundle.set_output_type(Target::CREATE_BUNDLE);
+ create_bundle.private_deps().push_back(LabelTargetPair(action.get()));
create_bundle.bundle_data().product_type().assign("com.apple.product-type");
create_bundle.bundle_data().set_partial_info_plist(
SourceFile("//out/Debug/baz/bar/bar_partial_info.plist"));
@@ -133,9 +162,9 @@
writer.Run();
const char expected[] =
- "build baz/bar/bar_partial_info.plist: stamp\n"
+ "build baz/bar/bar_partial_info.plist: stamp || obj/foo/bar.stamp\n"
"build obj/baz/bar.stamp: stamp "
- "baz/bar/bar_partial_info.plist\n"
+ "baz/bar/bar_partial_info.plist || obj/foo/bar.stamp\n"
"build bar.bundle: phony obj/baz/bar.stamp\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str);
@@ -147,6 +176,9 @@
Err err;
TestWithScope setup;
+ std::unique_ptr<Target> action = NewAction(setup);
+ ASSERT_TRUE(action->OnResolved(&err)) << err.message();
+
Target bundle_data(setup.settings(), Label(SourceDir("//foo/"), "data"));
bundle_data.set_output_type(Target::BUNDLE_DATA);
bundle_data.sources().push_back(
@@ -172,6 +204,7 @@
SetupBundleDataDir(&create_bundle.bundle_data(), "//out/Debug");
create_bundle.set_output_type(Target::CREATE_BUNDLE);
create_bundle.private_deps().push_back(LabelTargetPair(&bundle_data));
+ create_bundle.private_deps().push_back(LabelTargetPair(action.get()));
create_bundle.bundle_data().product_type().assign("com.apple.product-type");
create_bundle.SetToolchain(setup.toolchain());
ASSERT_TRUE(create_bundle.OnResolved(&err));
@@ -182,10 +215,10 @@
const char expected[] =
"build bar.bundle/Contents/Resources/Assets.car: compile_xcassets "
- "../../foo/Foo.xcassets | obj/foo/data.stamp\n"
+ "../../foo/Foo.xcassets | obj/foo/data.stamp || obj/foo/bar.stamp\n"
" product_type = com.apple.product-type\n"
"build obj/baz/bar.stamp: stamp "
- "bar.bundle/Contents/Resources/Assets.car\n"
+ "bar.bundle/Contents/Resources/Assets.car || obj/foo/bar.stamp\n"
"build bar.bundle: phony obj/baz/bar.stamp\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str);
@@ -223,6 +256,9 @@
Err err;
TestWithScope setup;
+ std::unique_ptr<Target> action = NewAction(setup);
+ ASSERT_TRUE(action->OnResolved(&err)) << err.message();
+
Target bundle_data0(setup.settings(),
Label(SourceDir("//qux/"), "info_plist"));
bundle_data0.set_output_type(Target::BUNDLE_DATA);
@@ -289,6 +325,7 @@
create_bundle.private_deps().push_back(LabelTargetPair(&bundle_data1));
create_bundle.private_deps().push_back(LabelTargetPair(&bundle_data2));
create_bundle.private_deps().push_back(LabelTargetPair(&bundle_data3));
+ create_bundle.private_deps().push_back(LabelTargetPair(action.get()));
create_bundle.bundle_data().product_type().assign("com.apple.product-type");
create_bundle.bundle_data().set_partial_info_plist(
SourceFile("//out/Debug/baz/bar/bar_partial_info.plist"));
@@ -301,26 +338,27 @@
const char expected[] =
"build bar.bundle/Contents/Info.plist: copy_bundle_data "
- "../../qux/qux-Info.plist\n"
+ "../../qux/qux-Info.plist || obj/foo/bar.stamp\n"
"build bar.bundle/Contents/Resources/input1.txt: copy_bundle_data "
- "../../foo/input1.txt\n"
+ "../../foo/input1.txt || obj/foo/bar.stamp\n"
"build bar.bundle/Contents/Resources/input2.txt: copy_bundle_data "
- "../../foo/input2.txt\n"
+ "../../foo/input2.txt || obj/foo/bar.stamp\n"
"build obj/baz/bar.xcassets.inputdeps.stamp: stamp "
- "obj/foo/assets.stamp "
- "obj/quz/assets.stamp\n"
+ "obj/foo/assets.stamp "
+ "obj/quz/assets.stamp\n"
"build bar.bundle/Contents/Resources/Assets.car | "
- "baz/bar/bar_partial_info.plist: compile_xcassets "
- "../../foo/Foo.xcassets "
- "../../quz/Quz.xcassets | obj/baz/bar.xcassets.inputdeps.stamp\n"
+ "baz/bar/bar_partial_info.plist: compile_xcassets "
+ "../../foo/Foo.xcassets "
+ "../../quz/Quz.xcassets | obj/baz/bar.xcassets.inputdeps.stamp || "
+ "obj/foo/bar.stamp\n"
" product_type = com.apple.product-type\n"
" partial_info_plist = baz/bar/bar_partial_info.plist\n"
"build obj/baz/bar.stamp: stamp "
- "bar.bundle/Contents/Info.plist "
- "bar.bundle/Contents/Resources/input1.txt "
- "bar.bundle/Contents/Resources/input2.txt "
- "bar.bundle/Contents/Resources/Assets.car "
- "baz/bar/bar_partial_info.plist\n"
+ "bar.bundle/Contents/Info.plist "
+ "bar.bundle/Contents/Resources/input1.txt "
+ "bar.bundle/Contents/Resources/input2.txt "
+ "bar.bundle/Contents/Resources/Assets.car "
+ "baz/bar/bar_partial_info.plist || obj/foo/bar.stamp\n"
"build bar.bundle: phony obj/baz/bar.stamp\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str);
@@ -331,6 +369,9 @@
Err err;
TestWithScope setup;
+ std::unique_ptr<Target> action = NewAction(setup);
+ ASSERT_TRUE(action->OnResolved(&err)) << err.message();
+
Target executable(setup.settings(), Label(SourceDir("//baz/"), "quz"));
executable.set_output_type(Target::EXECUTABLE);
executable.sources().push_back(SourceFile("//baz/quz.c"));
@@ -366,6 +407,7 @@
SubstitutionList::MakeForTest("-b=quz", "bar.bundle");
create_bundle.public_deps().push_back(LabelTargetPair(&executable));
create_bundle.private_deps().push_back(LabelTargetPair(&bundle_data));
+ create_bundle.private_deps().push_back(LabelTargetPair(action.get()));
create_bundle.SetToolchain(setup.toolchain());
ASSERT_TRUE(create_bundle.OnResolved(&err));
@@ -380,21 +422,20 @@
" restat = 1\n"
"\n"
"build bar.bundle/Contents/Resources/input1.txt: copy_bundle_data "
- "../../foo/input1.txt\n"
+ "../../foo/input1.txt || obj/foo/bar.stamp\n"
"build bar.bundle/Contents/Resources/input2.txt: copy_bundle_data "
- "../../foo/input2.txt\n"
+ "../../foo/input2.txt || obj/foo/bar.stamp\n"
"build obj/baz/bar.codesigning.inputdeps.stamp: stamp "
- "../../build/codesign.py "
- "quz "
- "bar.bundle/Contents/Resources/input1.txt "
- "bar.bundle/Contents/Resources/input2.txt "
- "./quz\n"
+ "../../build/codesign.py "
+ "quz "
+ "bar.bundle/Contents/Resources/input1.txt "
+ "bar.bundle/Contents/Resources/input2.txt || obj/foo/bar.stamp\n"
"build bar.bundle/Contents/quz bar.bundle/_CodeSignature/CodeResources: "
- "__baz_bar___toolchain_default__code_signing_rule "
- "| obj/baz/bar.codesigning.inputdeps.stamp\n"
+ "__baz_bar___toolchain_default__code_signing_rule "
+ "| obj/baz/bar.codesigning.inputdeps.stamp\n"
"build obj/baz/bar.stamp: stamp "
- "bar.bundle/Contents/quz "
- "bar.bundle/_CodeSignature/CodeResources\n"
+ "bar.bundle/Contents/quz "
+ "bar.bundle/_CodeSignature/CodeResources || obj/foo/bar.stamp\n"
"build bar.bundle: phony obj/baz/bar.stamp\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str);
