[visibility] Consider configs in visibility check Bug: 22 Change-Id: I4fa6286ade5a0d612f12f417c07e4319253061a0 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/9740 Reviewed-by: Shai Barack <shayba@google.com> Reviewed-by: Brett Wilson <brettw@chromium.org> Reviewed-by: Petr Hosek <phosek@google.com> Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/config_values_extractors_unittest.cc b/src/gn/config_values_extractors_unittest.cc index de9e63a..7c520ad 100644 --- a/src/gn/config_values_extractors_unittest.cc +++ b/src/gn/config_values_extractors_unittest.cc
@@ -81,12 +81,14 @@ // Set up target, direct and all dependent configs. Config target_all(setup.settings(), Label(SourceDir("//target/"), "all")); + target_all.visibility().SetPublic(); target_all.own_values().cflags().push_back("--target-all"); target_all.own_values().include_dirs().push_back(SourceDir("//target/all/")); ASSERT_TRUE(target_all.OnResolved(&err)); Config target_direct(setup.settings(), Label(SourceDir("//target/"), "direct")); + target_direct.visibility().SetPublic(); target_direct.own_values().cflags().push_back("--target-direct"); target_direct.own_values().include_dirs().push_back( SourceDir("//target/direct/")); @@ -95,6 +97,7 @@ // This config is applied directly to target. Config target_config(setup.settings(), Label(SourceDir("//target/"), "config")); + target_config.visibility().SetPublic(); target_config.own_values().cflags().push_back("--target-config"); target_config.own_values().include_dirs().push_back( SourceDir("//target/config/"));
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc index 391d1d7..542221a 100644 --- a/src/gn/ninja_c_binary_target_writer_unittest.cc +++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -527,6 +527,7 @@ // A config that force linking with the framework. Config framework_config(setup.settings(), Label(SourceDir("//bar"), "framework_config")); + framework_config.visibility().SetPublic(); framework_config.own_values().frameworks().push_back("Bar.framework"); framework_config.own_values().framework_dirs().push_back( SourceDir("//out/Debug/")); @@ -1222,6 +1223,7 @@ ASSERT_TRUE(far_config.OnResolved(&err)); Config config(setup.settings(), Label(SourceDir("//foo/"), "baz")); + config.visibility().SetPublic(); config.own_values().inputs().push_back(SourceFile("//foo/input2.data")); config.configs().push_back(LabelConfigPair(&far_config)); ASSERT_TRUE(config.OnResolved(&err));
diff --git a/src/gn/target.cc b/src/gn/target.cc index 39bf492..b8bf2f9 100644 --- a/src/gn/target.cc +++ b/src/gn/target.cc
@@ -338,6 +338,11 @@ ScopedTrace trace(TraceItem::TRACE_ON_RESOLVED, label()); trace.SetToolchain(settings()->toolchain_label()); + // Check visibility for just this target's own configs, before dependents are + // added. + if (!CheckConfigVisibility(err)) + return false; + // Copy this target's own dependent and public configs to the list of configs // applying to it. configs_.Append(all_dependent_configs_.begin(), all_dependent_configs_.end()); @@ -962,6 +967,15 @@ return true; } +bool Target::CheckConfigVisibility(Err* err) const { + for (ConfigValuesIterator iter(this); !iter.done(); iter.Next()) { + if (const Config* config = iter.GetCurrentConfig()) + if (!Visibility::CheckItemVisibility(this, config, err)) + return false; + } + return true; +} + bool Target::CheckSourceSetLanguages(Err* err) const { if (output_type() == Target::SOURCE_SET && source_types_used().RustSourceUsed()) {
diff --git a/src/gn/target.h b/src/gn/target.h index 336c20c..dbcfa69 100644 --- a/src/gn/target.h +++ b/src/gn/target.h
@@ -411,6 +411,7 @@ // Validates the given thing when a target is resolved. bool CheckVisibility(Err* err) const; + bool CheckConfigVisibility(Err* err) const; bool CheckTestonly(Err* err) const; bool CheckAssertNoDeps(Err* err) const; void CheckSourcesGenerated() const;
diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc index 99ce6b0..792c530 100644 --- a/src/gn/target_unittest.cc +++ b/src/gn/target_unittest.cc
@@ -142,16 +142,19 @@ // Normal non-inherited config. Config config(setup.settings(), Label(SourceDir("//foo/"), "config")); + config.visibility().SetPublic(); ASSERT_TRUE(config.OnResolved(&err)); c.configs().push_back(LabelConfigPair(&config)); // All dependent config. Config all(setup.settings(), Label(SourceDir("//foo/"), "all")); + all.visibility().SetPublic(); ASSERT_TRUE(all.OnResolved(&err)); c.all_dependent_configs().push_back(LabelConfigPair(&all)); // Direct dependent config. Config direct(setup.settings(), Label(SourceDir("//foo/"), "direct")); + direct.visibility().SetPublic(); ASSERT_TRUE(direct.OnResolved(&err)); c.public_configs().push_back(LabelConfigPair(&direct)); @@ -211,17 +214,20 @@ // All dependent config. Config all_dependent(setup.settings(), Label(SourceDir("//foo/"), "all")); + all_dependent.visibility().SetPublic(); ASSERT_TRUE(all_dependent.OnResolved(&err)); c.all_dependent_configs().push_back(LabelConfigPair(&all_dependent)); // Public config. Config public_config(setup.settings(), Label(SourceDir("//foo/"), "public")); + public_config.visibility().SetPublic(); ASSERT_TRUE(public_config.OnResolved(&err)); c.public_configs().push_back(LabelConfigPair(&public_config)); // Another public config. Config public_config2(setup.settings(), Label(SourceDir("//foo/"), "public2")); + public_config2.visibility().SetPublic(); ASSERT_TRUE(public_config2.OnResolved(&err)); b.public_configs().push_back(LabelConfigPair(&public_config2)); @@ -267,11 +273,13 @@ // All dependent config. Config all_dependent(setup.settings(), Label(SourceDir("//foo/"), "all")); + all_dependent.visibility().SetPublic(); ASSERT_TRUE(all_dependent.OnResolved(&err)); b.all_dependent_configs().push_back(LabelConfigPair(&all_dependent)); // Public config. Config public_config(setup.settings(), Label(SourceDir("//foo/"), "public")); + public_config.visibility().SetPublic(); ASSERT_TRUE(public_config.OnResolved(&err)); b.public_configs().push_back(LabelConfigPair(&public_config)); @@ -578,6 +586,7 @@ Label pub_config_label(SourceDir("//a/"), "pubconfig"); Config pub_config(setup.settings(), pub_config_label); + pub_config.visibility().SetPublic(); LibFile lib_name("testlib"); pub_config.own_values().libs().push_back(lib_name); ASSERT_TRUE(pub_config.OnResolved(&err)); @@ -620,11 +629,13 @@ TestTarget dep1(setup, "//:dep1", Target::SOURCE_SET); Label dep1_all_config_label(SourceDir("//"), "dep1_all_config"); Config dep1_all_config(setup.settings(), dep1_all_config_label); + dep1_all_config.visibility().SetPublic(); ASSERT_TRUE(dep1_all_config.OnResolved(&err)); dep1.all_dependent_configs().push_back(LabelConfigPair(&dep1_all_config)); Label dep1_public_config_label(SourceDir("//"), "dep1_public_config"); Config dep1_public_config(setup.settings(), dep1_public_config_label); + dep1_public_config.visibility().SetPublic(); ASSERT_TRUE(dep1_public_config.OnResolved(&err)); dep1.public_configs().push_back(LabelConfigPair(&dep1_public_config)); ASSERT_TRUE(dep1.OnResolved(&err)); @@ -633,11 +644,13 @@ TestTarget dep2(setup, "//:dep2", Target::SOURCE_SET); Label dep2_all_config_label(SourceDir("//"), "dep2_all_config"); Config dep2_all_config(setup.settings(), dep2_all_config_label); + dep2_all_config.visibility().SetPublic(); ASSERT_TRUE(dep2_all_config.OnResolved(&err)); dep2.all_dependent_configs().push_back(LabelConfigPair(&dep2_all_config)); Label dep2_public_config_label(SourceDir("//"), "dep2_public_config"); Config dep2_public_config(setup.settings(), dep2_public_config_label); + dep2_public_config.visibility().SetPublic(); ASSERT_TRUE(dep2_public_config.OnResolved(&err)); dep2.public_configs().push_back(LabelConfigPair(&dep2_public_config)); ASSERT_TRUE(dep2.OnResolved(&err)); @@ -650,11 +663,13 @@ // It also has a private and public config. Label public_config_label(SourceDir("//"), "public"); Config public_config(setup.settings(), public_config_label); + public_config.visibility().SetPublic(); ASSERT_TRUE(public_config.OnResolved(&err)); target.public_configs().push_back(LabelConfigPair(&public_config)); Label private_config_label(SourceDir("//"), "private"); Config private_config(setup.settings(), private_config_label); + private_config.visibility().SetPublic(); ASSERT_TRUE(private_config.OnResolved(&err)); target.configs().push_back(LabelConfigPair(&private_config));