Rollback of commit 82d4327.

*** Reason for rollback ***

Breaks TensorFlow and other Bazel jobs on ci.bazel.io

*** Original change description ***

Change execution root for external repositories to be ../repo

Some of the important aspect of this change:

* Remote repos in the execution root are under output_base/execroot/repo_name, so the prefix is ../repo_name (to escape the local workspace name).
* Package roots for external repos were previously "output_base/", they are now output_base/external/repo_name (which means source artifacts always have a relative path from their repository).
* Outputs are under bazel-bin/external/repo_name/ (or similarly under genfiles). Note that this is a bit of a change from how this was implemented in the previous cl.

Fixes bazelbuild#1262.

RELNOTES[INC]: Previously, an external repository would be symlinked into the
execution root at execroot/local_repo/external/remote_repo. This changes it to
be at execroot/remote_repo. This may break genrules/Skylark actions that
hardcode execution root paths. If this causes breakages for you, ensure that
genrules are using $(location :target) to access files and Skylark rules are
using http://bazel.io/docs/skylark/lib/File.html's path, dirname, etc.
functions.

Roll forward of bdfd58a.

--
MOS_MIGRATED_REVID=133709658

.gitignore

@@ -8,7 +8,6 @@
 /WORKSPACE.user.bzl
 /bazel-bazel
 /bazel-bin
-/bazel-external
 /bazel-genfiles
 /bazel-io_bazel
 /bazel-out

src/java_tools/buildjar/java/com/google/devtools/build/java/bazel/BUILD

@@ -12,7 +12,7 @@ genrule(
     name = "javac-bootclasspath-locations",
     srcs = ["@bazel_tools//tools/jdk:bootclasspath"],
     outs = ["JavacBootclasspathLocations.java"],
-    cmd = "$(location javac-bootclasspath-locations.sh) $@ $(GENDIR) 'bazel_tools/' $(SRCS)",
+    cmd = "$(location javac-bootclasspath-locations.sh) $@ $(GENDIR) '' $(SRCS)",
     tools = ["javac-bootclasspath-locations.sh"],
 )
 

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

