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