Rollback of commit 4b73e972d909bcd533f2f9940f95a00b9b73bdde.

*** Reason for rollback ***

Broke tests on CI: http://ci.bazel.io/job/bazel-tests/570/


*** Original change description ***

Roll forward execroot change

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. Cust...

--
PiperOrigin-RevId: 147833177
MOS_MIGRATED_REVID=147833177

14 files changed, 72 insertions, 59 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
index b5aff73920..d4a15834e8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceType;
 import com.google.devtools.build.lib.vfs.PathFragment;
+
 import java.util.Collection;
 import java.util.LinkedHashSet;
 
@@ -162,7 +163,8 @@ public final class LocalResourceContainer {
         PathFragment assetsDir, Iterable<? extends TransitiveInfoCollection> targets) {
       for (TransitiveInfoCollection target : targets) {
         for (Artifact file : target.getProvider(FileProvider.class).getFilesToBuild()) {
-          PathFragment packageFragment = file.getArtifactOwner().getLabel().getPackageFragment();
+          PathFragment packageFragment = file.getArtifactOwner().getLabel()
+              .getPackageIdentifier().getSourceRoot();
           PathFragment packageRelativePath =
               file.getRootRelativePath().relativeTo(packageFragment);
           if (packageRelativePath.startsWith(assetsDir)) {
@@ -190,7 +192,8 @@ public final class LocalResourceContainer {
       Artifact lastFile = null;
       for (FileProvider target : targets) {
         for (Artifact file : target.getFilesToBuild()) {
-          PathFragment packageFragment = file.getArtifactOwner().getLabel().getPackageFragment();
+          PathFragment packageFragment = file.getArtifactOwner().getLabel()
+              .getPackageIdentifier().getSourceRoot();
           PathFragment packageRelativePath =
               file.getRootRelativePath().relativeTo(packageFragment);
           PathFragment resourceDir = findResourceDir(file);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index e9ef99984f..597e5dd1a8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -414,7 +414,7 @@ public final class CcCommon {
   List<PathFragment> getSystemIncludeDirs() {
     List<PathFragment> result = new ArrayList<>();
     PackageIdentifier packageIdentifier = ruleContext.getLabel().getPackageIdentifier();
-    PathFragment pathUnderExecRoot = packageIdentifier.getPathUnderExecRoot();
+    PathFragment packageFragment = packageIdentifier.getPathUnderExecRoot();
     for (String includesAttr : ruleContext.attributes().get("includes", Type.STRING_LIST)) {
       includesAttr = ruleContext.expandMakeVariables("includes", includesAttr);
       if (includesAttr.startsWith("/")) {
@@ -422,14 +422,10 @@ public final class CcCommon {
             "ignoring invalid absolute path '" + includesAttr + "'");
         continue;
       }
-      PathFragment includesPath = pathUnderExecRoot.getRelative(includesAttr).normalize();
-      // It's okay for the includes path to start with ../workspace-name for external repos.
-      if ((packageIdentifier.getRepository().isMain() && !includesPath.isNormalized())
-          || (!packageIdentifier.getRepository().isMain()
-              && !includesPath.startsWith(
-                  packageIdentifier.getRepository().getPathUnderExecRoot()))) {
+      PathFragment includesPath = packageFragment.getRelative(includesAttr).normalize();
+      if (!includesPath.isNormalized()) {
         ruleContext.attributeError("includes",
-            includesAttr + " references a path above the execution root (" + includesPath + ").");
+            "Path references a path above the execution root.");
       }
       if (includesPath.segmentCount() == 0) {
         ruleContext.attributeError(
@@ -439,7 +435,7 @@ public final class CcCommon {
                 + "' resolves to the workspace root, which would allow this rule and all of its "
                 + "transitive dependents to include any file in your workspace. Please include only"
                 + " what you need");
-      } else if (!includesPath.startsWith(pathUnderExecRoot)) {
+      } else if (!includesPath.startsWith(packageFragment)) {
         ruleContext.attributeWarning(
             "includes",
             "'"
@@ -447,14 +443,11 @@ public final class CcCommon {
                 + "' resolves to '"
                 + includesPath
                 + "' not below the relative path of its package '"
-                + pathUnderExecRoot
+                + packageFragment
                 + "'. This will be an error in the future");
       }
       result.add(includesPath);
-      result.add(packageIdentifier.getRepository().getPathUnderExecRoot()
-          .getRelative(ruleContext.getConfiguration().getGenfilesFragment())
-          .getRelative(packageIdentifier.getPackageFragment())
-          .getRelative(includesAttr));
+      result.add(ruleContext.getConfiguration().getGenfilesFragment().getRelative(includesPath));
     }
     return result;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index faed073638..11fcd4777a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -1264,7 +1264,7 @@ public final class CcLibraryHelper {
         ruleContext.getLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot();
     contextBuilder.addQuoteIncludeDir(repositoryPath);
     contextBuilder.addQuoteIncludeDir(
-        repositoryPath.getRelative(ruleContext.getConfiguration().getGenfilesFragment()));
+        ruleContext.getConfiguration().getGenfilesFragment().getRelative(repositoryPath));
 
     for (PathFragment systemIncludeDir : systemIncludeDirs) {
       contextBuilder.addSystemIncludeDir(systemIncludeDir);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index 7679a99715..cc83edd40f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -475,14 +475,13 @@ public class CppCompileActionBuilder {
         continue;
       }
       // One starting ../ is okay for getting to a sibling repository.
-      PathFragment originalInclude = include;
       if (include.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX))) {
         include = include.relativeTo(Label.EXTERNAL_PATH_PREFIX);
       }
-      if (include.isAbsolute() || !include.normalize().isNormalized()) {
+      if (include.isAbsolute()
+          || !PathFragment.EMPTY_FRAGMENT.getRelative(include).normalize().isNormalized()) {
         ruleContext.ruleError(
-            "The include path '" + originalInclude
-                + "' references a path outside of the execution root.");
+            "The include path '" + include + "' references a path outside of the execution root.");
       }
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 1db3f3af5e..f5291b8311 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -519,6 +519,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     PathFragment defaultSysroot = toolchain.getBuiltinSysroot().length() == 0
         ? null
         : new PathFragment(toolchain.getBuiltinSysroot());
+    if ((defaultSysroot != null) && !defaultSysroot.isNormalized()) {
+      throw new IllegalArgumentException("The built-in sysroot '" + defaultSysroot
+          + "' is not normalized.");
+    }
 
     if ((cppOptions.libcTop != null) && (defaultSysroot == null)) {
       throw new InvalidConfigurationException("The selected toolchain " + toolchainIdentifier
@@ -1061,7 +1065,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     if (packageEndIndex != -1 && s.startsWith(PACKAGE_START)) {
       String packageString = s.substring(PACKAGE_START.length(), packageEndIndex);
       try {
-        pathPrefix = PackageIdentifier.parse(packageString).getPathUnderExecRoot();
+        pathPrefix = PackageIdentifier.parse(packageString).getSourceRoot();
       } catch (LabelSyntaxException e) {
         throw new InvalidConfigurationException("The package '" + packageString + "' is not valid");
       }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 6b3604dd2e..8541778e59 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -491,7 +491,7 @@ public class CppHelper {
         Preconditions.checkState(Link.SHARED_LIBRARY_FILETYPES.matches(artifact.getFilename()));
         symlinkedArtifacts.add(isCppRuntime
             ? SolibSymlinkAction.getCppRuntimeSymlink(
-            ruleContext, artifact, solibDirOverride, configuration)
+                ruleContext, artifact, solibDirOverride, configuration)
             : SolibSymlinkAction.getDynamicLibrarySymlink(
                 ruleContext, artifact, false, true, configuration));
       }
@@ -500,8 +500,7 @@ public class CppHelper {
     }
     return ImmutableList.of(
         factory.createMiddlemanAllowMultiple(ruleContext.getAnalysisEnvironment(), actionOwner,
-            ruleContext.getLabel().getPackageIdentifier().getPathUnderExecRoot(), purpose,
-            artifacts,
+            ruleContext.getPackageDirectory(), purpose, artifacts,
             configuration.getMiddlemanDirectory(ruleContext.getRule().getRepository())));
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index 3e0b716b58..8fd5728a56 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -50,7 +50,6 @@ import com.google.devtools.build.lib.rules.cpp.Link.Staticness;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -1541,9 +1540,8 @@ public class CppLinkActionBuilder {
       for (LinkerInput input : linkerInputs) {
         if (input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY) {
           PathFragment libDir = input.getArtifact().getExecPath().getParentDirectory();
-          Path rootPath = input.getArtifact().getRoot().getExecRoot();
           Preconditions.checkState(
-              rootPath.getRelative(libDir).startsWith(rootPath.getRelative(solibDir)),
+              libDir.startsWith(solibDir),
               "Artifact '%s' is not under directory '%s'.",
               input.getArtifact(),
               solibDir);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
index 847c1f5d4b..5b59a0601f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
@@ -20,7 +20,6 @@ import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactResolver;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -100,10 +99,10 @@ public class HeaderDiscovery {
       return inputs.build();
     }
     List<Path> systemIncludePrefixes = permittedSystemIncludePrefixes;
+
     // Check inclusions.
     IncludeProblems problems = new IncludeProblems();
     for (Path execPath : depSet.getDependencies()) {
-      RepositoryName repositoryName = RepositoryName.MAIN;
       PathFragment execPathFragment = execPath.asFragment();
       if (execPathFragment.isAbsolute()) {
         // Absolute includes from system paths are ignored.
@@ -115,24 +114,15 @@ public class HeaderDiscovery {
         // are, it's probably due to a non-hermetic #include, & we should stop
         // the build with an error.
         if (execPath.startsWith(execRoot)) {
-          // Funky but tolerable path.
-          execPathFragment = execPath.relativeTo(execRoot);
-        } else if (execPath.startsWith(execRoot.getParentDirectory())) {
-          // External repository.
-          execPathFragment = execPath.relativeTo(execRoot.getParentDirectory());
-          String workspace = execPathFragment.getSegment(0);
-          execPathFragment = execPathFragment.relativeTo(workspace);
-          try {
-            repositoryName = RepositoryName.create("@" + workspace);
-          } catch (LabelSyntaxException e) {
-            throw new IllegalStateException(workspace + " is not a valid repository name");
-          }
+          execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path
+        } else {
+          problems.add(execPathFragment.getPathString());
+          continue;
         }
       }
-      Artifact artifact = allowedDerivedInputsMap.get(
-          repositoryName.getPathUnderExecRoot().getRelative(execPathFragment));
+      Artifact artifact = allowedDerivedInputsMap.get(execPathFragment);
       if (artifact == null) {
-        artifact = artifactResolver.resolveSourceArtifact(execPathFragment, repositoryName);
+        artifact = artifactResolver.resolveSourceArtifact(execPathFragment, RepositoryName.MAIN);
       }
       if (artifact != null) {
         inputs.add(artifact);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
index fd77e0cda9..8abcaef9d8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
@@ -280,7 +280,18 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect
     }
 
     private void createProtoCompileAction(SupportData supportData, Collection<Artifact> outputs) {
-      String genfilesPath = ruleContext.getBinOrGenfilesDirectory().getExecPathString();
+      String genfilesPath =
+          ruleContext
+              .getConfiguration()
+              .getGenfilesFragment()
+              .getRelative(
+                  ruleContext
+                      .getLabel()
+                      .getPackageIdentifier()
+                      .getRepository()
+                      .getPathUnderExecRoot())
+              .getPathString();
+
       ImmutableList.Builder<ToolchainInvocation> invocations = ImmutableList.builder();
       invocations.add(
           new ToolchainInvocation("C++", checkNotNull(getProtoToolchainProvider()), genfilesPath));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
index e4cf0da886..a81a38ff1b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
@@ -34,7 +34,6 @@ import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -337,9 +336,9 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory {
           } else {
             dir = ruleContext.getConfiguration().getGenfilesFragment();
           }
-          PackageIdentifier pkgId = ruleContext.getRule().getLabel().getPackageIdentifier();
-          return pkgId.getRepository().getPathUnderExecRoot().getRelative(dir)
-              .getRelative(pkgId.getPackageFragment()).getPathString();
+          PathFragment relPath =
+              ruleContext.getRule().getLabel().getPackageIdentifier().getSourceRoot();
+          return dir.getRelative(relPath).getPathString();
         }
       } else if (JDK_MAKE_VARIABLE.matcher("$(" + name + ")").find()) {
         return new ConfigurationMakeVariableContext(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
index 06f4defac6..344793423e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
@@ -504,7 +504,7 @@ public class JavaCommon {
     if (launcher != null) {
       javaExecutable = launcher.getRootRelativePath();
     } else {
-      javaExecutable = ruleContext.getFragment(Jvm.class).getJavaExecutable();
+      javaExecutable = ruleContext.getFragment(Jvm.class).getRunfilesJavaExecutable();
     }
 
     if (!javaExecutable.isAbsolute()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
index d4bbad60a2..919538077f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
@@ -69,12 +69,7 @@ public final class Jvm extends BuildConfiguration.Fragment {
   @SkylarkCallable(name = "java_executable", structField = true,
       doc = "The java executable, i.e. bin/java relative to the Java home.")
   public PathFragment getJavaExecutable() {
-    if (jvmLabel == null || jvmLabel.getPackageIdentifier().getRepository().isMain()) {
-      return java;
-    } else {
-      return jvmLabel.getPackageIdentifier().getRepository().getPathUnderExecRoot()
-          .getRelative(BIN_JAVA);
-    }
+    return java;
   }
 
   /**
@@ -88,6 +83,17 @@ public final class Jvm extends BuildConfiguration.Fragment {
     return jvmLabel;
   }
 
+  /**
+   * If possible, resolves java relative to the jvmLabel's repository. Otherwise, returns the
+   * same thing as getJavaExecutable().
+   */
+  public PathFragment getRunfilesJavaExecutable() {
+    if (jvmLabel == null || jvmLabel.getPackageIdentifier().getRepository().isMain()) {
+      return getJavaExecutable();
+    }
+    return jvmLabel.getPackageIdentifier().getRepository().getRunfilesPath().getRelative(BIN_JAVA);
+  }
+
   @Override
   public void addGlobalMakeVariables(Builder<String, String> globalMakeEnvBuilder) {
     globalMakeEnvBuilder.put("JAVABASE", getJavaHome().getPathString());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
index e6ae20c98d..6501b31aac 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
@@ -149,7 +149,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor
     if (javaBase.getPackageIdentifier().getRepository().isDefault()) {
       return javaBase.getPackageFragment();
     }
-    return javaBase.getPackageIdentifier().getPathUnderExecRoot();
+    return javaBase.getPackageIdentifier().getSourceRoot();
   }
 
   private static Jvm createLegacy(String javaHome) throws InvalidConfigurationException {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
index 64594ad1e7..fd1f95f02e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
@@ -340,7 +340,18 @@ public class ProtoCompileActionBuilder {
     public Iterable<String> argv() {
       ImmutableList.Builder<String> builder = ImmutableList.builder();
       for (Artifact artifact : transitiveImports) {
-        builder.add("-I" + artifact.getRootRelativePath() + "=" + artifact.getExecPathString());
+        builder.add(
+            "-I"
+                + artifact
+                    .getRootRelativePath()
+                    .relativeTo(
+                        artifact
+                            .getOwnerLabel()
+                            .getPackageIdentifier()
+                            .getRepository()
+                            .getPathUnderExecRoot())
+                + "="
+                + artifact.getExecPathString());
       }
       if (protosInDirectDependencies != null) {
         ArrayList<String> rootRelativePaths = new ArrayList<>();