Revert "GN: do not make indirect dependency to direct dependency" This reverts commit e8a3ce9119189ad6ba5ecaa62bb0b53b07c895f4. Reason for revert: Looks to have broken the mac build; see crbug.com/843351 . Original change's description: > GN: do not make indirect dependency to direct dependency > > If a target is hard dep, there is no need to have the action targets's > recursive deps as direct dependency, because such dependency is > transitive. > > This is found by pcc in gn-dev > https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/BcrSfPQE84E > > With this change, generated android's toolchain.ninja size is reduced > from 262MB to 31MB. And ninja's startup time reduced from 4.9~5.1s to > 2.6s. > I used args.gn same with android_n5x_swarming_rel bot. > > Also this patch reduced the time of `gn gen` from 9.5~10.3s to 6.8~7.2s > on my machine. > > Change-Id: I0f0214d3abe74143516b263da839e98b3987fb64 > Reviewed-on: https://chromium-review.googlesource.com/1041506 > Reviewed-by: Brett Wilson <brettw@chromium.org> > Reviewed-by: Dirk Pranke <dpranke@chromium.org> > Commit-Queue: Takuto Ikuta <tikuta@chromium.org> > Cr-Commit-Position: refs/heads/master@{#557365} TBR=brettw@chromium.org,dpranke@chromium.org,pcc@chromium.org,tikuta@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: Id2bbb6fe1c8a0b10ee4b8d77de6ba83124519b68 Reviewed-on: https://chromium-review.googlesource.com/1060673 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Dirk Pranke <dpranke@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#558946} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: fa3fcf6849130fdeded2358d652faee1873b77b9
diff --git a/tools/gn/ninja_action_target_writer_unittest.cc b/tools/gn/ninja_action_target_writer_unittest.cc index 445a31a..1b2ddc8 100644 --- a/tools/gn/ninja_action_target_writer_unittest.cc +++ b/tools/gn/ninja_action_target_writer_unittest.cc
@@ -409,77 +409,3 @@ "build obj/foo/bar.stamp: stamp input1.out\n"; EXPECT_EQ(expected_linux, out.str()); } - -TEST(NinjaActionTargetWriter, NoTransitiveHardDeps) { - Err err; - TestWithScope setup; - - setup.build_settings()->set_python_path( - base::FilePath(FILE_PATH_LITERAL("/usr/bin/python"))); - - Target dep(setup.settings(), Label(SourceDir("//foo/"), "dep")); - dep.set_output_type(Target::ACTION); - dep.visibility().SetPublic(); - dep.SetToolchain(setup.toolchain()); - ASSERT_TRUE(dep.OnResolved(&err)); - - Target foo(setup.settings(), Label(SourceDir("//foo/"), "foo")); - foo.set_output_type(Target::ACTION); - foo.visibility().SetPublic(); - foo.sources().push_back(SourceFile("//foo/input1.txt")); - foo.action_values().set_script(SourceFile("//foo/script.py")); - foo.private_deps().push_back(LabelTargetPair(&dep)); - foo.SetToolchain(setup.toolchain()); - foo.action_values().outputs() = - SubstitutionList::MakeForTest("//out/Debug/foo.out"); - ASSERT_TRUE(foo.OnResolved(&err)); - - { - std::ostringstream out; - NinjaActionTargetWriter writer(&foo, out); - writer.Run(); - - const char expected_linux[] = - "rule __foo_foo___rule\n" - // These come from the args. - " command = /usr/bin/python ../../foo/script.py\n" - " description = ACTION //foo:foo()\n" - " restat = 1\n" - "\n" - "build foo.out: __foo_foo___rule | ../../foo/script.py" - " ../../foo/input1.txt obj/foo/dep.stamp\n" - "\n" - "build obj/foo/foo.stamp: stamp foo.out\n"; - EXPECT_EQ(expected_linux, out.str()); - } - - Target bar(setup.settings(), Label(SourceDir("//bar/"), "bar")); - bar.set_output_type(Target::ACTION); - bar.sources().push_back(SourceFile("//bar/input1.txt")); - bar.action_values().set_script(SourceFile("//bar/script.py")); - bar.private_deps().push_back(LabelTargetPair(&foo)); - bar.SetToolchain(setup.toolchain()); - bar.action_values().outputs() = - SubstitutionList::MakeForTest("//out/Debug/bar.out"); - ASSERT_TRUE(bar.OnResolved(&err)) << err.message(); - - { - std::ostringstream out; - NinjaActionTargetWriter writer(&bar, out); - writer.Run(); - - const char expected_linux[] = - "rule __bar_bar___rule\n" - // These come from the args. - " command = /usr/bin/python ../../bar/script.py\n" - " description = ACTION //bar:bar()\n" - " restat = 1\n" - "\n" - // Do not have obj/foo/dep.stamp as dependency. - "build bar.out: __bar_bar___rule | ../../bar/script.py" - " ../../bar/input1.txt obj/foo/foo.stamp\n" - "\n" - "build obj/bar/bar.stamp: stamp bar.out\n"; - EXPECT_EQ(expected_linux, out.str()); - } -}
diff --git a/tools/gn/ninja_target_writer_unittest.cc b/tools/gn/ninja_target_writer_unittest.cc index 2bcbc89..68a59c8 100644 --- a/tools/gn/ninja_target_writer_unittest.cc +++ b/tools/gn/ninja_target_writer_unittest.cc
@@ -5,7 +5,6 @@ #include <sstream> #include "testing/gtest/include/gtest/gtest.h" -#include "tools/gn/ninja_action_target_writer.h" #include "tools/gn/ninja_target_writer.h" #include "tools/gn/target.h" #include "tools/gn/test_with_scope.h" @@ -95,23 +94,6 @@ EXPECT_EQ("obj/foo/base.stamp", dep[0].value()); } - { - std::ostringstream stream; - NinjaActionTargetWriter writer(&action, stream); - writer.Run(); - EXPECT_EQ( - "rule __foo_action___rule\n" - " command = ../../foo/script.py\n" - " description = ACTION //foo:action()\n" - " restat = 1\n" - "\n" - "build: __foo_action___rule | ../../foo/script.py" - " ../../foo/action_source.txt ./target\n" - "\n" - "build obj/foo/action.stamp: stamp\n", - stream.str()); - } - // Input deps for action which should depend on the base since its a hard dep // that is a (indirect) dependency, as well as the the action source. { @@ -122,10 +104,9 @@ ASSERT_EQ(1u, dep.size()); EXPECT_EQ("obj/foo/action.inputdeps.stamp", dep[0].value()); - EXPECT_EQ( - "build obj/foo/action.inputdeps.stamp: stamp ../../foo/script.py " - "../../foo/action_source.txt\n", - stream.str()); + EXPECT_EQ("build obj/foo/action.inputdeps.stamp: stamp ../../foo/script.py " + "../../foo/action_source.txt obj/foo/base.stamp\n", + stream.str()); } }
diff --git a/tools/gn/target.cc b/tools/gn/target.cc index e6013e4..15df27f 100644 --- a/tools/gn/target.cc +++ b/tools/gn/target.cc
@@ -372,13 +372,7 @@ PullRecursiveBundleData(); PullDependentTargetLibs(); - - if (!hard_dep()) { - // If this target is not hard_dep type, we need to pull hard dependency from - // dependent target, because it may not transitive for compiling tasks. - PullRecursiveHardDeps(); - } - + PullRecursiveHardDeps(); if (!ResolvePrecompiledHeaders(err)) return false;