Put dependencies in modulemaps This consists of two parts: * The `use` declaration specifies that it is allowed to access (ie. has a dependency on) another module * The `extern module` declaration ensures that `-fmodule-map-file` is only required for the root modulemap file. Bug: b:500845363 Change-Id: Id75e0a93ffbd0ef89bc69000afb5a6276a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/21801 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc index 13bc73d..03a92b5 100644 --- a/src/gn/ninja_binary_target_writer.cc +++ b/src/gn/ninja_binary_target_writer.cc
@@ -4,7 +4,9 @@ #include "gn/ninja_binary_target_writer.h" +#include <algorithm> #include <sstream> +#include <unordered_set> #include "base/strings/string_util.h" #include "gn/builtin_tool.h" @@ -33,6 +35,61 @@ return opts; } +std::vector<const Target*> ExpandModules(const LabelTargetVector& targets) { + std::vector<const LabelTargetVector*> stack = {&targets}; + std::unordered_set<const Target*> visited; + + std::vector<const Target*> modules; + + while (!stack.empty()) { + const LabelTargetVector* current = stack.back(); + stack.pop_back(); + for (const auto& pair : *current) { + const Target* target = pair.ptr; + if (visited.insert(target).second) { + if (target->module_type() == Target::NO_MODULEMAP) { + stack.push_back(&target->public_deps()); + // If you declare `public_deps = ...` on a group, it shows up as a + // private dep. Probably because groups don't distinguish between + // public and private deps. + if (target->output_type() == Target::GROUP) { + stack.push_back(&target->private_deps()); + } + } else { + modules.push_back(target); + } + } + } + } + return modules; +} + +void WriteModuleMapHeaders(std::ostream& out, + const SourceDir& out_dir, + const Target::FileList& headers, + const Settings* settings) { + for (const auto& header : headers) { + if (header.GetType() == SourceFile::SOURCE_H) { + out << " textual header \""; + out << RebasePath(header.value(), out_dir, + settings->build_settings()->root_path_utf8()); + out << "\"\n"; + } + } +} + +void WriteModuleDeps(std::ostream& out, + const std::vector<const Target*>& deps, + const SourceDir& base) { + for (const auto& dep : deps) { + auto module_name = dep->module_name(); + auto modulemap = RebasePath(dep->modulemap_file()->value(), base); + out << " extern module \"" << module_name << "\" \"" << modulemap + << "\"\n"; + out << " use \"" << module_name << "\"\n"; + } +} + } // namespace NinjaBinaryTargetWriter::NinjaBinaryTargetWriter(const Target* target, @@ -61,21 +118,16 @@ const SourceDir& out_dir) { out << "module \"" << target_->module_name() << "\" {\n"; if (target_->all_headers_public()) { - for (const auto& header : target_->sources()) { - if (header.GetType() == SourceFile::SOURCE_H) { - out << " textual header \""; - out << RebasePath(header.value(), out_dir, - settings_->build_settings()->root_path_utf8()); - out << "\"\n"; - } - } + WriteModuleMapHeaders(out, out_dir, target_->sources(), settings_); + } else { + WriteModuleMapHeaders(out, out_dir, target_->public_headers(), settings_); } - for (const auto& header : target_->public_headers()) { - out << " textual header \""; - out << RebasePath(header.value(), out_dir, - settings_->build_settings()->root_path_utf8()); - out << "\"\n"; - } + auto base = target_->modulemap_file()->GetDir(); + auto deps = ExpandModules(target_->public_deps()); + std::ranges::sort(deps, [](const Target* lhs, const Target* rhs) { + return lhs->module_name() < rhs->module_name(); + }); + WriteModuleDeps(out, deps, base); out << " export *\n}\n"; }
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc index ccda8f9..6b36f91 100644 --- a/src/gn/ninja_c_binary_target_writer_unittest.cc +++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -2934,7 +2934,9 @@ target.set_output_type(Target::SOURCE_SET); target.visibility().SetPublic(); target.sources().push_back(SourceFile("//foo/source1.cc")); + target.sources().push_back(SourceFile("//foo/private_header.h")); target.public_headers().push_back(SourceFile("//foo/public_header.h")); + target.set_all_headers_public(false); target.source_types_used().Set(SourceFile::SOURCE_CPP); target.set_module_type(Target::GENERATED_TEXTUAL_MODULEMAP); target.SetToolchain(setup.toolchain()); @@ -3010,4 +3012,79 @@ EXPECT_EQ(expected_modulemap_no_public, modulemap_str_no_public) << expected_modulemap_no_public << "\n" << modulemap_str_no_public; + + Target transitive(setup.settings(), Label(SourceDir("//foo/"), "transitive", + setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); + transitive.visibility().SetPublic(); + transitive.sources().push_back(SourceFile("//foo/invisible.h")); + transitive.source_types_used().Set(SourceFile::SOURCE_H); + transitive.set_output_type(Target::SOURCE_SET); + transitive.set_module_type(Target::GENERATED_TEXTUAL_MODULEMAP); + transitive.SetToolchain(setup.toolchain()); + ASSERT_TRUE(transitive.OnResolved(&err)); + + Target private_dep(setup.settings(), + Label(SourceDir("//private/"), "private_dep", + setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); + private_dep.visibility().SetPublic(); + private_dep.set_output_type(Target::SOURCE_SET); + private_dep.sources().push_back(SourceFile("//foo/private_dep.h")); + private_dep.source_types_used().Set(SourceFile::SOURCE_H); + private_dep.public_deps().push_back(LabelTargetPair(&transitive)); + private_dep.set_module_type(Target::GENERATED_TEXTUAL_MODULEMAP); + private_dep.SetToolchain(setup.toolchain()); + ASSERT_TRUE(private_dep.OnResolved(&err)); + + Target public_dep(setup.settings(), Label(SourceDir("//foo/"), "public_dep", + setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); + public_dep.visibility().SetPublic(); + public_dep.set_output_type(Target::SOURCE_SET); + public_dep.sources().push_back(SourceFile("//foo/public_dep.h")); + public_dep.private_deps().push_back(LabelTargetPair(&transitive)); + public_dep.public_deps().push_back(LabelTargetPair(&transitive)); + public_dep.set_module_type(Target::GENERATED_TEXTUAL_MODULEMAP); + public_dep.SetToolchain(setup.toolchain()); + ASSERT_TRUE(public_dep.OnResolved(&err)); + + Target group_dep(setup.settings(), Label(SourceDir("//foo/"), "group_dep", + setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); + group_dep.set_output_type(Target::GROUP); + group_dep.visibility().SetPublic(); + group_dep.private_deps().push_back(LabelTargetPair(&public_dep)); + group_dep.SetToolchain(setup.toolchain()); + ASSERT_TRUE(group_dep.OnResolved(&err)); + + Target root(setup.settings(), Label(SourceDir("//foo/"), "root", + setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); + root.set_output_type(Target::SOURCE_SET); + root.visibility().SetPublic(); + root.sources().push_back(SourceFile("//foo/root.cc")); + root.sources().push_back(SourceFile("//foo/private_header.h")); + root.public_headers().push_back(SourceFile("//foo/public_header.h")); + root.set_all_headers_public(false); + root.source_types_used().Set(SourceFile::SOURCE_H); + root.public_deps().push_back(LabelTargetPair(&group_dep)); + root.private_deps().push_back(LabelTargetPair(&private_dep)); + root.set_module_type(Target::GENERATED_TEXTUAL_MODULEMAP); + root.SetToolchain(setup.toolchain()); + ASSERT_TRUE(root.OnResolved(&err)); + + std::ostringstream public_modulemap; + NinjaCBinaryTargetWriter(&root, ninja_out) + .WriteModuleMap(public_modulemap, out_dir); + + const char expected_public[] = + "module \"//foo:root\" {\n" + " textual header \"../../../foo/public_header.h\"\n" + " extern module \"//foo:public_dep\" \"public_dep.modulemap\"\n" + " use \"//foo:public_dep\"\n" + " export *\n" + "}\n"; + EXPECT_EQ(expected_public, public_modulemap.str()) << expected_public << "\n" + << public_modulemap.str(); }