From 63570924746274c128e80d3a43558e0d3fad3051 Mon Sep 17 00:00:00 2001
From: kchodorow <kchodorow@google.com>
Date: Tue, 11 Jul 2017 22:22:22 +0200
Subject: Automated rollback of commit
 937350211dcd55a4714ec32ebbf33fffcc42cdf2.

*** Reason for rollback ***

Broke the go rules (of course)

See http://ci.bazel.io/job/rules_go/BAZEL_VERSION=HEAD,PLATFORM_NAME=linux-x86_64/1044/console.

*** Original change description ***

Resolve references to @main-repo//foo to //foo

Bazel was creating an dummy external repository for @main-repo, which
doesn't work with package paths and will cause conflicts once
@main-repo//foo and //foo refer to the same path.

This adds a "soft pull" option to WorkspaceNameFunction: it can either
parse the entire WORKSPACE file to find the name or just the first section.
That way PackageLookupFunction can find the repository name without causing
a circular dependency.

This should have no ch...

***

PiperOrigin-RevId: 161572272
---
 .../devtools/build/lib/packages/Package.java       | 11 ++----
 .../build/lib/packages/PackageFactory.java         |  5 ++-
 .../build/lib/packages/WorkspaceFactory.java       | 20 ++++++++++
 .../build/lib/skyframe/PackageLookupFunction.java  | 15 +-------
 .../build/lib/skyframe/WorkspaceNameFunction.java  | 39 ++------------------
 .../build/lib/skyframe/WorkspaceNameValue.java     | 43 +++++++++++++++-------
 .../skylark/SkylarkRepositoryIntegrationTest.java  |  4 +-
 .../lib/skyframe/PackageLookupFunctionTest.java    |  1 -
 8 files changed, 63 insertions(+), 75 deletions(-)

(limited to 'src')

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 9132208b0e..4ec65dd3b0 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
@@ -207,13 +207,9 @@ public class Package {
    * @precondition {@code name} must be a suffix of
    * {@code filename.getParentDirectory())}.
    */
-  protected Package(PackageIdentifier packageId, String workspaceName) {
-    this.workspaceName = workspaceName;
-    if (workspaceName.equals(packageId.getRepository().strippedName())) {
-      this.packageIdentifier = PackageIdentifier.createInMainRepo(packageId.getPackageFragment());
-    } else {
-      this.packageIdentifier = packageId;
-    }
+  protected Package(PackageIdentifier packageId, String runfilesPrefix) {
+    this.packageIdentifier = packageId;
+    this.workspaceName = runfilesPrefix;
     this.nameFragment = Canonicalizer.fragments().intern(packageId.getPackageFragment());
     this.name = nameFragment.getPathString();
   }
@@ -304,6 +300,7 @@ public class Package {
       throw new IllegalArgumentException(
           "Invalid BUILD file name for package '" + packageIdentifier + "': " + filename);
     }
+
     this.makeEnv = builder.makeEnv.build();
     this.targets = ImmutableSortedKeyMap.copyOf(builder.targets);
     this.defaultVisibility = builder.defaultVisibility;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index da4d3e3163..cab4a4f6d6 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -1667,7 +1667,7 @@ public final class PackageFactory {
       ImmutableList<Label> skylarkFileDependencies)
       throws InterruptedException {
     Package.Builder pkgBuilder = new Package.Builder(packageBuilderHelper.createFreshPackage(
-        packageId, workspaceName));
+        packageId, ruleClassProvider.getRunfilesPrefix()));
     StoredEventHandler eventHandler = new StoredEventHandler();
 
     try (Mutability mutability = Mutability.create("package %s", packageId)) {
@@ -1687,7 +1687,8 @@ public final class PackageFactory {
           // "defaultVisibility" comes from the command line. Let's give the BUILD file a chance to
           // set default_visibility once, be reseting the PackageBuilder.defaultVisibilitySet flag.
           .setDefaultVisibilitySet(false)
-          .setSkylarkFileDependencies(skylarkFileDependencies);
+          .setSkylarkFileDependencies(skylarkFileDependencies)
+          .setWorkspaceName(workspaceName);
 
       Event.replayEventsOn(eventHandler, pastEvents);
       for (Postable post : pastPosts) {
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 f1c25777f6..ccb81e1201 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
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.events.NullEventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
 import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
+import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
 import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
 import com.google.devtools.build.lib.syntax.BaseFunction;
@@ -315,6 +316,25 @@ public class WorkspaceFactory {
                   throw new EvalException(ast.getLocation(), errorMessage);
                 }
                 PackageFactory.getContext(env, ast).pkgBuilder.setWorkspaceName(name);
+                Package.Builder builder = PackageFactory.getContext(env, ast).pkgBuilder;
+                RuleClass localRepositoryRuleClass = ruleFactory.getRuleClass("local_repository");
+                RuleClass bindRuleClass = ruleFactory.getRuleClass("bind");
+                Map<String, Object> kwargs = ImmutableMap.<String, Object>of(
+                    "name", name, "path", ".");
+                try {
+                  // This effectively adds a "local_repository(name = "<ws>", path = ".")"
+                  // definition to the WORKSPACE file.
+                  builder
+                      .externalPackageData()
+                      .createAndAddRepositoryRule(
+                          builder,
+                          localRepositoryRuleClass,
+                          bindRuleClass,
+                          kwargs,
+                          ast);
+                } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
+                  throw new EvalException(ast.getLocation(), e.getMessage());
+                }
                 return NONE;
               }
             };
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
index d264cea205..c0a115ad6c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
@@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelValidator;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
 import com.google.devtools.build.lib.packages.ErrorDeterminingRepositoryException;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
