Make Bazel handle visibility through //external: labels correctly: //external: targets can depend on anything and visibility is checked a the edges incoming to said labels.

Fixes #388.

--
MOS_MIGRATED_REVID=101363086

2 files changed, 75 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 52e85086cd..7cb8012848 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -118,6 +118,7 @@ import com.google.devtools.build.lib.rules.python.PythonOptions;
 import com.google.devtools.build.lib.rules.workspace.BindRule;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.util.ResourceFileLoader;
+import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.io.IOException;
 
@@ -125,6 +126,7 @@ import java.io.IOException;
  * A rule class provider implementing the rules Bazel knows.
  */
 public class BazelRuleClassProvider {
+  private static final PathFragment EXTERNAL = new PathFragment("external");
 
   /**
    * Used by the build encyclopedia generator.
@@ -154,12 +156,17 @@ public class BazelRuleClassProvider {
     private void validateDirectPrerequisiteVisibility(
         RuleContext.Builder context, ConfiguredTarget prerequisite, String attrName) {
       Rule rule = context.getRule();
+      if (rule.getLabel().getPackageFragment().equals(EXTERNAL)) {
+        // //external: labels are special. They have access to everything and visibility is checked
+        // at the edge that points to the //external: label.
+        return;
+      }
       Target prerequisiteTarget = prerequisite.getTarget();
       Label prerequisiteLabel = prerequisiteTarget.getLabel();
       // We don't check the visibility of late-bound attributes, because it would break some
       // features.
-      if (!context.getRule().getLabel().getPackageName().equals(
-              prerequisite.getTarget().getLabel().getPackageName())
+      if (!context.getRule().getLabel().getPackageIdentifier().equals(
+              prerequisite.getTarget().getLabel().getPackageIdentifier())
           && !context.isVisible(prerequisite)) {
         if (!context.getConfiguration().checkVisibility()) {
           context.ruleWarning(String.format("Target '%s' violates visibility of target "
diff --git a/src/test/shell/bazel/local_repository_test.sh b/src/test/shell/bazel/local_repository_test.sh
index e3f146044d..4629ae337e 100755
--- a/src/test/shell/bazel/local_repository_test.sh
+++ b/src/test/shell/bazel/local_repository_test.sh
@@ -636,7 +636,12 @@ function test_output_file_in_local_repository() {
   mkdir $r
   touch $r/WORKSPACE
   cat > $r/BUILD <<'EOF'
-genrule(name="r", srcs=[], outs=["r.out"], cmd="touch $@")
+genrule(
+    name="r",
+    srcs=[],
+    outs=["r.out"],
+    cmd="touch $@",
+    visibility=["//visibility:public"])
 EOF
 
   cat > WORKSPACE <<EOF
@@ -698,4 +703,64 @@ EOF
   assert_contains "abcxyz" bazel-bin/x.sh
 }
 
+function test_visibility_through_bind() {
+  local r=$TEST_TMPDIR/r
+  rm -fr $r
+  mkdir $r
+
+  cat > $r/BUILD <<EOF
+genrule(
+    name = "public",
+    srcs = ["//external:public"],
+    outs = ["public.out"],
+    cmd = "cp \$< \$@",
+)
+
+genrule(
+    name = "private",
+    srcs = ["//external:private"],
+    outs = ["private.out"],
+    cmd = "cp \$< \$@",
+)
+EOF
+
+  cat > WORKSPACE <<EOF
+local_repository(
+    name = "r",
+    path = "$r",
+)
+
+bind(
+    name = "public",
+    actual = "//:public",
+)
+
+bind(
+    name = "private",
+    actual = "//:private",
+)
+EOF
+
+  cat > BUILD <<EOF
+genrule(
+    name = "public",
+    srcs = [],
+    outs = ["public.out"],
+    cmd = "echo PUBLIC > \$@",
+    visibility = ["//visibility:public"],
+)
+
+genrule(
+    name = "private",
+    srcs = [],
+    outs = ["private.out"],
+    cmd = "echo PRIVATE > \$@",
+)
+EOF
+
+  bazel build @r//:public >& $TEST_log || fail "failed to build public target"
+  bazel build @r//:private >& $TEST_log && fail "could build private target"
+  expect_log "Target '//:private' is not visible from target '@r//:private'"
+}
+
 run_suite "local repository tests"