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-Commit-Position: refs/heads/master@{#371539}

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 @@ bool Setup::FillOtherConfig(const base::CommandLine& cmdline) {
   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.

tools/gn/target.cc

@@ -129,6 +129,61 @@ bool EnsureFileIsGeneratedByDependency(const Target* target,
   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 @@ bool Target::OnResolved(Err* err) {
       return false;
     if (!CheckNoNestedStaticLibs(err))
       return false;
+    if (!CheckAssertNoDeps(err))
+      return false;
     CheckSourcesGenerated();
   }
 
@@ -604,6 +661,27 @@ bool Target::CheckNoNestedStaticLibs(Err* err) const {
   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

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 @@ class Target : public Item {
     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 @@ class Target : public Item {
   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 @@ class Target : public Item {
   // 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.

tools/gn/target_generator.cc

@@ -50,6 +50,9 @@ void TargetGenerator::Run() {
   if (!FillTestonly())
     return;
 
+  if (!FillAssertNoDeps())
+    return;
+
   if (!Visibility::FillItemVisibility(target_, scope_, err_))
     return;
 
@@ -266,6 +269,15 @@ bool TargetGenerator::FillTestonly() {
   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)

tools/gn/target_generator.h

@@ -70,6 +70,7 @@ class TargetGenerator {
   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.

tools/gn/target_unittest.cc

@@ -668,3 +668,58 @@ TEST(Target, ResolvePrecompiledHeaders) {
       "  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));
+}

tools/gn/value_extractors.cc

@@ -144,6 +144,17 @@ template<typename T> struct LabelPtrResolver {
   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 @@ bool ExtractRelativeFile(const BuildSettings* build_settings,
   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));
+}

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 @@ bool ExtractRelativeFile(const BuildSettings* build_settings,
                          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_

tools/gn/variables.cc

@@ -391,6 +391,49 @@ const char kArgs_Help[] =
     "\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 @@ const VariableInfoMap& GetTargetVariables() {
     INSERT_VARIABLE(AllowCircularIncludesFrom)
     INSERT_VARIABLE(Args)
     INSERT_VARIABLE(Asmflags)
+    INSERT_VARIABLE(AssertNoDeps)
     INSERT_VARIABLE(Cflags)
     INSERT_VARIABLE(CflagsC)
     INSERT_VARIABLE(CflagsCC)

tools/gn/variables.h

@@ -87,6 +87,10 @@ extern const char kAsmflags[];
 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;