[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));
