GN: Fix ordering of public and all dep configs.
Previously GN iterated through the deps and added the all_dependent followed by the public configs for each target.
But the ordering is documented to be all of the all_dependent_configs, followed by all of the public_configs. This patch brings the ordering in line with the documentation and adds a test.
BUG=474000
Review-Url: https://codereview.chromium.org/2140363002
Cr-Original-Commit-Position: refs/heads/master@{#405257}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 47e6f464f5f85e3ad0b4bc21b570735e2fcfba7d
diff --git a/tools/gn/target.cc b/tools/gn/target.cc
index 85955a7..afcde09 100644
--- a/tools/gn/target.cc
+++ b/tools/gn/target.cc
@@ -424,14 +424,11 @@
return !outputs->empty();
}
-void Target::PullDependentTargetConfigsFrom(const Target* dep) {
- MergeAllDependentConfigsFrom(dep, &configs_, &all_dependent_configs_);
- MergePublicConfigsFrom(dep, &configs_);
-}
-
void Target::PullDependentTargetConfigs() {
for (const auto& pair : GetDeps(DEPS_LINKED))
- PullDependentTargetConfigsFrom(pair.ptr);
+ MergeAllDependentConfigsFrom(pair.ptr, &configs_, &all_dependent_configs_);
+ for (const auto& pair : GetDeps(DEPS_LINKED))
+ MergePublicConfigsFrom(pair.ptr, &configs_);
}
void Target::PullDependentTargetLibsFrom(const Target* dep, bool is_public) {
diff --git a/tools/gn/target.h b/tools/gn/target.h
index 495a9c1..bd84ec3 100644
--- a/tools/gn/target.h
+++ b/tools/gn/target.h
@@ -313,7 +313,6 @@
// Pulls necessary information from dependencies to this one when all
// dependencies have been resolved.
- void PullDependentTargetConfigsFrom(const Target* dep);
void PullDependentTargetConfigs();
void PullDependentTargetLibsFrom(const Target* dep, bool is_public);
void PullDependentTargetLibs();
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc
index d1c7ab9..2e2e63e 100644
--- a/tools/gn/target_unittest.cc
+++ b/tools/gn/target_unittest.cc
@@ -462,6 +462,71 @@
ASSERT_TRUE(forward.OnResolved(&err));
}
+// Tests that configs are ordered properly between local and pulled ones.
+TEST(Target, ConfigOrdering) {
+ TestWithScope setup;
+ Err err;
+
+ // Make Dep1. It has all_dependent_configs and public_configs.
+ TestTarget dep1(setup, "//:dep1", Target::SOURCE_SET);
+ Label dep1_all_config_label(SourceDir("//"), "dep1_all_config");
+ Config dep1_all_config(setup.settings(), dep1_all_config_label);
+ ASSERT_TRUE(dep1_all_config.OnResolved(&err));
+ dep1.all_dependent_configs().push_back(LabelConfigPair(&dep1_all_config));
+
+ Label dep1_public_config_label(SourceDir("//"), "dep1_public_config");
+ Config dep1_public_config(setup.settings(), dep1_public_config_label);
+ ASSERT_TRUE(dep1_public_config.OnResolved(&err));
+ dep1.public_configs().push_back(LabelConfigPair(&dep1_public_config));
+ ASSERT_TRUE(dep1.OnResolved(&err));
+
+ // Make Dep2 with the same structure.
+ TestTarget dep2(setup, "//:dep2", Target::SOURCE_SET);
+ Label dep2_all_config_label(SourceDir("//"), "dep2_all_config");
+ Config dep2_all_config(setup.settings(), dep2_all_config_label);
+ ASSERT_TRUE(dep2_all_config.OnResolved(&err));
+ dep2.all_dependent_configs().push_back(LabelConfigPair(&dep2_all_config));
+
+ Label dep2_public_config_label(SourceDir("//"), "dep2_public_config");
+ Config dep2_public_config(setup.settings(), dep2_public_config_label);
+ ASSERT_TRUE(dep2_public_config.OnResolved(&err));
+ dep2.public_configs().push_back(LabelConfigPair(&dep2_public_config));
+ ASSERT_TRUE(dep2.OnResolved(&err));
+
+ // This target depends on both previous targets.
+ TestTarget target(setup, "//:foo", Target::SOURCE_SET);
+ target.private_deps().push_back(LabelTargetPair(&dep1));
+ target.private_deps().push_back(LabelTargetPair(&dep2));
+
+ // It also has a private and public config.
+ Label public_config_label(SourceDir("//"), "public");
+ Config public_config(setup.settings(), public_config_label);
+ ASSERT_TRUE(public_config.OnResolved(&err));
+ target.public_configs().push_back(LabelConfigPair(&public_config));
+
+ Label private_config_label(SourceDir("//"), "private");
+ Config private_config(setup.settings(), private_config_label);
+ ASSERT_TRUE(private_config.OnResolved(&err));
+ target.configs().push_back(LabelConfigPair(&private_config));
+
+ // Resolve to get the computed list of configs applying.
+ ASSERT_TRUE(target.OnResolved(&err));
+ const auto& computed = target.configs();
+
+ // Order should be:
+ // 1. local private
+ // 2. local public
+ // 3. inherited all dependent
+ // 4. inherited public
+ ASSERT_EQ(6u, computed.size());
+ EXPECT_EQ(private_config_label, computed[0].label);
+ EXPECT_EQ(public_config_label, computed[1].label);
+ EXPECT_EQ(dep1_all_config_label, computed[2].label);
+ EXPECT_EQ(dep2_all_config_label, computed[3].label);
+ EXPECT_EQ(dep1_public_config_label, computed[4].label);
+ EXPECT_EQ(dep2_public_config_label, computed[5].label);
+}
+
// Tests that different link/depend outputs work for solink tools.
TEST(Target, LinkAndDepOutputs) {
TestWithScope setup;