@@ -40,19 +40,17 @@
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.List;
-import java.util.Objects;
 import javax.annotation.Nullable;
 
 /**
  * An Artifact represents a file used by the build system, whether it's a source
  * file or a derived (output) file. Not all Artifacts have a corresponding
- * FileTarget object in the <code>build.lib.packages</code> API: for example,
+ * FileTarget object in the <code>build.packages</code> API: for example,
  * low-level intermediaries internal to a given rule, such as a Java class files
  * or C++ object files. However all FileTargets have a corresponding Artifact.
  *
- * <p>In any given call to
- * {@link com.google.devtools.build.lib.skyframe.SkyframeExecutor#buildArtifacts}, no two Artifacts
- * in the action graph may refer to the same path.
+ * <p>In any given call to Builder#buildArtifacts(), no two Artifacts in the
+ * action graph may refer to the same path.
  *
  * <p>Artifacts generally fall into two classifications, source and derived, but
  * there exist a few other cases that are fuzzy and difficult to classify. The
@@ -130,6 +128,7 @@ public int compareTo(Object o) {
     return EvalUtils.compareByClass(this, o);
   }
 
+
   /** An object that can expand middleman artifacts. */
   public interface ArtifactExpander {
 
@@ -183,10 +182,10 @@ public boolean apply(Artifact input) {
    * </pre>
    *
    * <p>In a derived Artifact, the execPath will overlap with part of the root, which in turn will
-   * be below the execRoot.
+   * be below of the execRoot.
    * <pre>
    *  [path] == [/root][pathTail] == [/execRoot][execPath] == [/execRoot][rootPrefix][pathTail]
-   * </pre>
+   * <pre>
    */
   @VisibleForTesting
   public Artifact(Path path, Root root, PathFragment execPath, ArtifactOwner owner) {
@@ -201,6 +200,7 @@ public Artifact(Path path, Root root, PathFragment execPath, ArtifactOwner owner
     this.hashCode = path.hashCode();
     this.path = path;
     this.root = root;
+    this.execPath = execPath;
     // These two lines establish the invariant that
     // execPath == rootRelativePath <=> execPath.equals(rootRelativePath)
     // This is important for isSourceArtifact.
@@ -210,7 +210,6 @@ public Artifact(Path path, Root root, PathFragment execPath, ArtifactOwner owner
           + rootRel + " at " + path + " with root " + root);
     }
     this.rootRelativePath = rootRel.equals(execPath) ? execPath : rootRel;
-    this.execPath = externalfy(execPath);
     this.owner = Preconditions.checkNotNull(owner, path);
   }
 
@@ -351,12 +350,7 @@ public PathFragment getParentRelativePath() {
   @SkylarkCallable(name = "is_source", structField =  true,
       doc = "Returns true if this is a source file, i.e. it is not generated")
   public final boolean isSourceArtifact() {
-    // All source roots should, you know, point to sources. However, for embedded binaries, they
-    // are actually created as derived artifacts, so we have to special-case isSourceArtifact to
-    // treat derived roots in the main repo where execPath==rootRelPath as source roots.
-    // Source artifacts have reference-identical execPaths and rootRelativePaths, so use
-    // of == instead of .equals() is intentional here.
-    return execPath == rootRelativePath || root.isSourceRoot();
+    return execPath == rootRelativePath;
   }
 
   /**
@@ -521,9 +515,15 @@ public final PathFragment getRootRelativePath() {
    * For targets in external repositories, this returns the path the artifact live at in the
    * runfiles tree. For local targets, it returns the rootRelativePath.
    */
-  // TODO(kchodorow): remove.
   public final PathFragment getRunfilesPath() {
-    return externalfy(rootRelativePath);
+    PathFragment relativePath = rootRelativePath;
+    if (relativePath.segmentCount() > 1
+        && relativePath.getSegment(0).equals(Label.EXTERNAL_PATH_PREFIX)) {
+      // Turn external/repo/foo into ../repo/foo.
+      relativePath = relativePath.relativeTo(Label.EXTERNAL_PATH_PREFIX);
+      relativePath = new PathFragment("..").getRelative(relativePath);
+    }
+    return relativePath;
   }
 
   @SkylarkCallable(
@@ -557,31 +557,7 @@ public final String getRunfilesPathString() {
             + "runfiles of a binary."
   )
   public final String getExecPathString() {
-    return getExecPath().toString();
-  }
-
-  private PathFragment externalfy(PathFragment relativePath) {
-    if (root.isMainRepo()) {
-      return relativePath;
-    }
-
-    PathFragment prefix;
-    if (root.isSourceRoot()) {
-      prefix = new PathFragment(Label.EXTERNAL_PATH_PREFIX)
-          .getRelative(root.getPath().getBaseName());
-    } else {
-      prefix = new PathFragment(Label.EXTERNAL_PATH_PREFIX)
-          .getRelative(root.getExecRoot().getBaseName());
-    }
-    return prefix.getRelative(relativePath);
-  }
-
-  /**
-   * Returns if this artifact is in the main repository or not.
-   */
-  // TODO(kchodorow): remove once execroot path is correct.
-  public boolean isInMainRepo() {
-    return root.isMainRepo();
+    return getExecPath().getPathString();
   }
 
   /*
@@ -609,7 +585,7 @@ public final boolean equals(Object other) {
     // We don't bother to check root in the equivalence relation, because we
     // assume that no root is an ancestor of another one.
     Artifact that = (Artifact) other;
-    return Objects.equals(this.path, that.path);
+    return this.path.equals(that.path);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java

@@ -287,9 +287,8 @@ private Artifact createArtifact(Path path, Root root, PathFragment execPath, Art
    * not null). That Artifact will have root determined by the package roots of this factory if it
    * lives in a subpackage distinct from that of baseExecPath, and {@code baseRoot} otherwise.
    */
-  private synchronized Artifact resolveSourceArtifactWithAncestor(
-      PathFragment relativePath, PathFragment baseExecPath, Root baseRoot,
-      RepositoryName repositoryName) {
+  public synchronized Artifact resolveSourceArtifactWithAncestor(
+      PathFragment relativePath, PathFragment baseExecPath, Root baseRoot) {
     Preconditions.checkState(
         (baseExecPath == null) == (baseRoot == null),
         "%s %s %s",
@@ -314,15 +313,7 @@ private synchronized Artifact resolveSourceArtifactWithAncestor(
       return null;
     }
 
-    return createArtifactIfNotValid(
-        findSourceRoot(execPath, baseExecPath, baseRoot, repositoryName), execPath);
-  }
-
-  // TODO(kchodorow): make remote repositories work with include scanning.
-  public synchronized Artifact resolveSourceArtifactWithAncestor(
-      PathFragment relativePath, PathFragment baseExecPath, Root baseRoot) {
-    return resolveSourceArtifactWithAncestor(
-        relativePath, baseExecPath, baseRoot, RepositoryName.MAIN);
+    return createArtifactIfNotValid(findSourceRoot(execPath, baseExecPath, baseRoot), execPath);
   }
 
   /**
@@ -331,20 +322,21 @@ public synchronized Artifact resolveSourceArtifactWithAncestor(
    */
   @Nullable
   private Root findSourceRoot(
-      PathFragment execPath, @Nullable PathFragment baseExecPath, @Nullable Root baseRoot,
-      RepositoryName repoName) {
+      PathFragment execPath, @Nullable PathFragment baseExecPath, @Nullable Root baseRoot) {
     PathFragment dir = execPath.getParentDirectory();
     if (dir == null) {
       return null;
     }
 
+    RepositoryName repoName = RepositoryName.MAIN;
+
     Pair<RepositoryName, PathFragment> repo = RepositoryName.fromPathFragment(dir);
     if (repo != null) {
       repoName = repo.getFirst();
       dir = repo.getSecond();
     }
 
-    while (packageRoots != null && dir != null && !dir.equals(baseExecPath)) {
+    while (dir != null && !dir.equals(baseExecPath)) {
       Root sourceRoot = packageRoots.get(PackageIdentifier.create(repoName, dir));
       if (sourceRoot != null) {
         return sourceRoot;
@@ -356,8 +348,9 @@ private Root findSourceRoot(
   }
 
   @Override
-  public Artifact resolveSourceArtifact(PathFragment execPath, RepositoryName repositoryName) {
-    return resolveSourceArtifactWithAncestor(execPath, null, null, repositoryName);
+  public Artifact resolveSourceArtifact(PathFragment execPath,
+      @SuppressWarnings("unused") RepositoryName repositoryName) {
+    return resolveSourceArtifactWithAncestor(execPath, null, null);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/actions/Root.java

@@ -170,7 +170,7 @@ boolean isMiddlemanRoot() {
     return isMiddlemanRoot;
   }
 
-  boolean isMainRepo() {
+  public boolean isMainRepo() {
     return isMainRepo;
   }
 
@@ -181,7 +181,7 @@ public int compareTo(Root o) {
 
   @Override
   public int hashCode() {
-    return Objects.hash(execRoot, path.hashCode(), isMainRepo);
+    return Objects.hash(execRoot, path.hashCode());
   }
 
   @Override
@@ -193,9 +193,7 @@ public boolean equals(Object o) {
       return false;
     }
     Root r = (Root) o;
-    return path.equals(r.path)
-        && Objects.equals(execRoot, r.execRoot)
-        && Objects.equals(isMainRepo, r.isMainRepo);
+    return path.equals(r.path) && Objects.equals(execRoot, r.execRoot);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java

@@ -123,7 +123,7 @@ public static NestedSet<Artifact> getMiddlemanFor(RuleContext rule, String attri
    * <p>For example "//pkg:target" -> "pkg/&lt;fragment&gt;/target.
    */
   public static PathFragment getUniqueDirectory(Label label, PathFragment fragment) {
-    return label.getPackageIdentifier().getPackageFragment().getRelative(fragment)
+    return label.getPackageIdentifier().getSourceRoot().getRelative(fragment)
         .getRelative(label.getName());
   }
 

src/main/java/com/google/devtools/build/lib/analysis/CompilationHelper.java

@@ -54,8 +54,8 @@ private static List<Artifact> getMiddlemanInternal(AnalysisEnvironment env,
     }
     MiddlemanFactory factory = env.getMiddlemanFactory();
     return ImmutableList.of(factory.createMiddlemanAllowMultiple(
-        env, actionOwner, ruleContext.getRule().getLabel().getPackageIdentifier().getSourceRoot(),
-        purpose, filesToBuild, ruleContext.getConfiguration().getMiddlemanDirectory(
+        env, actionOwner, ruleContext.getPackageDirectory(), purpose, filesToBuild,
+        ruleContext.getConfiguration().getMiddlemanDirectory(
             ruleContext.getRule().getRepository())));
   }
 
@@ -87,8 +87,7 @@ private static List<Artifact> getMiddlemanInternal(AnalysisEnvironment env,
     MiddlemanFactory factory = env.getMiddlemanFactory();
     Iterable<Artifact> artifacts = dep.getProvider(FileProvider.class).getFilesToBuild();
     return ImmutableList.of(
-        factory.createMiddlemanAllowMultiple(env, actionOwner,
-            ruleContext.getRule().getLabel().getPackageIdentifier().getSourceRoot(),
+        factory.createMiddlemanAllowMultiple(env, actionOwner, ruleContext.getPackageDirectory(),
             purpose, artifacts, ruleContext.getConfiguration().getMiddlemanDirectory(
                 ruleContext.getRule().getRepository())));
   }

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

@@ -143,7 +143,9 @@ private Artifact getOutputArtifact(OutputFile outputFile, BuildConfiguration con
         : configuration.getGenfilesDirectory(rule.getRepository());
     ArtifactOwner owner =
         new ConfiguredTargetKey(rule.getLabel(), configuration.getArtifactOwnerConfiguration());
-    PathFragment rootRelativePath = outputFile.getLabel().toPathFragment();
+    PathFragment rootRelativePath =
+        outputFile.getLabel().getPackageIdentifier().getSourceRoot().getRelative(
+            outputFile.getLabel().getName());
     Artifact result = isFileset
         ? artifactFactory.getFilesetArtifact(rootRelativePath, root, owner)
         : artifactFactory.getDerivedArtifact(rootRelativePath, root, owner);
@@ -194,7 +196,7 @@ public final ConfiguredTarget createConfiguredTarget(AnalysisEnvironment analysi
     } else if (target instanceof InputFile) {
       InputFile inputFile = (InputFile) target;
       Artifact artifact = artifactFactory.getSourceArtifact(
-          inputFile.getLabel().toPathFragment(),
+          inputFile.getExecPath(),
           Root.asSourceRoot(inputFile.getPackage().getSourceRoot(),
               inputFile.getPackage().getPackageIdentifier().getRepository().isMain()),
           new ConfiguredTargetKey(target.getLabel(), config));

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -609,7 +609,7 @@ public Artifact getPackageRelativeArtifact(PathFragment relative, Root root) {
    * {@link #getUniqueDirectoryArtifact(String, PathFragment, Root)}) ensures that this is the case.
    */
   public PathFragment getPackageDirectory() {
-    return getLabel().getPackageIdentifier().getPackageFragment();
+    return getLabel().getPackageIdentifier().getSourceRoot();
   }
 
   /**