Change default module name in GN to be the full label. This guaruntees uniqueness of module names, preventing clashes. Bug: b:500845363 Change-Id: I62dba8b8754ab5f1b89410566177d4fe6a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/21680 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/src/gn/compile_commands_writer_unittest.cc b/src/gn/compile_commands_writer_unittest.cc index c645ac6..0011f84 100644 --- a/src/gn/compile_commands_writer_unittest.cc +++ b/src/gn/compile_commands_writer_unittest.cc
@@ -635,7 +635,10 @@ module_toolchain.ToolchainSetupComplete(); - Target module_target(&module_settings, Label(SourceDir("//foo/"), "module")); + Target module_target( + &module_settings, + Label(SourceDir("//foo/"), "module", module_toolchain.label().dir(), + module_toolchain.label().name())); module_target.set_output_type(Target::SOURCE_SET); module_target.visibility().SetPublic(); module_target.sources().push_back(SourceFile("//foo/foo.modulemap")); @@ -644,7 +647,9 @@ module_target.SetToolchain(&module_toolchain); ASSERT_TRUE(module_target.OnResolved(&err)); - Target dep_target(&module_settings, Label(SourceDir("//foo/"), "dep")); + Target dep_target(&module_settings, Label(SourceDir("//foo/"), "dep", + toolchain()->label().dir(), + toolchain()->label().name())); dep_target.set_output_type(Target::SOURCE_SET); dep_target.visibility().SetPublic(); dep_target.sources().push_back(SourceFile("//foo/dep.cc")); @@ -667,7 +672,8 @@ " \"file\": \"../../foo/foo.modulemap\",\r\n" " \"directory\": \"out/Debug\",\r\n" " \"command\": \"c++ ../../foo/foo.modulemap " - "-fmodule-name=module -c -x c++ -Xclang -emit-module -o " + "-fmodule-name=//foo:module(//toolchain:withmodules) -c -x c++ -Xclang " + "-emit-module -o " "withmodules/obj/foo/module.foo.pcm\"\r\n" " },\r\n" " {\r\n" @@ -675,7 +681,8 @@ " \"directory\": \"out/Debug\",\r\n" " \"command\": \"c++ ../../foo/dep.cc " "-fmodule-map-file=../../foo/foo.modulemap " - "-fmodule-file=module=withmodules/obj/foo/module.foo.pcm -o " + "-fmodule-file=//foo:module(//toolchain:withmodules)=withmodules/obj/foo/" + "module.foo.pcm -o " "withmodules/obj/foo/dep.dep.o\"\r\n" " }\r\n" "]\r\n"; @@ -686,7 +693,8 @@ " \"file\": \"../../foo/foo.modulemap\",\n" " \"directory\": \"out/Debug\",\n" " \"command\": \"c++ ../../foo/foo.modulemap " - "-fmodule-name=module -c -x c++ -Xclang -emit-module -o " + "-fmodule-name=//foo:module(//toolchain:withmodules) -c -x c++ -Xclang " + "-emit-module -o " "withmodules/obj/foo/module.foo.pcm\"\n" " },\n" " {\n" @@ -694,7 +702,8 @@ " \"directory\": \"out/Debug\",\n" " \"command\": \"c++ ../../foo/dep.cc " "-fmodule-map-file=../../foo/foo.modulemap " - "-fmodule-file=module=withmodules/obj/foo/module.foo.pcm -o " + "-fmodule-file=//foo:module(//toolchain:withmodules)=withmodules/obj/foo/" + "module.foo.pcm -o " "withmodules/obj/foo/dep.dep.o\"\n" " }\n" "]\n";
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc index 9041341..64f4ee9 100644 --- a/src/gn/ninja_c_binary_target_writer.cc +++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -196,7 +196,7 @@ &CSubstitutionModuleName)) { out_ << CSubstitutionModuleName.ninja_name << " = "; EscapeOptions options; - options.mode = ESCAPE_NINJA_COMMAND; + options.mode = ESCAPE_NINJA; EscapeStringToStream(out_, target_->module_name(), options); out_ << std::endl; }
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc index bf81b23..dae7692 100644 --- a/src/gn/ninja_c_binary_target_writer_unittest.cc +++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -2612,7 +2612,9 @@ EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } - Target target2(&module_settings, Label(SourceDir("//stuff/"), "b")); + Target target2(&module_settings, Label(SourceDir("//stuff/"), "b", + setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); target2.set_output_type(Target::STATIC_LIBRARY); target2.visibility().SetPublic(); target2.sources().push_back(SourceFile("//stuff/b.modulemap")); @@ -2635,8 +2637,8 @@ include_dirs = cflags = cflags_cc = -cc_module_name = b -module_deps = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=b=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm +cc_module_name = //stuff$:b(//toolchain$:default) +module_deps = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=//stuff:b(//toolchain:default)=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm module_deps_no_self = -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm root_out_dir = withmodules target_out_dir = obj/stuff @@ -2659,7 +2661,9 @@ EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } - Target target3(&module_settings, Label(SourceDir("//things/"), "c")); + Target target3(&module_settings, Label(SourceDir("//things/"), "c", + module_toolchain.label().dir(), + module_toolchain.label().name())); target3.set_output_type(Target::STATIC_LIBRARY); target3.visibility().SetPublic(); target3.sources().push_back(SourceFile("//stuff/c.modulemap")); @@ -2680,9 +2684,9 @@ include_dirs = cflags = cflags_cc = -cc_module_name = c -module_deps = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=b=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm -fmodule-map-file=../../stuff/c.modulemap -fmodule-file=c=obj/stuff/libc.c.pcm -module_deps_no_self = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=b=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm +cc_module_name = //things$:c +module_deps = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=//stuff:b(//toolchain:default)=obj/stuff/libb.b.pcm -fmodule-map-file=../../stuff/c.modulemap -fmodule-file=//things:c=obj/stuff/libc.c.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm +module_deps_no_self = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=//stuff:b(//toolchain:default)=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm root_out_dir = withmodules target_out_dir = obj/things target_output_name = libc @@ -2701,7 +2705,9 @@ EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } - Target depender(&module_settings, Label(SourceDir("//zap/"), "c")); + Target depender(&module_settings, Label(SourceDir("//zap/"), "c", + setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); depender.set_output_type(Target::EXECUTABLE); depender.sources().push_back(SourceFile("//zap/x.cc")); depender.sources().push_back(SourceFile("//zap/y.cc")); @@ -2720,9 +2726,9 @@ include_dirs = cflags = cflags_cc = -cc_module_name = c -module_deps = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=b=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm -module_deps_no_self = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=b=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm +cc_module_name = //zap$:c(//toolchain$:default) +module_deps = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=//stuff:b(//toolchain:default)=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm +module_deps_no_self = -fmodule-map-file=../../stuff/b.modulemap -fmodule-file=//stuff:b(//toolchain:default)=obj/stuff/libb.b.pcm -fmodule-map-file=../../blah/a.modulemap -fmodule-file=blah_a=obj/blah/liba.a.pcm root_out_dir = withmodules target_out_dir = obj/zap target_output_name = c @@ -2922,7 +2928,9 @@ TestWithScope setup; // Let's create a target and give it public headers. - Target target(setup.settings(), Label(SourceDir("//foo/"), "bar")); + Target target(setup.settings(), Label(SourceDir("//foo/"), "bar", + setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); target.set_output_type(Target::SOURCE_SET); target.visibility().SetPublic(); target.sources().push_back(SourceFile("//foo/source1.cc")); @@ -2941,7 +2949,7 @@ writer.WriteModuleMap(modulemap_out, out_dir); const char expected_modulemap[] = - "module \"bar\" {\n" + "module \"//foo:bar\" {\n" " textual header \"../../../foo/public_header.h\"\n" " export *\n" "}\n"; @@ -2970,8 +2978,10 @@ EXPECT_EQ(expected_ninja, ninja_str) << expected_ninja << "\n" << ninja_str; // Test generation without explicit public headers (uses sources instead) - Target target_no_public(setup.settings(), - Label(SourceDir("//foo/"), "no_public")); + Target target_no_public( + setup.settings(), + Label(SourceDir("//foo/"), "no_public", setup.toolchain()->label().dir(), + setup.toolchain()->label().name())); target_no_public.set_output_type(Target::SOURCE_SET); target_no_public.visibility().SetPublic(); target_no_public.sources().push_back(SourceFile("//foo/source1.cc")); @@ -2991,7 +3001,7 @@ writer_no_public.WriteModuleMap(modulemap_out_no_public, out_dir); const char expected_modulemap_no_public[] = - "module \"no_public\" {\n" + "module \"//foo:no_public\" {\n" " textual header \"../../../foo/header1.h\"\n" " export *\n" "}\n";
diff --git a/src/gn/target.cc b/src/gn/target.cc index f2130e6..54281b3 100644 --- a/src/gn/target.cc +++ b/src/gn/target.cc
@@ -405,7 +405,9 @@ Target::Target(const Settings* settings, const Label& label, const SourceFileSet& build_dependency_files) - : Item(settings, label, build_dependency_files) {} + : Item(settings, label, build_dependency_files), + module_name_( + label.GetUserVisibleName(settings->default_toolchain_label())) {} Target::~Target() = default;
diff --git a/src/gn/target.h b/src/gn/target.h index f219402..ddaf2b5 100644 --- a/src/gn/target.h +++ b/src/gn/target.h
@@ -22,6 +22,7 @@ #include "gn/output_file.h" #include "gn/pointer_set.h" #include "gn/rust_values.h" +#include "gn/settings.h" #include "gn/source_file.h" #include "gn/swift_values.h" #include "gn/toolchain.h" @@ -443,11 +444,10 @@ // The module name for the target. std::string module_name() const { - return module_name_override_.empty() ? label().name() - : module_name_override_; + return module_name_; } void set_module_name(std::string module_name) { - module_name_override_ = std::move(module_name); + module_name_ = std::move(module_name); } // Computes and returns the outputs of this target expressed as SourceFiles. @@ -537,7 +537,7 @@ std::string output_extension_; bool output_extension_set_ = false; - std::string module_name_override_; + std::string module_name_; ModuleType module_type_ = NO_MODULEMAP; // Only filled if the module type is GENERATED_* SourceFile generated_modulemap_file_;