GN: Look at all target outputs when detecting duplicates
BUG=554660
Review URL: https://codereview.chromium.org/1828113002
Cr-Original-Commit-Position: refs/heads/master@{#384357}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 83671be906e98ac69bbfaf2c2206977806ff5448
diff --git a/tools/gn/BUILD.gn b/tools/gn/BUILD.gn
index 37bce8d..bb36b1f 100644
--- a/tools/gn/BUILD.gn
+++ b/tools/gn/BUILD.gn
@@ -285,6 +285,7 @@
"loader_unittest.cc",
"ninja_action_target_writer_unittest.cc",
"ninja_binary_target_writer_unittest.cc",
+ "ninja_build_writer_unittest.cc",
"ninja_copy_target_writer_unittest.cc",
"ninja_create_bundle_target_writer_unittest.cc",
"ninja_group_target_writer_unittest.cc",
diff --git a/tools/gn/ninja_build_writer.cc b/tools/gn/ninja_build_writer.cc
index d27ce2d..d289690 100644
--- a/tools/gn/ninja_build_writer.cc
+++ b/tools/gn/ninja_build_writer.cc
@@ -114,8 +114,12 @@
const OutputFile& bad_output) {
std::vector<const Target*> matches;
for (const Target* target : all_targets) {
- if (GetTargetOutputFile(target) == bad_output)
- matches.push_back(target);
+ for (const auto& output : target->computed_outputs()) {
+ if (output == bad_output) {
+ matches.push_back(target);
+ break;
+ }
+ }
}
// There should always be at least two targets generating this file for this
@@ -127,11 +131,10 @@
Err result(matches[0]->defined_from(), "Duplicate output file.",
"Two or more targets generate the same output:\n " +
- bad_output.value() + "\n"
- "This is normally the result of either overriding the output name or\n"
- "having two shared libraries or executables in different directories\n"
- "with the same name (since all such targets will be written to the root\n"
- "output directory).\n\nCollisions:\n" + matches_string);
+ bad_output.value() + "\n\n"
+ "This is can often be fixed by changing one of the target names, or by \n"
+ "setting an output_name on one of them.\n"
+ "\nCollisions:\n" + matches_string);
for (size_t i = 1; i < matches.size(); i++)
result.AppendSubErr(Err(matches[i]->defined_from(), "Collision."));
return result;
@@ -294,12 +297,14 @@
for (const auto& target : default_toolchain_targets_) {
const Label& label = target->label();
- OutputFile target_file = GetTargetOutputFile(target);
- if (!target_files.insert(target_file.value()).second) {
- *err = GetDuplicateOutputError(default_toolchain_targets_, target_file);
- return false;
+ for (const auto& output : target->computed_outputs()) {
+ if (!target_files.insert(output.value()).second) {
+ *err = GetDuplicateOutputError(default_toolchain_targets_, output);
+ return false;
+ }
}
+ OutputFile target_file = GetTargetOutputFile(target);
// Write the long name "foo/bar:baz" for the target "//foo/bar:baz".
std::string long_name = label.GetUserVisibleName(false);
base::TrimString(long_name, "/", &long_name);
diff --git a/tools/gn/ninja_build_writer.h b/tools/gn/ninja_build_writer.h
index 2df3365..0d5b40e 100644
--- a/tools/gn/ninja_build_writer.h
+++ b/tools/gn/ninja_build_writer.h
@@ -30,7 +30,6 @@
const std::vector<const Target*>& default_toolchain_targets,
Err* err);
- private:
NinjaBuildWriter(const BuildSettings* settings,
const std::vector<const Settings*>& all_settings,
const Toolchain* default_toolchain,
@@ -41,6 +40,7 @@
bool Run(Err* err);
+ private:
void WriteNinjaRules();
void WriteLinkPool();
void WriteSubninjas();
diff --git a/tools/gn/ninja_build_writer_unittest.cc b/tools/gn/ninja_build_writer_unittest.cc
new file mode 100644
index 0000000..f82dc83
--- /dev/null
+++ b/tools/gn/ninja_build_writer_unittest.cc
@@ -0,0 +1,119 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <sstream>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "tools/gn/ninja_build_writer.h"
+#include "tools/gn/scheduler.h"
+#include "tools/gn/target.h"
+#include "tools/gn/test_with_scope.h"
+
+TEST(NinjaBuildWriter, TwoTargets) {
+ Scheduler scheduler;
+ TestWithScope setup;
+ Err err;
+
+ Target target_foo(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+ target_foo.set_output_type(Target::ACTION);
+ target_foo.action_values().set_script(SourceFile("//foo/script.py"));
+ target_foo.action_values().outputs() = SubstitutionList::MakeForTest(
+ "//out/Debug/out1.out", "//out/Debug/out2.out");
+ target_foo.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(target_foo.OnResolved(&err));
+
+ Target target_bar(setup.settings(), Label(SourceDir("//bar/"), "bar"));
+ target_bar.set_output_type(Target::ACTION);
+ target_bar.action_values().set_script(SourceFile("//bar/script.py"));
+ target_bar.action_values().outputs() = SubstitutionList::MakeForTest(
+ "//out/Debug/out3.out", "//out/Debug/out4.out");
+ target_bar.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(target_bar.OnResolved(&err));
+
+ std::ostringstream ninja_out;
+ std::ostringstream depfile_out;
+ std::vector<const Settings*> all_settings = {setup.settings()};
+ std::vector<const Target*> targets = {&target_foo, &target_bar};
+ NinjaBuildWriter writer(setup.build_settings(), all_settings,
+ setup.toolchain(), targets, ninja_out, depfile_out);
+ ASSERT_TRUE(writer.Run(&err));
+
+ const char expected_rule_gn[] = "rule gn\n";
+ const char expected_build_ninja[] =
+ "build build.ninja: gn\n"
+ " generator = 1\n"
+ " depfile = build.ninja.d\n"
+ "\n";
+ const char expected_link_pool[] =
+ "pool link_pool\n"
+ " depth = 0\n"
+ "\n";
+ const char expected_toolchain[] =
+ "subninja toolchain.ninja\n"
+ "\n";
+ const char expected_targets[] =
+ "build foo$:bar: phony obj/foo/bar.stamp\n"
+ "build bar$:bar: phony obj/bar/bar.stamp\n"
+ "build bar: phony obj/bar/bar.stamp\n"
+ "\n";
+ const char expected_root_target[] =
+ "build all: phony obj/foo/bar.stamp $\n"
+ " obj/bar/bar.stamp\n"
+ "default all\n";
+ std::string out_str = ninja_out.str();
+#define EXPECT_SNIPPET(expected) \
+ EXPECT_NE(std::string::npos, out_str.find(expected)) << \
+ "Expected to find: " << expected << std::endl << \
+ "Within: " << out_str
+ EXPECT_SNIPPET(expected_rule_gn);
+ EXPECT_SNIPPET(expected_build_ninja);
+ EXPECT_SNIPPET(expected_link_pool);
+ EXPECT_SNIPPET(expected_toolchain);
+ EXPECT_SNIPPET(expected_targets);
+ EXPECT_SNIPPET(expected_root_target);
+#undef EXPECT_SNIPPET
+}
+
+TEST(NinjaBuildWriter, DuplicateOutputs) {
+ Scheduler scheduler;
+ TestWithScope setup;
+ Err err;
+
+ Target target_foo(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+ target_foo.set_output_type(Target::ACTION);
+ target_foo.action_values().set_script(SourceFile("//foo/script.py"));
+ target_foo.action_values().outputs() = SubstitutionList::MakeForTest(
+ "//out/Debug/out1.out", "//out/Debug/out2.out");
+ target_foo.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(target_foo.OnResolved(&err));
+
+ Target target_bar(setup.settings(), Label(SourceDir("//bar/"), "bar"));
+ target_bar.set_output_type(Target::ACTION);
+ target_bar.action_values().set_script(SourceFile("//bar/script.py"));
+ target_bar.action_values().outputs() = SubstitutionList::MakeForTest(
+ "//out/Debug/out3.out", "//out/Debug/out2.out");
+ target_bar.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(target_bar.OnResolved(&err));
+
+ std::ostringstream ninja_out;
+ std::ostringstream depfile_out;
+ std::vector<const Settings*> all_settings = { setup.settings() };
+ std::vector<const Target*> targets = { &target_foo, &target_bar };
+ NinjaBuildWriter writer(setup.build_settings(), all_settings,
+ setup.toolchain(), targets, ninja_out, depfile_out);
+ ASSERT_FALSE(writer.Run(&err));
+
+ const char expected_help_test[] =
+ "Two or more targets generate the same output:\n"
+ " out2.out\n"
+ "\n"
+ "This is can often be fixed by changing one of the target names, or by \n"
+ "setting an output_name on one of them.\n"
+ "\n"
+ "Collisions:\n"
+ " //foo:bar\n"
+ " //bar:bar\n";
+
+ EXPECT_EQ(expected_help_test, err.help_text());
+}
