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);
}