Revert "[bundle] Use "phony" builtin tool for create_bundle targets" This reverts commit cfddfffb7913868936e76a269ae824aadd737b1b. Reason for revert: this breaks the incremental build of certains targets of Chromium (i.e. base_unittests for device when the gn variable `enable_run_ios_unittests_with_xctest = true`). This is due to https://github.com/ninja-build/ninja/issues/2405 which is fixed in the development branch but not yet released. Original change's description: > [bundle] Use "phony" builtin tool for create_bundle targets > > Instead of using the "stamp" tool which creates files (and potentially > slow down the build since this requires access to the filesystem), use > the builtin "phony" tool. > > Bug: 194 > Change-Id: Ie1af7020af4e7efc6c8848244c21dac549f179aa > Reviewed-on: https://gn-review.googlesource.com/c/gn/+/16920 > Reviewed-by: David Turner <digit@google.com> > Commit-Queue: Sylvain Defresne <sdefresne@chromium.org> # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 194 Change-Id: Id14e5c08e2ca40f6b2fe40d3a526295786f83302 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/16940 Reviewed-by: David Turner <digit@google.com> Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
diff --git a/src/gn/ninja_create_bundle_target_writer.cc b/src/gn/ninja_create_bundle_target_writer.cc index 2eacee4..efa950e 100644 --- a/src/gn/ninja_create_bundle_target_writer.cc +++ b/src/gn/ninja_create_bundle_target_writer.cc
@@ -7,7 +7,6 @@ #include <iterator> #include "base/strings/string_util.h" -#include "gn/builtin_tool.h" #include "gn/filesystem_utils.h" #include "gn/general_tool.h" #include "gn/ninja_utils.h" @@ -37,23 +36,15 @@ tool_name + "\" tool.")); } -bool EnsureToolAvailable(const Target* target, const char* tool) { - if (!target->toolchain()->GetTool(tool)) { - FailWithMissingToolError(tool, target); - return false; - } - - return true; -} - bool EnsureAllToolsAvailable(const Target* target) { const char* kRequiredTools[] = { GeneralTool::kGeneralToolCopyBundleData, - GeneralTool::kGeneralToolStamp, // To create empty partial Info.plist. + GeneralTool::kGeneralToolStamp, }; - for (const char* tool : kRequiredTools) { - if (!EnsureToolAvailable(target, tool)) { + for (size_t i = 0; i < std::size(kRequiredTools); ++i) { + if (!target->toolchain()->GetTool(kRequiredTools[i])) { + FailWithMissingToolError(kRequiredTools[i], target); return false; } } @@ -61,8 +52,10 @@ // The compile_xcassets tool is only required if the target has asset // catalog resources to compile. if (TargetRequireAssetCatalogCompilation(target)) { - if (!EnsureToolAvailable(target, - GeneralTool::kGeneralToolCompileXCAssets)) { + if (!target->toolchain()->GetTool( + GeneralTool::kGeneralToolCompileXCAssets)) { + FailWithMissingToolError(GeneralTool::kGeneralToolCompileXCAssets, + target); return false; } } @@ -109,8 +102,7 @@ OutputFile(settings_->build_settings(), target_->bundle_data().GetBundleRootDirOutput(settings_))); - out_ << ": " << BuiltinTool::kBuiltinToolPhony << " " - << target_->dependency_output_file().value(); + out_ << ": phony " << target_->dependency_output_file().value(); out_ << std::endl; } @@ -299,7 +291,8 @@ out_ << "build "; WriteOutput(xcassets_input_stamp_file); - out_ << ": " << BuiltinTool::kBuiltinToolPhony; + out_ << ": " << GetNinjaRulePrefixForToolchain(settings_) + << GeneralTool::kGeneralToolStamp; for (const Target* target : dependencies) { out_ << " "; @@ -364,7 +357,8 @@ out_ << "build "; WriteOutput(code_signing_input_stamp_file); - out_ << ": " << BuiltinTool::kBuiltinToolPhony; + out_ << ": " << GetNinjaRulePrefixForToolchain(settings_) + << GeneralTool::kGeneralToolStamp; for (const SourceFile& source : code_signing_input_files) { out_ << " ";
diff --git a/src/gn/ninja_create_bundle_target_writer_unittest.cc b/src/gn/ninja_create_bundle_target_writer_unittest.cc index a5fecca..997c058 100644 --- a/src/gn/ninja_create_bundle_target_writer_unittest.cc +++ b/src/gn/ninja_create_bundle_target_writer_unittest.cc
@@ -170,7 +170,7 @@ "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) << expected << "----\n" << out_str; + EXPECT_EQ(expected, out_str); } // Tests multiple files from asset catalog. @@ -393,7 +393,7 @@ "../../foo/input1.txt || obj/baz/bar.inputdeps.stamp\n" "build bar.bundle/Contents/Resources/input2.txt: copy_bundle_data " "../../foo/input2.txt || obj/baz/bar.inputdeps.stamp\n" - "build obj/baz/bar.xcassets.inputdeps.stamp: phony " + "build obj/baz/bar.xcassets.inputdeps.stamp: stamp " "obj/foo/assets.stamp " "obj/quz/assets.stamp obj/biz/assets.stamp\n" "build bar.bundle/Contents/Resources/Assets.car | " @@ -411,7 +411,7 @@ "baz/bar/bar_partial_info.plist || obj/baz/bar.inputdeps.stamp\n" "build bar.bundle: phony obj/baz/bar.stamp\n"; std::string out_str = out.str(); - EXPECT_EQ(expected, out_str) << expected << "----\n" << out_str; + EXPECT_EQ(expected, out_str); } // Tests code signing steps. @@ -477,7 +477,7 @@ "../../foo/input1.txt || obj/baz/bar.inputdeps.stamp\n" "build bar.bundle/Contents/Resources/input2.txt: copy_bundle_data " "../../foo/input2.txt || obj/baz/bar.inputdeps.stamp\n" - "build obj/baz/bar.codesigning.inputdeps.stamp: phony " + "build obj/baz/bar.codesigning.inputdeps.stamp: stamp " "../../build/codesign.py " "quz " "bar.bundle/Contents/Resources/input1.txt " @@ -491,5 +491,5 @@ "bar.bundle/_CodeSignature/CodeResources || obj/baz/bar.inputdeps.stamp\n" "build bar.bundle: phony obj/baz/bar.stamp\n"; std::string out_str = out.str(); - EXPECT_EQ(expected, out_str) << expected << "----\n" << out_str; + EXPECT_EQ(expected, out_str); }