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: 75dfa0ea1bf5f385a36e4d696bb5afeb9a6637bf
diff --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);