Support public_inputs in group targets This change allows `group` targets to declare `public_inputs`. When a target defines `public_inputs`, the files are treated as implicit inputs of dependent targets (direct dependents and transitively via `public_deps`). Allowing `public_inputs` on `group` targets makes it possible to aggregate generated files (e.g. `*.d.ts` declaration files, tsconfig refs) and export them as public inputs to downstream consumers without defining dummy stamp actions or creating self-dependency cycles in actions. I'd like to use this to simplify GN config like https://crrev.com/c/8303334/10/scripts/build/typescript/ts_library_split.gni Bug: 513105742, 547522497 Change-Id: I4cdb23f64478a268a975b4834fbc79edf0ea8a5b Reviewed-on: https://gn-review.googlesource.com/c/gn/+/25780 Reviewed-by: David Turner <digit@google.com> Reviewed-by: Matt Stark <msta@google.com> Commit-Queue: Takuto Ikuta <tikuta@google.com>
diff --git a/docs/reference.md b/docs/reference.md index 41bd09f..4194378 100644 --- a/docs/reference.md +++ b/docs/reference.md
@@ -1644,8 +1644,8 @@ visibility Action variables: args, bridge_header, configs, data, depfile, framework_dirs, inputs, mnemonic, module_deps, - module_name, outputs*, pool, response_file_contents, - script*, sources + module_name, outputs*, pool, public_inputs, + response_file_contents, script*, sources * = required ``` @@ -1745,8 +1745,8 @@ visibility Action variables: args, bridge_header, configs, data, depfile, framework_dirs, inputs, mnemonic, module_deps, - module_name, outputs*, pool, response_file_contents, - script*, sources + module_name, outputs*, pool, public_inputs, + response_file_contents, script*, sources * = required ``` @@ -2254,6 +2254,7 @@ #### **Variables** ``` + Group variables: public_inputs Deps: assert_no_deps, data_deps, deps, public_deps, runtime_deps, write_runtime_deps Dependent configs: all_dependent_configs, public_configs @@ -7058,7 +7059,9 @@ propagate to any targets depending on B. This is particularly useful for actions that generate source code which - contain implicit imports/includes of the files declared in public_inputs. + contain implicit imports/includes of the files declared in public_inputs, + or for groups that aggregate and export generated files as public inputs + to downstream consumers. Dependent targets will automatically inherit these dependencies and trigger rebuilds when the public inputs change. @@ -7081,6 +7084,11 @@ public_deps = [ ":A" ] # C inherits "a.in", and propagates it to # C's dependents. } + + group("my_module") { + public_deps = [ ":generate_dts" ] + public_inputs = [ "$target_gen_dir/foo.d.ts" ] + } ``` ### <a name="var_rebase"></a>**rebase**: Rebase collected metadata as files. [Back to Top](#gn-reference)
diff --git a/src/gn/action_target_generator.cc b/src/gn/action_target_generator.cc index 4a7b2b7..443d18c 100644 --- a/src/gn/action_target_generator.cc +++ b/src/gn/action_target_generator.cc
@@ -263,17 +263,3 @@ target_->config_values().inputs().swap(dest_inputs); return true; } - -bool ActionTargetGenerator::FillPublicInputs() { - const Value* value = scope_->GetValue(variables::kPublicInputs, true); - if (!value) - return true; - - Target::FileList dest_public_inputs; - if (!ExtractListOfRelativeFiles(scope_->settings()->build_settings(), *value, - scope_->GetSourceDir(), &dest_public_inputs, - err_)) - return false; - target_->public_inputs().swap(dest_public_inputs); - return true; -}
diff --git a/src/gn/action_target_generator.h b/src/gn/action_target_generator.h index 61e11df..6cbe8ae 100644 --- a/src/gn/action_target_generator.h +++ b/src/gn/action_target_generator.h
@@ -29,7 +29,6 @@ bool FillMnemonic(); bool FillPool(); bool FillInputs(); - bool FillPublicInputs(); // Checks for errors in the outputs variable. bool CheckOutputs();
diff --git a/src/gn/functions_target.cc b/src/gn/functions_target.cc index 9a19c41..e50fc9b 100644 --- a/src/gn/functions_target.cc +++ b/src/gn/functions_target.cc
@@ -25,11 +25,11 @@ #define RUST_VARS " Rust variables: aliased_deps, crate_root, crate_name\n" #define RUST_SHARED_VARS \ " Rust variables: aliased_deps, crate_root, crate_name, crate_type\n" -#define ACTION_VARS \ - " Action variables: args, bridge_header, configs, data, depfile,\n" \ - " framework_dirs, inputs, mnemonic, module_deps,\n" \ - " module_name, outputs*, pool, response_file_contents,\n" \ - " script*, sources\n" +#define ACTION_VARS \ + " Action variables: args, bridge_header, configs, data, depfile,\n" \ + " framework_dirs, inputs, mnemonic, module_deps,\n" \ + " module_name, outputs*, pool, public_inputs,\n" \ + " response_file_contents, script*, sources\n" namespace functions { @@ -648,6 +648,7 @@ Variables + Group variables: public_inputs )" DEPS_VARS DEPENDENT_CONFIG_VARS GENERAL_TARGET_VARS R"(
diff --git a/src/gn/functions_target_unittest.cc b/src/gn/functions_target_unittest.cc index 4a397c9..3d66b68 100644 --- a/src/gn/functions_target_unittest.cc +++ b/src/gn/functions_target_unittest.cc
@@ -204,3 +204,25 @@ ASSERT_TRUE(err.has_error()); ASSERT_EQ(err.message(), "More than one language used in target sources."); } + +TEST_F(FunctionsTarget, GroupPublicInputs) { + TestWithScope setup; + + Scope::ItemVector item_collector; + setup.scope()->set_item_collector(&item_collector); + + TestParseInput input( + R"(group("foo") { + public_inputs = [ "//bar.in" ] + })"); + ASSERT_FALSE(input.has_error()); + Err err; + input.parsed()->Execute(setup.scope(), &err); + ASSERT_SUCCESS(err); + + ASSERT_EQ(1u, item_collector.size()); + const Target* target = item_collector[0]->AsTarget(); + ASSERT_TRUE(target); + ASSERT_EQ(1u, target->public_inputs().size()); + EXPECT_EQ("//bar.in", target->public_inputs()[0].value()); +}
diff --git a/src/gn/group_target_generator.cc b/src/gn/group_target_generator.cc index 2b91066..0a4631f 100644 --- a/src/gn/group_target_generator.cc +++ b/src/gn/group_target_generator.cc
@@ -18,6 +18,7 @@ void GroupTargetGenerator::DoRun() { target_->set_output_type(Target::GROUP); - // Groups only have the default types filled in by the target generator - // base class. + + if (!FillPublicInputs()) + return; }
diff --git a/src/gn/ninja_target_writer_unittest.cc b/src/gn/ninja_target_writer_unittest.cc index ac5b154..53b1a4d 100644 --- a/src/gn/ninja_target_writer_unittest.cc +++ b/src/gn/ninja_target_writer_unittest.cc
@@ -712,3 +712,51 @@ EXPECT_FALSE(out.contains("../../foo/a.in")) << out; } } + +TEST(NinjaTargetWriter, GroupPublicInputs) { + TestWithScope setup; + Err err; + + // Group G has public_inputs. + Target g(setup.settings(), Label(SourceDir("//foo/"), "g")); + g.set_output_type(Target::GROUP); + g.visibility().SetPublic(); + g.SetToolchain(setup.toolchain()); + g.public_inputs().push_back(SourceFile("//foo/g.in")); + + // Action B depends on G. + Target b(setup.settings(), Label(SourceDir("//foo/"), "b")); + b.set_output_type(Target::ACTION); + b.visibility().SetPublic(); + b.SetToolchain(setup.toolchain()); + b.action_values().set_script(SourceFile("//foo/script.py")); + b.private_deps().push_back(LabelTargetPair(&g)); + + ASSERT_TRUE(g.OnResolved(&err)); + ASSERT_TRUE(b.OnResolved(&err)); + + // 1. Verify G's public_inputs phony/stamp target is written. + { + std::ostringstream stream; + ResolvedTargetData resolved; + TestingNinjaTargetWriter::WritePublicInputsStampOrPhony(&g, &resolved, + stream); + EXPECT_EQ("build phony/foo/g.public_inputs: phony ../../foo/g.in\n\n", + stream.str()); + } + + // 2. Verify B's input deps. It should depend on G's public_inputs. + { + std::ostringstream stream; + TestingNinjaTargetWriter writer(&b, setup.toolchain(), stream); + auto dep = writer.WriteInputDepsStampOrPhonyAndGetDep( + std::vector<const Target*>(), 10u); + + ASSERT_EQ(1u, dep.implicit.size()); + EXPECT_EQ("phony/foo/b.inputdeps", dep.implicit[0].value()); + + std::string out = stream.str(); + EXPECT_TRUE(out.contains("phony/foo/g.public_inputs")) << out; + EXPECT_FALSE(out.contains("../../foo/g.in")) << out; + } +}
diff --git a/src/gn/resolved_target_data_unittest.cc b/src/gn/resolved_target_data_unittest.cc index be3e950..49dc623 100644 --- a/src/gn/resolved_target_data_unittest.cc +++ b/src/gn/resolved_target_data_unittest.cc
@@ -490,3 +490,43 @@ // E has B as a private dependency, B has no public inputs, so E has none. EXPECT_FALSE(resolved.ExportsPublicInputs(&e)); } + +TEST(ResolvedTargetDataTest, GroupPublicInputsInheritance) { + TestWithScope setup; + Err err; + + // Group G has public_inputs. + TestTarget g(setup, "//foo:g", Target::GROUP); + g.public_inputs().push_back(SourceFile("//foo/g.in")); + + // Action A depends on G via private deps. + TestTarget a(setup, "//foo:a", Target::ACTION); + a.private_deps().push_back(LabelTargetPair(&g)); + + // Group G2 depends on G via public deps. + TestTarget g2(setup, "//foo:g2", Target::GROUP); + g2.public_deps().push_back(LabelTargetPair(&g)); + + // Action A2 depends on G2 via private deps. + TestTarget a2(setup, "//foo:a2", Target::ACTION); + a2.private_deps().push_back(LabelTargetPair(&g2)); + + ASSERT_TRUE(g.OnResolved(&err)); + ASSERT_TRUE(a.OnResolved(&err)); + ASSERT_TRUE(g2.OnResolved(&err)); + ASSERT_TRUE(a2.OnResolved(&err)); + + ResolvedTargetData resolved; + + // G has public_inputs directly. + EXPECT_TRUE(resolved.ExportsPublicInputs(&g)); + + // A has G as private dep, so A does not export public_inputs. + EXPECT_FALSE(resolved.ExportsPublicInputs(&a)); + + // G2 has G as public dep, so G2 exports public_inputs. + EXPECT_TRUE(resolved.ExportsPublicInputs(&g2)); + + // A2 has G2 as private dep, so A2 does not export public_inputs. + EXPECT_FALSE(resolved.ExportsPublicInputs(&a2)); +}
diff --git a/src/gn/target_generator.cc b/src/gn/target_generator.cc index 69cbaea..85ddf6e 100644 --- a/src/gn/target_generator.cc +++ b/src/gn/target_generator.cc
@@ -213,6 +213,20 @@ return true; } +bool TargetGenerator::FillPublicInputs() { + const Value* value = scope_->GetValue(variables::kPublicInputs, true); + if (!value) + return true; + + Target::FileList dest_public_inputs; + if (!ExtractListOfRelativeFiles(scope_->settings()->build_settings(), *value, + scope_->GetSourceDir(), &dest_public_inputs, + err_)) + return false; + target_->public_inputs().swap(dest_public_inputs); + return true; +} + bool TargetGenerator::FillConfigs() { return FillGenericConfigs(variables::kConfigs, &target_->configs()); }
diff --git a/src/gn/target_generator.h b/src/gn/target_generator.h index 442e9a8..bef5ddf 100644 --- a/src/gn/target_generator.h +++ b/src/gn/target_generator.h
@@ -48,6 +48,7 @@ virtual bool FillSources(); bool FillPublic(); + bool FillPublicInputs(); bool FillConfigs(); bool FillOutputs(bool allow_substitutions); bool FillCheckIncludes();
diff --git a/src/gn/variables.cc b/src/gn/variables.cc index 3564369..f90a758 100644 --- a/src/gn/variables.cc +++ b/src/gn/variables.cc
@@ -1446,7 +1446,9 @@ propagate to any targets depending on B. This is particularly useful for actions that generate source code which - contain implicit imports/includes of the files declared in public_inputs. + contain implicit imports/includes of the files declared in public_inputs, + or for groups that aggregate and export generated files as public inputs + to downstream consumers. Dependent targets will automatically inherit these dependencies and trigger rebuilds when the public inputs change. @@ -1469,6 +1471,11 @@ public_deps = [ ":A" ] # C inherits "a.in", and propagates it to # C's dependents. } + + group("my_module") { + public_deps = [ ":generate_dts" ] + public_inputs = [ "$target_gen_dir/foo.d.ts" ] + } )"; const char kLdflags[] = "ldflags";