Add ResolvedTargetData::GetLinkedFrameworks()
This CL removes Target::all_framework_dirs(), Target::all_frameworks()
and Target::all_weak_frameworks(), moving their computation to the
ResolvedTargetData class, where they will be created on demand, and
returned by the GetLinkedFrameworkDirs(), GetLinkedFrameworks() and
GetLinkedWeakFrameworks() methods respectively.
Bug: 331
Change-Id: Iecefbcc4182265113d5939f8a854d12c2670b8f5
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/15324
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Commit-Queue: David Turner <digit@google.com>
diff --git a/src/gn/desc_builder.cc b/src/gn/desc_builder.cc
index f82c079..58bebef 100644
--- a/src/gn/desc_builder.cc
+++ b/src/gn/desc_builder.cc
@@ -608,7 +608,7 @@
}
if (what(variables::kFrameworks)) {
- const auto& all_frameworks = target_->all_frameworks();
+ const auto& all_frameworks = resolved.GetLinkedFrameworks(target_);
if (!all_frameworks.empty()) {
auto frameworks = std::make_unique<base::ListValue>();
for (size_t i = 0; i < all_frameworks.size(); i++)
@@ -618,7 +618,7 @@
}
}
if (what(variables::kWeakFrameworks)) {
- const auto& weak_frameworks = target_->all_weak_frameworks();
+ const auto& weak_frameworks = resolved.GetLinkedWeakFrameworks(target_);
if (!weak_frameworks.empty()) {
auto frameworks = std::make_unique<base::ListValue>();
for (size_t i = 0; i < weak_frameworks.size(); i++)
@@ -629,7 +629,7 @@
}
if (what(variables::kFrameworkDirs)) {
- const auto& all_framework_dirs = target_->all_framework_dirs();
+ const auto& all_framework_dirs = resolved.GetLinkedFrameworkDirs(target_);
if (!all_framework_dirs.empty()) {
auto framework_dirs = std::make_unique<base::ListValue>();
for (size_t i = 0; i < all_framework_dirs.size(); i++)
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc
index d9d403f..b1dbbef 100644
--- a/src/gn/ninja_binary_target_writer.cc
+++ b/src/gn/ninja_binary_target_writer.cc
@@ -341,7 +341,7 @@
}
}
- const auto& all_framework_dirs = target_->all_framework_dirs();
+ const auto& all_framework_dirs = resolved().GetLinkedFrameworkDirs(target_);
if (!all_framework_dirs.empty()) {
// Since we're passing these on the command line to the linker and not
// to Ninja, we need to do shell escaping.
@@ -398,13 +398,13 @@
const Tool* tool) {
// Frameworks that have been recursively pushed through the dependency tree.
FrameworksWriter writer(tool->framework_switch());
- const auto& all_frameworks = target_->all_frameworks();
+ const auto& all_frameworks = resolved().GetLinkedFrameworks(target_);
for (size_t i = 0; i < all_frameworks.size(); i++) {
writer(all_frameworks[i], out);
}
FrameworksWriter weak_writer(tool->weak_framework_switch());
- const auto& all_weak_frameworks = target_->all_weak_frameworks();
+ const auto& all_weak_frameworks = resolved().GetLinkedWeakFrameworks(target_);
for (size_t i = 0; i < all_weak_frameworks.size(); i++) {
weak_writer(all_weak_frameworks[i], out);
}
diff --git a/src/gn/resolved_target_data.cc b/src/gn/resolved_target_data.cc
index 722858d..3208f04 100644
--- a/src/gn/resolved_target_data.cc
+++ b/src/gn/resolved_target_data.cc
@@ -36,3 +36,29 @@
info->libs = all_libs.release();
info->has_lib_info = true;
}
+
+void ResolvedTargetData::ComputeFrameworkInfo(TargetInfo* info) const {
+ UniqueVector<SourceDir> all_framework_dirs;
+ UniqueVector<std::string> all_frameworks;
+ UniqueVector<std::string> all_weak_frameworks;
+
+ for (ConfigValuesIterator iter(info->target); !iter.done(); iter.Next()) {
+ const ConfigValues& cur = iter.cur();
+ all_framework_dirs.Append(cur.framework_dirs());
+ all_frameworks.Append(cur.frameworks());
+ all_weak_frameworks.Append(cur.weak_frameworks());
+ }
+ for (const Target* dep : info->deps.linked_deps()) {
+ if (!dep->IsFinal() || dep->output_type() == Target::STATIC_LIBRARY) {
+ const TargetInfo* dep_info = GetTargetFrameworkInfo(dep);
+ all_framework_dirs.Append(dep_info->framework_dirs);
+ all_frameworks.Append(dep_info->frameworks);
+ all_weak_frameworks.Append(dep_info->weak_frameworks);
+ }
+ }
+
+ info->framework_dirs = all_framework_dirs.release();
+ info->frameworks = all_frameworks.release();
+ info->weak_frameworks = all_weak_frameworks.release();
+ info->has_framework_info = true;
+}
diff --git a/src/gn/resolved_target_data.h b/src/gn/resolved_target_data.h
index 13d7797..754abc2 100644
--- a/src/gn/resolved_target_data.h
+++ b/src/gn/resolved_target_data.h
@@ -75,6 +75,27 @@
return GetTargetLibInfo(target)->libs;
}
+ // The list of framework directories search paths to use at link time
+ // when generating macOS or iOS linkable binaries.
+ const std::vector<SourceDir>& GetLinkedFrameworkDirs(
+ const Target* target) const {
+ return GetTargetFrameworkInfo(target)->framework_dirs;
+ }
+
+ // The list of framework names to use at link time when generating macOS
+ // or iOS linkable binaries.
+ const std::vector<std::string>& GetLinkedFrameworks(
+ const Target* target) const {
+ return GetTargetFrameworkInfo(target)->frameworks;
+ }
+
+ // The list of weak framework names to use at link time when generating macOS
+ // or iOS linkable binaries.
+ const std::vector<std::string>& GetLinkedWeakFrameworks(
+ const Target* target) const {
+ return GetTargetFrameworkInfo(target)->weak_frameworks;
+ }
+
private:
// The information associated with a given Target pointer.
struct TargetInfo {
@@ -90,10 +111,16 @@
ResolvedTargetDeps deps;
bool has_lib_info = false;
+ bool has_framework_info = false;
// Only valid if |has_lib_info| is true.
std::vector<SourceDir> lib_dirs;
std::vector<LibFile> libs;
+
+ // Only valid if |has_framework_info| is true.
+ std::vector<SourceDir> framework_dirs;
+ std::vector<std::string> frameworks;
+ std::vector<std::string> weak_frameworks;
};
// Retrieve TargetInfo value associated with |target|. Create
@@ -109,10 +136,20 @@
return info;
}
+ const TargetInfo* GetTargetFrameworkInfo(const Target* target) const {
+ TargetInfo* info = GetTargetInfo(target);
+ if (!info->has_framework_info) {
+ ComputeFrameworkInfo(info);
+ DCHECK(info->has_framework_info);
+ }
+ return info;
+ }
+
// Compute the portion of TargetInfo guarded by one of the |has_xxx|
// booleans. This performs recursive and expensive computations and
// should only be called once per TargetInfo instance.
void ComputeLibInfo(TargetInfo* info) const;
+ void ComputeFrameworkInfo(TargetInfo* info) const;
// A { Target* -> TargetInfo } map that will create entries
// on demand (hence the mutable qualifier). Implemented with a
diff --git a/src/gn/resolved_target_data_unittest.cc b/src/gn/resolved_target_data_unittest.cc
index 440c604..5871d39 100644
--- a/src/gn/resolved_target_data_unittest.cc
+++ b/src/gn/resolved_target_data_unittest.cc
@@ -109,3 +109,61 @@
const auto& all_lib_dirs3 = resolved.GetLinkedLibraryDirs(&exec);
EXPECT_EQ(0u, all_lib_dirs3.size());
}
+
+// Tests that framework[_dir]s are inherited across deps boundaries for static
+// libraries but not executables.
+TEST(ResolvedTargetDataTest, FrameworkInheritance) {
+ TestWithScope setup;
+ Err err;
+
+ const std::string framework("Foo.framework");
+ const SourceDir frameworkdir("//out/foo/");
+
+ // Leaf target with ldflags set.
+ TestTarget z(setup, "//foo:z", Target::STATIC_LIBRARY);
+ z.config_values().frameworks().push_back(framework);
+ z.config_values().framework_dirs().push_back(frameworkdir);
+ ASSERT_TRUE(z.OnResolved(&err));
+
+ ResolvedTargetData resolved;
+
+ // All framework[_dir]s should be set when target is resolved.
+ const auto& frameworks = resolved.GetLinkedFrameworks(&z);
+ ASSERT_EQ(1u, frameworks.size());
+ EXPECT_EQ(framework, frameworks[0]);
+
+ const auto& framework_dirs = resolved.GetLinkedFrameworkDirs(&z);
+ ASSERT_EQ(1u, framework_dirs.size());
+ EXPECT_EQ(frameworkdir, framework_dirs[0]);
+
+ // Shared library target should inherit the libs from the static library
+ // and its own. Its own flag should be before the inherited one.
+ const std::string second_framework("Bar.framework");
+ const SourceDir second_frameworkdir("//out/bar/");
+ TestTarget shared(setup, "//foo:shared", Target::SHARED_LIBRARY);
+ shared.config_values().frameworks().push_back(second_framework);
+ shared.config_values().framework_dirs().push_back(second_frameworkdir);
+ shared.private_deps().push_back(LabelTargetPair(&z));
+ ASSERT_TRUE(shared.OnResolved(&err));
+
+ const auto& frameworks2 = resolved.GetLinkedFrameworks(&shared);
+ ASSERT_EQ(2u, frameworks2.size());
+ EXPECT_EQ(second_framework, frameworks2[0]);
+ EXPECT_EQ(framework, frameworks2[1]);
+
+ const auto& framework_dirs2 = resolved.GetLinkedFrameworkDirs(&shared);
+ ASSERT_EQ(2u, framework_dirs2.size());
+ EXPECT_EQ(second_frameworkdir, framework_dirs2[0]);
+ EXPECT_EQ(frameworkdir, framework_dirs2[1]);
+
+ // Executable target shouldn't get either by depending on shared.
+ TestTarget exec(setup, "//foo:exec", Target::EXECUTABLE);
+ exec.private_deps().push_back(LabelTargetPair(&shared));
+ ASSERT_TRUE(exec.OnResolved(&err));
+
+ const auto& frameworks3 = resolved.GetLinkedFrameworks(&exec);
+ EXPECT_EQ(0u, frameworks3.size());
+
+ const auto& framework_dirs3 = resolved.GetLinkedFrameworkDirs(&exec);
+ EXPECT_EQ(0u, framework_dirs3.size());
+}
diff --git a/src/gn/target.cc b/src/gn/target.cc
index 4d8cc56..0ad2dd2 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -483,23 +483,6 @@
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_framework_dirs_.Append(cur.framework_dirs().begin(),
- cur.framework_dirs().end());
- all_frameworks_.Append(cur.frameworks().begin(), cur.frameworks().end());
- all_weak_frameworks_.Append(cur.weak_frameworks().begin(),
- cur.weak_frameworks().end());
- }
-
PullRecursiveBundleData();
PullDependentTargetLibs();
PullRecursiveHardDeps();
@@ -856,13 +839,6 @@
}
}
}
-
- // Library settings are always inherited across static library boundaries.
- if (!dep->IsFinal() || dep->output_type() == STATIC_LIBRARY) {
- all_framework_dirs_.Append(dep->all_framework_dirs());
- all_frameworks_.Append(dep->all_frameworks());
- all_weak_frameworks_.Append(dep->all_weak_frameworks());
- }
}
void Target::PullDependentTargetLibs() {
diff --git a/src/gn/target.h b/src/gn/target.h
index c6f93c4..899e06b 100644
--- a/src/gn/target.h
+++ b/src/gn/target.h
@@ -332,16 +332,6 @@
return rust_transitive_inheritable_libs_;
}
- const UniqueVector<SourceDir>& all_framework_dirs() const {
- return all_framework_dirs_;
- }
- const UniqueVector<std::string>& all_frameworks() const {
- return all_frameworks_;
- }
- const UniqueVector<std::string>& all_weak_frameworks() const {
- return all_weak_frameworks_;
- }
-
const TargetSet& recursive_hard_deps() const { return recursive_hard_deps_; }
std::vector<LabelPattern>& friends() { return friends_; }
@@ -501,12 +491,6 @@
// that need to be linked.
InheritedLibraries inherited_libraries_;
- // These frameworks and dirs are inherited from statically linked deps and
- // all configs applying to this target.
- UniqueVector<SourceDir> all_framework_dirs_;
- UniqueVector<std::string> all_frameworks_;
- UniqueVector<std::string> all_weak_frameworks_;
-
// All hard deps from this target and all dependencies. Filled in when this
// target is marked resolved. This will not include the current target.
TargetSet recursive_hard_deps_;
diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc
index 7a3ca6e..6031fed 100644
--- a/src/gn/target_unittest.cc
+++ b/src/gn/target_unittest.cc
@@ -37,52 +37,6 @@
using TargetTest = TestWithScheduler;
-// Tests that framework[_dir]s are inherited across deps boundaries for static
-// libraries but not executables.
-TEST_F(TargetTest, FrameworkInheritance) {
- TestWithScope setup;
- Err err;
-
- const std::string framework("Foo.framework");
- const SourceDir frameworkdir("//out/foo/");
-
- // Leaf target with ldflags set.
- TestTarget z(setup, "//foo:z", Target::STATIC_LIBRARY);
- z.config_values().frameworks().push_back(framework);
- z.config_values().framework_dirs().push_back(frameworkdir);
- ASSERT_TRUE(z.OnResolved(&err));
-
- // All framework[_dir]s should be set when target is resolved.
- ASSERT_EQ(1u, z.all_frameworks().size());
- EXPECT_EQ(framework, z.all_frameworks()[0]);
- ASSERT_EQ(1u, z.all_framework_dirs().size());
- EXPECT_EQ(frameworkdir, z.all_framework_dirs()[0]);
-
- // Shared library target should inherit the libs from the static library
- // and its own. Its own flag should be before the inherited one.
- const std::string second_framework("Bar.framework");
- const SourceDir second_frameworkdir("//out/bar/");
- TestTarget shared(setup, "//foo:shared", Target::SHARED_LIBRARY);
- shared.config_values().frameworks().push_back(second_framework);
- shared.config_values().framework_dirs().push_back(second_frameworkdir);
- shared.private_deps().push_back(LabelTargetPair(&z));
- ASSERT_TRUE(shared.OnResolved(&err));
-
- ASSERT_EQ(2u, shared.all_frameworks().size());
- EXPECT_EQ(second_framework, shared.all_frameworks()[0]);
- EXPECT_EQ(framework, shared.all_frameworks()[1]);
- ASSERT_EQ(2u, shared.all_framework_dirs().size());
- EXPECT_EQ(second_frameworkdir, shared.all_framework_dirs()[0]);
- EXPECT_EQ(frameworkdir, shared.all_framework_dirs()[1]);
-
- // Executable target shouldn't get either by depending on shared.
- TestTarget exec(setup, "//foo:exec", Target::EXECUTABLE);
- exec.private_deps().push_back(LabelTargetPair(&shared));
- ASSERT_TRUE(exec.OnResolved(&err));
- EXPECT_EQ(0u, exec.all_frameworks().size());
- EXPECT_EQ(0u, exec.all_framework_dirs().size());
-}
-
// Test all_dependent_configs and public_config inheritance.
TEST_F(TargetTest, DependentConfigs) {
TestWithScope setup;
