Corrects the generation of .stamp file names.
When output_name is defined, only the output artifact should be
renamed, not the target's stamp file.
Renaming the stamp file too prevents a refactoring from:
source_set("examples_gn_hello") {
sources = [ "lib.cc" ]
}
to:
# Creates a file examples_gn_hello.stamp
group("examples_gn_hello") {
public_deps = [ ":actual_target" ]
}
# Creates a file examples_gn_hello.stamp
# ... but should be actual_target.stamp
# ... but the artifact output name should still be libexamples_gn_hello.a.
source_set("actual_target") {
sources = [ "lib.cc" ]
output_name = "examples_gn_hello"
}
Both above targets try to generate the file
examples_gn_hello.stamp, which is an error.
How does this situation come up? Originally there was
a source_set("examples_gn_hello"), but through refactoring
we added a layer of indirection. In theory this would be
a correct thing to do, but the implementation detail of the stamp file
naming results in a colission.
Note that because of the requirement that the end product, the library, to
still be named `libexamples_gn_hello`, we can not change the output_name.
Also, because the target //examples/gn:examples_gn_hello is used downstream,
we can not rename it because it would break downstream users.
Change-Id: I8e0adbd73d2bca3aecf61bfe01696bbd75335232
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/12220
Reviewed-by: Roland McGrath <mcgrathr@google.com>
Commit-Queue: Roland McGrath <mcgrathr@google.com>
diff --git a/src/gn/target.cc b/src/gn/target.cc
index 44c5f14..66ffc1f 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -810,10 +810,12 @@
case GENERATED_FILE: {
// These don't get linked to and use stamps which should be the first
// entry in the outputs. These stamps are named
- // "<target_out_dir>/<targetname>.stamp".
+ // "<target_out_dir>/<targetname>.stamp". Setting "output_name" does not
+ // affect the stamp file name: it is always based on the original target
+ // name.
dependency_output_file_ =
GetBuildDirForTargetAsOutputFile(this, BuildDirType::OBJ);
- dependency_output_file_.value().append(GetComputedOutputName());
+ dependency_output_file_.value().append(label().name());
dependency_output_file_.value().append(".stamp");
break;
}
diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc
index 311cbf9..add6d82 100644
--- a/src/gn/target_unittest.cc
+++ b/src/gn/target_unittest.cc
@@ -874,6 +874,36 @@
EXPECT_EQ("//out/Debug/obj/a/a.stamp", computed_outputs[0].value());
}
+TEST_F(TargetTest, CheckStampFileName) {
+ TestWithScope setup;
+
+ Toolchain toolchain(setup.settings(), Label(SourceDir("//tc/"), "tc"));
+
+ std::unique_ptr<Tool> tool = Tool::CreateTool(CTool::kCToolCxx);
+ CTool* cxx = tool->AsC();
+ cxx->set_outputs(SubstitutionList::MakeForTest("{{source_file_part}}.o"));
+ toolchain.SetTool(std::move(tool));
+
+ Target target(setup.settings(), Label(SourceDir("//a/"), "a"));
+ target.set_output_type(Target::SOURCE_SET);
+ target.SetToolchain(&toolchain);
+
+ // Change the output artifact name on purpose.
+ target.set_output_name("b");
+
+ Err err;
+ ASSERT_TRUE(target.OnResolved(&err));
+
+ // Test GetOutputsAsSourceFiles(). Since this is a source set it should give a
+ // stamp file.
+ std::vector<SourceFile> computed_outputs;
+ EXPECT_TRUE(target.GetOutputsAsSourceFiles(LocationRange(), true,
+ &computed_outputs, &err));
+ ASSERT_EQ(1u, computed_outputs.size());
+ EXPECT_EQ("//out/Debug/obj/a/a.stamp", computed_outputs[0].value())
+ << "was instead: " << computed_outputs[0].value();
+}
+
// Tests Target::GetOutputFilesForSource for action_foreach targets (these, like
// copy targets, apply a pattern to the source file). Also tests
// GetOutputsAsSourceFiles() for action_foreach().
