Make GN public configs apply libs to targets.
Previously, putting libs or lib_dirs in a config that a target exports as a public config would not result in those values getting applied to dependant targets.
This patch applies those to the targets how one would expect by resolving the public configs before the libs are copied. This involved splitting up pulling dependencies' information into a configs phase (which must happen before we update the libs on the target) and the libs phase (which must happen after so the dependencies' libs come after).
Clarify ordering in the help.
Review URL: https://codereview.chromium.org/1518663003
Cr-Original-Commit-Position: refs/heads/master@{#364749}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 567cb68891e32e2a398302b2b2405e4250ccc1d7
diff --git a/tools/gn/target.cc b/tools/gn/target.cc
index 4a93abc..16a0ba4 100644
--- a/tools/gn/target.cc
+++ b/tools/gn/target.cc
@@ -160,21 +160,40 @@
ScopedTrace trace(TraceItem::TRACE_ON_RESOLVED, label());
trace.SetToolchain(settings()->toolchain_label());
- // Copy our own dependent configs to the list of configs applying to us.
+ // Copy this target's own dependent and public configs to the list of configs
+ // applying to it.
configs_.Append(all_dependent_configs_.begin(), all_dependent_configs_.end());
MergePublicConfigsFrom(this, &configs_);
+ // Copy public configs from all dependencies into the list of configs
+ // applying to this target (configs_).
+ PullDependentTargetConfigs();
+
+ // Copies public dependencies' public configs to this target's public
+ // configs. These configs have already been applied to this target by
+ // PullDependentTargetConfigs above, along with the public configs from
+ // private deps. This step re-exports them as public configs for targets that
+ // depend on this one.
+ for (const auto& dep : public_deps_) {
+ public_configs_.Append(dep.ptr->public_configs().begin(),
+ dep.ptr->public_configs().end());
+ }
+
// Copy our own libs and lib_dirs to the final set. This will be from our
// target and all of our configs. We do this specially since these must be
// inherited through the dependency tree (other flags don't work this way).
+ //
+ // This needs to happen after we pull dependent target configs for the
+ // public config's libs to be included here. And it needs to happen
+ // before pulling the dependent target libs so the libs are in the correct
+ // order (local ones first, then the dependency's).
for (ConfigValuesIterator iter(this); !iter.done(); iter.Next()) {
const ConfigValues& cur = iter.cur();
all_lib_dirs_.append(cur.lib_dirs().begin(), cur.lib_dirs().end());
all_libs_.append(cur.libs().begin(), cur.libs().end());
}
- PullDependentTargets();
- PullPublicConfigs();
+ PullDependentTargetLibs();
PullRecursiveHardDeps();
if (!ResolvePrecompiledHeaders(err))
return false;
@@ -266,10 +285,17 @@
return false;
}
-void Target::PullDependentTarget(const Target* dep, bool is_public) {
+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);
+}
+
+void Target::PullDependentTargetLibsFrom(const Target* dep, bool is_public) {
// Direct dependent libraries.
if (dep->output_type() == STATIC_LIBRARY ||
dep->output_type() == SHARED_LIBRARY ||
@@ -309,22 +335,11 @@
}
}
-void Target::PullDependentTargets() {
+void Target::PullDependentTargetLibs() {
for (const auto& dep : public_deps_)
- PullDependentTarget(dep.ptr, true);
+ PullDependentTargetLibsFrom(dep.ptr, true);
for (const auto& dep : private_deps_)
- PullDependentTarget(dep.ptr, false);
-}
-
-void Target::PullPublicConfigs() {
- // Pull public configs from each of our dependency's public deps.
- for (const auto& dep : public_deps_)
- PullPublicConfigsFrom(dep.ptr);
-}
-
-void Target::PullPublicConfigsFrom(const Target* from) {
- public_configs_.Append(from->public_configs().begin(),
- from->public_configs().end());
+ PullDependentTargetLibsFrom(dep.ptr, false);
}
void Target::PullRecursiveHardDeps() {
diff --git a/tools/gn/target.h b/tools/gn/target.h
index 10d3b05..eff85e8 100644
--- a/tools/gn/target.h
+++ b/tools/gn/target.h
@@ -249,13 +249,10 @@
// Pulls necessary information from dependencies to this one when all
// dependencies have been resolved.
- void PullDependentTarget(const Target* dep, bool is_public);
- void PullDependentTargets();
-
- // These each pull specific things from dependencies to this one when all
- // deps have been resolved.
- void PullPublicConfigs();
- void PullPublicConfigsFrom(const Target* from);
+ void PullDependentTargetConfigsFrom(const Target* dep);
+ void PullDependentTargetConfigs();
+ void PullDependentTargetLibsFrom(const Target* dep, bool is_public);
+ void PullDependentTargetLibs();
void PullRecursiveHardDeps();
// Fills the link and dependency output files when a target is resolved.
@@ -289,6 +286,7 @@
LabelTargetVector public_deps_;
LabelTargetVector data_deps_;
+ // See getters for more info.
UniqueVector<LabelConfigPair> configs_;
UniqueVector<LabelConfigPair> all_dependent_configs_;
UniqueVector<LabelConfigPair> public_configs_;
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc
index 426243e..e7c4c31 100644
--- a/tools/gn/target_unittest.cc
+++ b/tools/gn/target_unittest.cc
@@ -386,6 +386,8 @@
Label pub_config_label(SourceDir("//a/"), "pubconfig");
Config pub_config(setup.settings(), pub_config_label);
+ std::string lib_name("testlib");
+ pub_config.own_values().libs().push_back(lib_name);
ASSERT_TRUE(pub_config.OnResolved(&err));
// This is the destination target that has a public config.
@@ -406,6 +408,11 @@
ASSERT_EQ(1u, dep_on_pub.configs().size());
EXPECT_EQ(&pub_config, dep_on_pub.configs()[0].ptr);
+ // Libs have special handling, check that they were forwarded from the
+ // public config to all_libs.
+ ASSERT_EQ(1u, dep_on_pub.all_libs().size());
+ ASSERT_EQ(lib_name, dep_on_pub.all_libs()[0]);
+
// This target has a private dependency on dest for forwards configs.
TestTarget forward(setup, "//a:f", Target::SOURCE_SET);
forward.private_deps().push_back(LabelTargetPair(&dest));
diff --git a/tools/gn/variables.cc b/tools/gn/variables.cc
index 3b1f7b1..d79c3aa 100644
--- a/tools/gn/variables.cc
+++ b/tools/gn/variables.cc
@@ -861,6 +861,12 @@
" uniquified so each one is only passed once (the first instance of it\n" \
" will be the one used).\n"
+#define LIBS_AND_LIB_DIRS_ORDERING_HELP \
+ "\n" \
+ " For \"libs\" and \"lib_dirs\" only, the values propagated from\n" \
+ " dependencies (as described above) are applied last assuming they\n" \
+ " are not already in the list.\n"
+
const char kLibDirs[] = "lib_dirs";
const char kLibDirs_HelpShort[] =
"lib_dirs: [directory list] Additional library directories.";
@@ -874,6 +880,7 @@
" will be treated as being relative to the current build file.\n"
COMMON_LIB_INHERITANCE_HELP
COMMON_ORDERING_HELP
+ LIBS_AND_LIB_DIRS_ORDERING_HELP
"\n"
"Example\n"
"\n"
@@ -906,6 +913,7 @@
" links framework dependencies.\n"
COMMON_LIB_INHERITANCE_HELP
COMMON_ORDERING_HELP
+ LIBS_AND_LIB_DIRS_ORDERING_HELP
"\n"
"Examples\n"
"\n"
