Revert "Fix output directories for generated source files."
This reverts commit ab9104586734cb45aa77c70ca5042edbcc9f6aa5.
Reason for revert: There may be other ways to solve the original issue.
Original change's description:
> Fix output directories for generated source files.
>
> This CL fixes an annoying issue where the local build directory path
> leaks into the generated Ninja files when {{source_out_dir}} or
> even {{source_gen_dir}} are expanded.
>
> In the case of a generated source file, its path will be source-absolute
> but located in the build directory, for example:
>
> //out/Debug/gen/header.h
> //out/Debug/toolchain/gen/src/source.cc
>
> Before this CL, the computed output directory would leak the 'out/Debug'
> prefix, i.e. compute an output path, relative to `out/Debug` as:
>
> obj/out/Debug/gen/
> obj/out/Debug/toolchain/gen/src/
>
> After this CL, the build directory prefix is replaced with the
> "BUILD_DIR" string literal, and the result becomes:
>
> obj/BUILD_DIR/gen/
> obj/BUILD_DIR/toolchain/gen/src
>
> This makes the commands more hermetic, especially with regards to remote
> builds which implement content-based caching, using the command hash as
> a key.
>
> I tested that I could still build Fuchsia, Chromium and Pigweed with
> this patch (as long as Pigwedd's build system is fixed with [1] to
> support upstream GN).
>
> Note that this CL is required to unblock the GN auto-roller for the
> Fuchsia project: without it, the output directory leak checks will fail
> the builds since the latest Rust-related changes adds a ton of
> `-Lnative=<path-with-build-directory>` to a very large number of
> Rust link commands.
>
> [1] https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/90080
>
> Change-Id: I2dc811c295aa47dc412175bd74acc1333f8145e8
> Reviewed-on: https://gn-review.googlesource.com/c/gn/+/13240
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Commit-Queue: David Turner <digit@google.com>
TBR=brettw@chromium.org,digit@google.com,sdefresne@chromium.org
Change-Id: I290e2e8375c1e6aebfa76c2c00910bd364a54562
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/13540
Reviewed-by: David Turner <digit@google.com>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
diff --git a/src/gn/filesystem_utils.cc b/src/gn/filesystem_utils.cc
index 7a7078e..6de3fbd 100644
--- a/src/gn/filesystem_utils.cc
+++ b/src/gn/filesystem_utils.cc
@@ -1025,25 +1025,10 @@
OutputFile result = GetBuildDirAsOutputFile(context, type);
if (source_dir.is_source_absolute()) {
- std::string_view build_dir = context.build_settings->build_dir().value();
- std::string_view source_dir_path = source_dir.value();
- if (source_dir_path.substr(0, build_dir.size()) == build_dir) {
- // The source dir is source-absolute, but in the build directory
- // (e.g. `//out/Debug/gen/src/foo.cc` or
- // `//out/Debug/toolchain1/gen/foo.cc`), which happens for generated
- // sources. In this case, remove the build directory prefix, and replace
- // it with `BUILD_DIR`. This will create results like `obj/BUILD_DIR/gen`
- // or `toolchain2/obj/BUILD_DIR/toolchain1/gen` which look surprising,
- // but guarantee unicity.
- result.value().append("BUILD_DIR/");
- result.value().append(&source_dir_path[build_dir.size()],
- source_dir_path.size() - build_dir.size());
- } else {
- // The source dir is source-absolute, so we trim off the two leading
- // slashes to append to the toolchain object directory.
- result.value().append(&source_dir.value()[2],
- source_dir.value().size() - 2);
- }
+ // The source dir is source-absolute, so we trim off the two leading
+ // slashes to append to the toolchain object directory.
+ result.value().append(&source_dir.value()[2],
+ source_dir.value().size() - 2);
} else {
// System-absolute.
AppendFixedAbsolutePathSuffix(context.build_settings, source_dir, &result);
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc
index b32d8c7..e4ec800 100644
--- a/src/gn/ninja_c_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -405,18 +405,18 @@
"target_out_dir = obj/foo\n"
"target_output_name = gen_obj\n"
"\n"
- "build obj/BUILD_DIR/gen_obj.generated.o: cxx generated.cc"
+ "build obj/out/Debug/gen_obj.generated.o: cxx generated.cc"
" || obj/foo/generate.stamp\n"
" source_file_part = generated.cc\n"
" source_name_part = generated\n"
"\n"
- "build obj/foo/gen_obj.stamp: stamp obj/BUILD_DIR/gen_obj.generated.o"
+ "build obj/foo/gen_obj.stamp: stamp obj/out/Debug/gen_obj.generated.o"
// The order-only dependency here is strictly unnecessary since the
// sources list this as an order-only dep.
" || obj/foo/generate.stamp\n";
std::string obj_str = obj_out.str();
- EXPECT_EQ(std::string(obj_expected), obj_str);
+ EXPECT_EQ(obj_expected, obj_str);
// A shared library depends on gen_obj, having corresponding header for
// generated obj.
@@ -442,7 +442,7 @@
"target_output_name = libgen_lib\n"
"\n"
"\n"
- "build ./libgen_lib.so: solink obj/BUILD_DIR/gen_obj.generated.o"
+ "build ./libgen_lib.so: solink obj/out/Debug/gen_obj.generated.o"
// The order-only dependency here is strictly unnecessary since
// obj/out/Debug/gen_obj.generated.o has dependency to
// obj/foo/gen_obj.stamp
diff --git a/src/gn/substitution_writer_unittest.cc b/src/gn/substitution_writer_unittest.cc
index eaa521a..fc3c446 100644
--- a/src/gn/substitution_writer_unittest.cc
+++ b/src/gn/substitution_writer_unittest.cc
@@ -45,12 +45,6 @@
SourceFile result = SubstitutionWriter::ApplyPatternToSource(
nullptr, setup.settings(), pattern, SourceFile("//foo/bar/myfile.txt"));
ASSERT_EQ("//out/Debug/gen/foo/bar/myfile.tmp", result.value());
-
- result = SubstitutionWriter::ApplyPatternToSource(
- nullptr, setup.settings(), pattern,
- SourceFile("//out/Debug/gen/generated_file.cc"));
- ASSERT_EQ("//out/Debug/gen/BUILD_DIR/gen/generated_file.tmp", result.value())
- << result.value();
}
TEST(SubstitutionWriter, ApplyPatternToSourceAsOutputFile) {
