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;