Make the rule in the workspace package reference the correct Package object.

Fixes #1228.

--
MOS_MIGRATED_REVID=122511655

4 files changed, 93 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 1776ed6626..fc2a1931ba 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1190,6 +1190,7 @@ public class Package {
     }
 
     void addRule(Rule rule) throws NameConflictException {
+      Preconditions.checkArgument(rule.getPackage() == pkg);
       checkForConflicts(rule);
       // Now, modify the package:
       for (OutputFile outputFile : rule.getOutputFiles()) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index 6df9f05411..625e3fe8fd 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -24,6 +24,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.LabelValidator;
 import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.NullEventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Package.Builder;
 import com.google.devtools.build.lib.packages.Package.LegacyBuilder;
@@ -247,7 +248,7 @@ public class WorkspaceFactory {
       Package aPackage,
       ImmutableMap<String, Extension> importMap,
       ImmutableMap<String, Object> bindings)
-      throws NameConflictException {
+      throws NameConflictException, InterruptedException {
     this.parentVariableBindings = bindings;
     this.parentImportMap = importMap;
     builder.setWorkspaceName(aPackage.getWorkspaceName());
@@ -256,8 +257,24 @@ public class WorkspaceFactory {
     if (aPackage.containsErrors()) {
       builder.setContainsErrors();
     }
-    for (Target target : aPackage.getTargets(Rule.class)) {
-      builder.addRule((Rule) target);
+    for (Rule rule : aPackage.getTargets(Rule.class)) {
+      try {
+        // The old rule references another Package instance and we wan't to keep the invariant that
+        // every Rule references the Package it is contained within
+        Rule newRule = builder.createRule(
+            rule.getLabel(),
+            rule.getRuleClassObject(),
+            rule.getLocation(),
+            rule.getAttributeContainer());
+        newRule.populateOutputFiles(NullEventHandler.INSTANCE, builder);
+        if (rule.containsErrors()) {
+          newRule.setContainsErrors();
+        }
+        builder.addRule(newRule);
+      } catch (LabelSyntaxException e) {
+        // This rule has already been created once, so it should have worked the second time, too
+        throw new IllegalStateException(e);
+      }
     }
   }
 
diff --git a/src/test/shell/bazel/android/android_integration_test.sh b/src/test/shell/bazel/android/android_integration_test.sh
index c838c0eb91..7d3dd34f4f 100755
--- a/src/test/shell/bazel/android/android_integration_test.sh
+++ b/src/test/shell/bazel/android/android_integration_test.sh
@@ -159,6 +159,21 @@ function check_num_sos() {
   assert_equals "11" "$num_sos"
 }
 
+function test_sdk_library_deps() {
+  create_new_workspace
+  setup_android_support
+
+  mkdir -p java/a
+  cat > java/a/BUILD<<EOF
+android_library(
+    name = "a",
+    deps = ["//external:android/mediarouter_v7"],
+)
+EOF
+
+  bazel build --nobuild //java/a:a || fail "build failed"
+}
+
 function test_android_binary() {
   create_new_workspace
   setup_android_support
diff --git a/src/test/shell/bazel/external_skylark_load_test.sh b/src/test/shell/bazel/external_skylark_load_test.sh
index bd8318ad87..a198371595 100755
--- a/src/test/shell/bazel/external_skylark_load_test.sh
+++ b/src/test/shell/bazel/external_skylark_load_test.sh
@@ -236,4 +236,61 @@ EOF
   expect_log "@r1//foo:bar"
 }
 
+function test_aspects_and_skylark_repositories() {
+cat > WORKSPACE <<EOF
+bind(name="x1", actual="//:x1")
+load("//:repo.bzl", "repo")
+bind(name="x2", actual="//:x2")
+EOF
+
+cat > BUILD <<EOF
+load("//:rule.bzl", "test_rule")
+
+filegroup(name = "x1")
+filegroup(name = "x2")
+test_rule(
+    name = "tr",
+    deps = ["//external:x1", "//external:x2"],
+)
+EOF
+
+cat > repo.bzl <<EOF
+def repo():
+  pass
+EOF
+
+cat > rule.bzl <<EOF
+def test_aspect_impl(target, ctx):
+  return struct()
+
+test_aspect = aspect(
+    attrs = {
+        "_x": attr.label_list(default = [
+            Label("//external:x1"),
+            Label("//external:x2"),
+        ]),
+    },
+    implementation = test_aspect_impl,
+)
+
+def test_rule_impl(ctx):
+  return struct()
+
+test_rule = rule(
+    attrs = {
+        "deps": attr.label_list(
+            allow_files = True,
+            allow_rules = [
+                "filegroup",
+            ],
+            aspects = [test_aspect],
+        ),
+    },
+    implementation = test_rule_impl,
+)
+EOF
+
+  bazel build //:tr || fail "build failed"
+}
+
 run_suite "Test Skylark loads from/in external repositories"