Add an assert_no_deps variable to GN.
This asserts that there is no dependency path to a given target.
BUG=
Review URL: https://codereview.chromium.org/1621053002
Cr-Original-Commit-Position: refs/heads/master@{#371539}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 81aa4e8f010454835da6caa14507bcee73578e83
diff --git a/tools/gn/setup.cc b/tools/gn/setup.cc
index 459b87b..79814e6 100644
--- a/tools/gn/setup.cc
+++ b/tools/gn/setup.cc
@@ -31,6 +31,7 @@
#include "tools/gn/tokenizer.h"
#include "tools/gn/trace.h"
#include "tools/gn/value.h"
+#include "tools/gn/value_extractors.h"
#if defined(OS_WIN)
#include <windows.h>
@@ -655,21 +656,13 @@
const Value* check_targets_value =
dotfile_scope_.GetValue("check_targets", true);
if (check_targets_value) {
- // Fill the list of targets to check.
- if (!check_targets_value->VerifyTypeIs(Value::LIST, &err)) {
+ check_patterns_.reset(new std::vector<LabelPattern>);
+ ExtractListOfLabelPatterns(*check_targets_value, current_dir,
+ check_patterns_.get(), &err);
+ if (err.has_error()) {
err.PrintToStdout();
return false;
}
-
- check_patterns_.reset(new std::vector<LabelPattern>);
- for (const auto& item : check_targets_value->list_value()) {
- check_patterns_->push_back(
- LabelPattern::GetPattern(current_dir, item, &err));
- if (err.has_error()) {
- err.PrintToStdout();
- return false;
- }
- }
}
// Fill exec_script_whitelist.
diff --git a/tools/gn/target.cc b/tools/gn/target.cc
index 905ee4f..7dd1921 100644
--- a/tools/gn/target.cc
+++ b/tools/gn/target.cc
@@ -129,6 +129,61 @@
return false;
}
+// check_this indicates if the given target should be matched against the
+// patterns. It should be set to false for the first call since assert_no_deps
+// shouldn't match the target itself.
+//
+// visited should point to an empty set, this will be used to prevent
+// multiple visits.
+//
+// *failure_path_str will be filled with a string describing the path of the
+// dependency failure, and failure_pattern will indicate the pattern in
+// assert_no that matched the target.
+//
+// Returns true if everything is OK. failure_path_str and failure_pattern_index
+// will be unchanged in this case.
+bool RecursiveCheckAssertNoDeps(const Target* target,
+ bool check_this,
+ const std::vector<LabelPattern>& assert_no,
+ std::set<const Target*>* visited,
+ std::string* failure_path_str,
+ const LabelPattern** failure_pattern) {
+ static const char kIndentPath[] = " ";
+
+ if (visited->find(target) != visited->end())
+ return true; // Already checked this target.
+ visited->insert(target);
+
+ if (check_this) {
+ // Check this target against the given list of patterns.
+ for (const LabelPattern& pattern : assert_no) {
+ if (pattern.Matches(target->label())) {
+ // Found a match.
+ *failure_pattern = &pattern;
+ *failure_path_str =
+ kIndentPath + target->label().GetUserVisibleName(false);
+ return false;
+ }
+ }
+ }
+
+ // Recursively check dependencies.
+ for (const auto& pair : target->GetDeps(Target::DEPS_ALL)) {
+ if (pair.ptr->output_type() == Target::EXECUTABLE)
+ continue;
+ if (!RecursiveCheckAssertNoDeps(pair.ptr, true, assert_no, visited,
+ failure_path_str, failure_pattern)) {
+ // To reconstruct the path, prepend the current target to the error.
+ std::string prepend_path =
+ kIndentPath + target->label().GetUserVisibleName(false) + " ->\n";
+ failure_path_str->insert(0, prepend_path);
+ return false;
+ }
+ }
+
+ return true;
+}
+
} // namespace
Target::Target(const Settings* settings, const Label& label)
@@ -234,6 +289,8 @@
return false;
if (!CheckNoNestedStaticLibs(err))
return false;
+ if (!CheckAssertNoDeps(err))
+ return false;
CheckSourcesGenerated();
}
@@ -604,6 +661,27 @@
return true;
}
+bool Target::CheckAssertNoDeps(Err* err) const {
+ if (assert_no_deps_.empty())
+ return true;
+
+ std::set<const Target*> visited;
+ std::string failure_path_str;
+ const LabelPattern* failure_pattern = nullptr;
+
+ if (!RecursiveCheckAssertNoDeps(this, false, assert_no_deps_, &visited,
+ &failure_path_str, &failure_pattern)) {
+ *err = Err(defined_from(), "assert_no_deps failed.",
+ label().GetUserVisibleName(false) +
+ " has an assert_no_deps entry:\n " +
+ failure_pattern->Describe() +
+ "\nwhich fails for the dependency path:\n" +
+ failure_path_str);
+ return false;
+ }
+ return true;
+}
+
void Target::CheckSourcesGenerated() const {
// Checks that any inputs or sources to this target that are in the build
// directory are generated by a target that this one transitively depends on
diff --git a/tools/gn/target.h b/tools/gn/target.h
index cbdab35..18e5fb8 100644
--- a/tools/gn/target.h
+++ b/tools/gn/target.h
@@ -16,6 +16,7 @@
#include "tools/gn/config_values.h"
#include "tools/gn/inherited_libraries.h"
#include "tools/gn/item.h"
+#include "tools/gn/label_pattern.h"
#include "tools/gn/label_ptr.h"
#include "tools/gn/lib_file.h"
#include "tools/gn/ordered_set.h"
@@ -209,6 +210,13 @@
return recursive_hard_deps_;
}
+ std::vector<LabelPattern>& assert_no_deps() {
+ return assert_no_deps_;
+ }
+ const std::vector<LabelPattern>& assert_no_deps() const {
+ return assert_no_deps_;
+ }
+
// The toolchain is only known once this target is resolved (all if its
// dependencies are known). They will be null until then. Generally, this can
// only be used during target writing.
@@ -284,6 +292,7 @@
bool CheckVisibility(Err* err) const;
bool CheckTestonly(Err* err) const;
bool CheckNoNestedStaticLibs(Err* err) const;
+ bool CheckAssertNoDeps(Err* err) const;
void CheckSourcesGenerated() const;
void CheckSourceGenerated(const SourceFile& source) const;
@@ -324,6 +333,8 @@
// target is marked resolved. This will not include the current target.
std::set<const Target*> recursive_hard_deps_;
+ std::vector<LabelPattern> assert_no_deps_;
+
// Used for all binary targets. The precompiled header values in this struct
// will be resolved to the ones to use for this target, if precompiled
// headers are used.
diff --git a/tools/gn/target_generator.cc b/tools/gn/target_generator.cc
index 3734ea9..b1ed1ea 100644
--- a/tools/gn/target_generator.cc
+++ b/tools/gn/target_generator.cc
@@ -50,6 +50,9 @@
if (!FillTestonly())
return;
+ if (!FillAssertNoDeps())
+ return;
+
if (!Visibility::FillItemVisibility(target_, scope_, err_))
return;
@@ -266,6 +269,15 @@
return true;
}
+bool TargetGenerator::FillAssertNoDeps() {
+ const Value* value = scope_->GetValue(variables::kAssertNoDeps, true);
+ if (value) {
+ return ExtractListOfLabelPatterns(*value, scope_->GetSourceDir(),
+ &target_->assert_no_deps(), err_);
+ }
+ return true;
+}
+
bool TargetGenerator::FillOutputs(bool allow_substitutions) {
const Value* value = scope_->GetValue(variables::kOutputs, true);
if (!value)
diff --git a/tools/gn/target_generator.h b/tools/gn/target_generator.h
index 1fd055b..4fa51f6 100644
--- a/tools/gn/target_generator.h
+++ b/tools/gn/target_generator.h
@@ -70,6 +70,7 @@
bool FillData();
bool FillDependencies(); // Includes data dependencies.
bool FillTestonly();
+ bool FillAssertNoDeps();
// Reads configs/deps from the given var name, and uses the given setting on
// the target to save them.
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc
index c59f44c..c99ebe8 100644
--- a/tools/gn/target_unittest.cc
+++ b/tools/gn/target_unittest.cc
@@ -668,3 +668,58 @@
" source: //pcs2.cc",
err.help_text());
}
+
+TEST(Target, AssertNoDeps) {
+ TestWithScope setup;
+ Err err;
+
+ // A target.
+ TestTarget a(setup, "//a", Target::SHARED_LIBRARY);
+ ASSERT_TRUE(a.OnResolved(&err));
+
+ // B depends on A and has an assert_no_deps for a random dir.
+ TestTarget b(setup, "//b", Target::SHARED_LIBRARY);
+ b.private_deps().push_back(LabelTargetPair(&a));
+ b.assert_no_deps().push_back(LabelPattern(
+ LabelPattern::RECURSIVE_DIRECTORY, SourceDir("//disallowed/"),
+ std::string(), Label()));
+ ASSERT_TRUE(b.OnResolved(&err));
+
+ LabelPattern disallow_a(LabelPattern::RECURSIVE_DIRECTORY, SourceDir("//a/"),
+ std::string(), Label());
+
+ // C depends on B and disallows depending on A. This should fail.
+ TestTarget c(setup, "//c", Target::EXECUTABLE);
+ c.private_deps().push_back(LabelTargetPair(&b));
+ c.assert_no_deps().push_back(disallow_a);
+ ASSERT_FALSE(c.OnResolved(&err));
+
+ // Validate the error message has the proper path.
+ EXPECT_EQ(
+ "//c:c has an assert_no_deps entry:\n"
+ " //a/*\n"
+ "which fails for the dependency path:\n"
+ " //c:c ->\n"
+ " //b:b ->\n"
+ " //a:a",
+ err.help_text());
+ err = Err();
+
+ // Add an intermediate executable with: exe -> b -> a
+ TestTarget exe(setup, "//exe", Target::EXECUTABLE);
+ exe.private_deps().push_back(LabelTargetPair(&b));
+ ASSERT_TRUE(exe.OnResolved(&err));
+
+ // D depends on the executable and disallows depending on A. Since there is
+ // an intermediate executable, this should be OK.
+ TestTarget d(setup, "//d", Target::EXECUTABLE);
+ d.private_deps().push_back(LabelTargetPair(&exe));
+ d.assert_no_deps().push_back(disallow_a);
+ ASSERT_TRUE(d.OnResolved(&err));
+
+ // A2 disallows depending on anything in its own directory, but the
+ // assertions should not match the target itself so this should be OK.
+ TestTarget a2(setup, "//a:a2", Target::EXECUTABLE);
+ a2.assert_no_deps().push_back(disallow_a);
+ ASSERT_TRUE(a2.OnResolved(&err));
+}
diff --git a/tools/gn/value_extractors.cc b/tools/gn/value_extractors.cc
index 23c5a27..ff009ce 100644
--- a/tools/gn/value_extractors.cc
+++ b/tools/gn/value_extractors.cc
@@ -144,6 +144,17 @@
const Label& current_toolchain;
};
+struct LabelPatternResolver {
+ LabelPatternResolver(const SourceDir& current_dir_in)
+ : current_dir(current_dir_in) {
+ }
+ bool operator()(const Value& v, LabelPattern* out, Err* err) const {
+ *out = LabelPattern::GetPattern(current_dir, v, err);
+ return !err->has_error();
+ }
+ const SourceDir& current_dir;
+};
+
} // namespace
bool ExtractListOfStringValues(const Value& value,
@@ -236,3 +247,11 @@
RelativeFileConverter converter(build_settings, current_dir);
return converter(value, file, err);
}
+
+bool ExtractListOfLabelPatterns(const Value& value,
+ const SourceDir& current_dir,
+ std::vector<LabelPattern>* patterns,
+ Err* err) {
+ return ListValueExtractor(value, patterns, err,
+ LabelPatternResolver(current_dir));
+}
diff --git a/tools/gn/value_extractors.h b/tools/gn/value_extractors.h
index 06d64ce..1e42650 100644
--- a/tools/gn/value_extractors.h
+++ b/tools/gn/value_extractors.h
@@ -15,6 +15,7 @@
class BuildSettings;
class Err;
class Label;
+class LabelPattern;
class SourceDir;
class SourceFile;
class Value;
@@ -80,4 +81,9 @@
SourceFile* file,
Err* err);
+bool ExtractListOfLabelPatterns(const Value& value,
+ const SourceDir& current_dir,
+ std::vector<LabelPattern>* patterns,
+ Err* err);
+
#endif // TOOLS_GN_VALUE_EXTRACTORS_H_
diff --git a/tools/gn/variables.cc b/tools/gn/variables.cc
index a835b22..c88a182 100644
--- a/tools/gn/variables.cc
+++ b/tools/gn/variables.cc
@@ -391,6 +391,49 @@
"\n"
" See also \"gn help action\" and \"gn help action_foreach\".\n";
+const char kAssertNoDeps[] = "assert_no_deps";
+const char kAssertNoDeps_HelpShort[] =
+ "assert_no_deps: [label pattern list] Ensure no deps on these targets.";
+const char kAssertNoDeps_Help[] =
+ "assert_no_deps: Ensure no deps on these targets.\n"
+ "\n"
+ " A list of label patterns.\n"
+ "\n"
+ " This list is a list of patterns that must not match any of the\n"
+ " transitive dependencies of the target. These include all public,\n"
+ " private, and data dependencies, and cross shared library boundaries.\n"
+ " This allows you to express that undesirable code isn't accidentally\n"
+ " added to downstream dependencies in a way that might otherwise be\n"
+ " difficult to notice.\n"
+ "\n"
+ " Checking does not cross executable boundaries. If a target depends on\n"
+ " an executable, it's assumed that the executable is a tool that is\n"
+ " producing part of the build rather than something that is linked and\n"
+ " distributed. This allows assert_no_deps to express what is distributed\n"
+ " in the final target rather than depend on the internal build steps\n"
+ " (which may include non-distributable code).\n"
+ "\n"
+ " See \"gn help label_pattern\" for the format of the entries in the\n"
+ " list. These patterns allow blacklisting individual targets or whole\n"
+ " directory hierarchies.\n"
+ "\n"
+ " Sometimes it is desirable to enforce that many targets have no\n"
+ " dependencies on a target or set of targets. One efficient way to\n"
+ " express this is to create a group with the assert_no_deps rule on\n"
+ " it, and make that group depend on all targets you want to apply that\n"
+ " assertion to.\n"
+ "\n"
+ "Example\n"
+ "\n"
+ " executable(\"doom_melon\") {\n"
+ " deps = [ \"//foo:bar\" ]\n"
+ " ...\n"
+ " assert_no_deps = [\n"
+ " \"//evil/*\", # Don't link any code from the evil directory.\n"
+ " \"//foo:test_support\", # This target is also disallowed.\n"
+ " ]\n"
+ " }\n";
+
const char kCflags[] = "cflags";
const char kCflags_HelpShort[] =
"cflags: [string list] Flags passed to all C compiler variants.";
@@ -1400,6 +1443,7 @@
INSERT_VARIABLE(AllowCircularIncludesFrom)
INSERT_VARIABLE(Args)
INSERT_VARIABLE(Asmflags)
+ INSERT_VARIABLE(AssertNoDeps)
INSERT_VARIABLE(Cflags)
INSERT_VARIABLE(CflagsC)
INSERT_VARIABLE(CflagsCC)
diff --git a/tools/gn/variables.h b/tools/gn/variables.h
index d02d2c6..ee68bd5 100644
--- a/tools/gn/variables.h
+++ b/tools/gn/variables.h
@@ -87,6 +87,10 @@
extern const char kAsmflags_HelpShort[];
extern const char* kAsmflags_Help;
+extern const char kAssertNoDeps[];
+extern const char kAssertNoDeps_HelpShort[];
+extern const char kAssertNoDeps_Help[];
+
extern const char kCflags[];
extern const char kCflags_HelpShort[];
extern const char* kCflags_Help;
