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: fa3fcf6849130fdeded2358d652faee1873b77b9diff --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;
