Consider target outputs as existing files in `gn suggest`. When suggesting dependency changes via `gn suggest`, a file in the output directory is considered to exist if it is output by a target in the build graph, even if it has not yet been written to disk. This enables suggesting dependencies on targets that produce generated header files. When multiple targets are found (e.g. an action target producing the file and a source_set target exposing it in headers), we prefer the consumer target to avoid suggesting direct dependencies on raw action targets. Bug: 500845363 Change-Id: I46ce193aba7f439925921df97a16385e6a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/22980 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/src/gn/command_suggest.cc b/src/gn/command_suggest.cc index 9b7ca28..f9f9f15 100644 --- a/src/gn/command_suggest.cc +++ b/src/gn/command_suggest.cc
@@ -64,6 +64,11 @@ return commands::ApiScope::kPublic; } } + for (const auto& output : target->computed_outputs()) { + if (output.AsSourceFile(target->settings()->build_settings()) == file) { + return commands::ApiScope::kOutput; + } + } return std::nullopt; } @@ -85,11 +90,36 @@ return !results.empty(); } +bool FileExists(const std::vector<const Target*>& all_targets, + const SourceFile& file, + const BuildSettings* build_settings) { + base::FilePath build_dir_path = + build_settings->GetFullPath(build_settings->build_dir()); + base::FilePath file_path = build_settings->GetFullPath(file); + + if (build_dir_path.IsParent(file_path)) { + // It's in the output directory, so check if it was generated by a target. + OutputFile target_file(build_settings, file); + for (const Target* target : all_targets) { + for (const OutputFile& output : target->computed_outputs()) { + if (output == target_file) { + return true; + } + } + } + return false; + } else { + // It's a source file, so check if it exists on disk. + return base::PathExists(file_path); + } +} + SourceFile ResolveFilePath(const BuildSettings* build_settings, + const std::vector<const Target*>& all_targets, std::string_view input) { if (input.starts_with("//")) { SourceFile file = SourceFile(input); - if (base::PathExists(build_settings->GetFullPath(file))) { + if (FileExists(all_targets, file, build_settings)) { return file; } return SourceFile(); @@ -101,7 +131,7 @@ Err err; SourceFile file = build_settings->build_dir().ResolveRelativeFile( Value(nullptr, std::string(input)), &err); - if (!err.has_error() && base::PathExists(build_settings->GetFullPath(file))) { + if (!err.has_error() && FileExists(all_targets, file, build_settings)) { return file; } return SourceFile(); @@ -243,7 +273,7 @@ } // If that doesn't work, try to resolve as a file path. - SourceFile file = ResolveFilePath(build_settings, input); + SourceFile file = ResolveFilePath(build_settings, all_targets, input); if (file.is_null()) { return {results, false}; } @@ -253,6 +283,19 @@ if (!AddToolchainSources(all_targets, ¤t_toolchain, file, results)) { AddToolchainSources(all_targets, nullptr, file, results); } + // If we have an action that generates "gen/foo.h", we should prefer + // depending on the source set that declares it as a header. + if (std::ranges::all_of(results, [](const auto& result) { + return result.second == commands::ApiScope::kOutput; + })) { + for (auto& result : results) { + result.second = commands::ApiScope::kPublic; + } + } else { + std::erase_if(results, [](const auto& result) { + return result.second == commands::ApiScope::kOutput; + }); + } sort_results(results); return {results, true}; }
diff --git a/src/gn/command_suggest_unittest.cc b/src/gn/command_suggest_unittest.cc index edf342d..28131b2 100644 --- a/src/gn/command_suggest_unittest.cc +++ b/src/gn/command_suggest_unittest.cc
@@ -146,8 +146,29 @@ simple_secondary.public_headers().push_back( SourceFile("//secondary_toolchain.h")); + Target generated(setup_scope.settings(), + Label(SourceDir("//"), "generated", current_toolchain.dir(), + current_toolchain.name())); + generated.set_output_type(Target::ACTION); + generated.SetToolchain(setup_scope.toolchain()); + generated.action_values().outputs() = + SubstitutionList::MakeForTest("//out/Debug/generated_file.h"); + Err resolve_err; + ASSERT_TRUE(generated.OnResolvedWithoutChecks(&resolve_err)); + + Target consumer(setup_scope.settings(), + Label(SourceDir("//"), "consumer", current_toolchain.dir(), + current_toolchain.name())); + consumer.set_output_type(Target::SOURCE_SET); + consumer.SetToolchain(setup_scope.toolchain()); + consumer.set_all_headers_public(true); + consumer.public_headers().push_back( + SourceFile("//out/Debug/generated_file.h")); + ASSERT_TRUE(consumer.OnResolvedWithoutChecks(&resolve_err)); + std::vector<const Target*> all_targets = {&explicit_target, &implicit_target, - &simple_default, &simple_secondary}; + &simple_default, &simple_secondary, + &generated}; { auto [results, ok] = commands::ResolveSuggestionToTarget( @@ -189,6 +210,27 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, + "//out/Debug/generated_file.h"); + std::vector<std::pair<const Target*, commands::ApiScope>> expected = { + {&generated, commands::ApiScope::kPublic}}; + EXPECT_TRUE(ok); + EXPECT_EQ(expected, results); + } + + all_targets.push_back(&consumer); + { + auto [results, ok] = commands::ResolveSuggestionToTarget( + setup_scope.build_settings(), all_targets, current_toolchain, + "//out/Debug/generated_file.h"); + std::vector<std::pair<const Target*, commands::ApiScope>> expected = { + {&consumer, commands::ApiScope::kPublic}}; + EXPECT_TRUE(ok); + EXPECT_EQ(expected, results); + } + + { + auto [results, ok] = commands::ResolveSuggestionToTarget( + setup_scope.build_settings(), all_targets, current_toolchain, "//no_target.h"); std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets; EXPECT_TRUE(ok);
diff --git a/src/gn/commands.h b/src/gn/commands.h index 62d0ffb..c25ba85 100644 --- a/src/gn/commands.h +++ b/src/gn/commands.h
@@ -276,6 +276,7 @@ enum class ApiScope { kPublic, kPrivate, + kOutput, }; // Resolves an input to a list of targets for suggestion.