@@ -295,20 +294,8 @@ public class PackageLookupFunction implements SkyFunction {
   private PackageLookupValue computeExternalPackageLookupValue(
       SkyKey skyKey, Environment env, PackageIdentifier packageIdentifier)
       throws PackageLookupFunctionException, InterruptedException {
-    // Check if this is the main repository.
     PackageIdentifier id = (PackageIdentifier) skyKey.argument();
-    RepositoryName repositoryName = id.getRepository();
-    WorkspaceNameValue workspaceNameValue = (WorkspaceNameValue)
-        env.getValue(WorkspaceNameValue.firstChunk());
-    if (workspaceNameValue == null) {
-      return null;
-    }
-    if (repositoryName.strippedName().equals(workspaceNameValue.getName())) {
-      return (PackageLookupValue) env.getValue(PackageLookupValue.key(
-          PackageIdentifier.createInMainRepo(id.getPackageFragment())));
-    }
-    // Otherwise look up the repository.
-    SkyKey repositoryKey = RepositoryValue.key(repositoryName);
+    SkyKey repositoryKey = RepositoryValue.key(id.getRepository());
     RepositoryValue repositoryValue;
     try {
       repositoryValue = (RepositoryValue) env.getValueOrThrow(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java
index 4582572c4a..9284f5c2e9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java
@@ -14,11 +14,9 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Package;
-import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -36,47 +34,18 @@ public class WorkspaceNameFunction implements SkyFunction {
   @Nullable
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws InterruptedException, WorkspaceNameFunctionException {
-    boolean firstChunk = (boolean) skyKey.argument();
-    Package externalPackage = firstChunk ? firstChunk(env) : parseFullPackage(env);
-    if (externalPackage == null) {
+    SkyKey externalPackageKey = PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER);
+    PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey);
+    if (externalPackageValue == null) {
       return null;
     }
+    Package externalPackage = externalPackageValue.getPackage();
     if (externalPackage.containsErrors()) {
-      Event.replayEventsOn(env.getListener(), externalPackage.getEvents());
       throw new WorkspaceNameFunctionException();
     }
     return WorkspaceNameValue.withName(externalPackage.getWorkspaceName());
   }
 
-  /**
-   * This just examines the first chunk of the WORKSPACE file to avoid circular dependencies and
-   * overeager invalidation during package loading.
-   */
-  private Package firstChunk(Environment env) throws InterruptedException {
-    SkyKey packageLookupKey = PackageLookupValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER);
-    PackageLookupValue packageLookupValue = (PackageLookupValue) env.getValue(packageLookupKey);
-    if (packageLookupValue == null) {
-      return null;
-    }
-    RootedPath workspacePath = packageLookupValue.getRootedPath(Label.EXTERNAL_PACKAGE_IDENTIFIER);
-
-    SkyKey workspaceKey = WorkspaceFileValue.key(workspacePath);
-    WorkspaceFileValue value = (WorkspaceFileValue) env.getValue(workspaceKey);
-    if (value == null) {
-      return null;
-    }
-    return value.getPackage();
-  }
-
-  private Package parseFullPackage(Environment env) throws InterruptedException {
-    SkyKey externalPackageKey = PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER);
-    PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey);
-    if (externalPackageValue == null) {
-      return null;
-    }
-    return externalPackageValue.getPackage();
-  }
-
   @Override
   @Nullable
   public String extractTag(SkyKey skyKey) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java
index 98fe8b2416..4da943b6ab 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameValue.java
@@ -28,6 +28,9 @@ import java.util.Objects;
  * the WORKSPACE file.
  */
 public class WorkspaceNameValue implements SkyValue {
+  private static final SkyKey KEY =
+      LegacySkyKey.create(SkyFunctions.WORKSPACE_NAME, DummyArgument.INSTANCE);
+
   private final String workspaceName;
 
   private WorkspaceNameValue(String workspaceName) {
@@ -41,21 +44,9 @@ public class WorkspaceNameValue implements SkyValue {
     return workspaceName;
   }
 
-  /** Returns the {@link SkyKey} for {@link WorkspaceNameValue}s. */
+  /** Returns the (singleton) {@link SkyKey} for {@link WorkspaceNameValue}s. */
   public static SkyKey key() {
-    return createKey(false);
-  }
-
-  /**
-   * Returns the {@link SkyKey} for a less-aggressive {@link WorkspaceNameValue} query, when
-   * we don't want to parse the whole WORKSPACE file.
-   */
-  public static SkyKey firstChunk() {
-    return createKey(true);
-  }
-
-  private static SkyKey createKey(boolean firstChunk) {
-    return LegacySkyKey.create(SkyFunctions.WORKSPACE_NAME, firstChunk);
+    return KEY;
   }
 
   /** Returns a {@link WorkspaceNameValue} for a workspace with the given name. */
@@ -81,4 +72,28 @@ public class WorkspaceNameValue implements SkyValue {
   public String toString() {
     return String.format("WorkspaceNameValue[name=%s]", workspaceName);
   }
+
+  /** Singleton class used as the {@link SkyKey#argument} for {@link WorkspaceNameValue#key}. */
+  public static final class DummyArgument {
+    static final int HASHCODE = DummyArgument.class.getCanonicalName().hashCode();
+    public static final DummyArgument INSTANCE = new DummyArgument();
+
+    private DummyArgument() {
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      return obj instanceof DummyArgument;
+    }
+
+    @Override
+    public int hashCode() {
+      return HASHCODE;
+    }
+
+    @Override
+    public String toString() {
+      return "#";
+    }
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
index 35b08e1444..004987d7b8 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java
@@ -297,7 +297,7 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
 
     invalidatePackages();
     try {
-      getTarget("@git_repo//:whatever");
+      getTarget("@//:git_repo");
       fail();
     } catch (AssertionError expected) {
       assertThat(expected)
@@ -332,7 +332,7 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase {
       fail();
     } catch (NoSuchPackageException e) {
       // This is expected
-      assertThat(e).hasMessageThat().contains("Package 'external' contains errors");
+      assertThat(e).hasMessageThat().contains("Could not load //external package");
     }
     assertContainsEvent("missing value for mandatory attribute 'path' in 'local_repository' rule");
   }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
index f407c247c1..7e770ffe81 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
@@ -123,7 +123,6 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase {
                     ruleClassProvider,
                     scratch.getFileSystem()),
             directories));
-    skyFunctions.put(SkyFunctions.WORKSPACE_NAME, new WorkspaceNameFunction());
     skyFunctions.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction());
     skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
     skyFunctions.put(
-- 
cgit v1.2.3