Rollback of commit 82d43279f93d95e4c41b4bc598a3cc05ddd1ae1a.
*** 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 #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
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java
index 734f7d4..f8bacac 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java
@@ -21,12 +21,15 @@
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Root;
+import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.vfs.PathFragment;
+
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
+
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -318,36 +321,37 @@
@Test
public void testLegacyRunfilesStructure() {
- Root root = Root.asSourceRoot(scratch.resolve("/workspace/external/repo"));
+ Root root = Root.asSourceRoot(scratch.resolve("/workspace"));
PathFragment workspaceName = new PathFragment("wsname");
- PathFragment pathB = new PathFragment("b");
+ PathFragment pathB = new PathFragment("external/repo/b");
Artifact artifactB = new Artifact(pathB, root);
Runfiles.ManifestBuilder builder = new Runfiles.ManifestBuilder(workspaceName, true);
Map<PathFragment, Artifact> inputManifest = Maps.newHashMap();
- inputManifest.put(new PathFragment("../repo").getRelative(pathB), artifactB);
+ inputManifest.put(pathB, artifactB);
Runfiles.ConflictChecker checker = new Runfiles.ConflictChecker(
Runfiles.ConflictPolicy.WARN, reporter, null);
builder.addUnderWorkspace(inputManifest, checker);
assertThat(builder.build().entrySet()).containsExactly(
- Maps.immutableEntry(workspaceName.getRelative("external/repo/b"), artifactB),
+ Maps.immutableEntry(workspaceName.getRelative(pathB), artifactB),
Maps.immutableEntry(new PathFragment("repo/b"), artifactB));
assertNoEvents();
}
@Test
public void testRunfileAdded() {
- Root root = Root.asSourceRoot(scratch.resolve("/workspace/external/repo"));
+ Root root = Root.asSourceRoot(scratch.resolve("/workspace"));
PathFragment workspaceName = new PathFragment("wsname");
- PathFragment pathB = new PathFragment("b");
+ PathFragment pathB = new PathFragment("external/repo/b");
Artifact artifactB = new Artifact(pathB, root);
Runfiles.ManifestBuilder builder = new Runfiles.ManifestBuilder(workspaceName, false);
- Map<PathFragment, Artifact> inputManifest = ImmutableMap.of(
- new PathFragment("../repo").getRelative(pathB), artifactB);
+ Map<PathFragment, Artifact> inputManifest = ImmutableMap.<PathFragment, Artifact>builder()
+ .put(pathB, artifactB)
+ .build();
Runfiles.ConflictChecker checker = new Runfiles.ConflictChecker(
Runfiles.ConflictPolicy.WARN, reporter, null);
builder.addUnderWorkspace(inputManifest, checker);
@@ -357,4 +361,29 @@
Maps.immutableEntry(new PathFragment("repo/b"), artifactB));
assertNoEvents();
}
+
+ // TODO(kchodorow): remove this once the default workspace name is always set.
+ @Test
+ public void testConflictWithExternal() {
+ Root root = Root.asSourceRoot(scratch.resolve("/workspace"));
+ PathFragment pathB = new PathFragment("repo/b");
+ PathFragment externalPathB = Label.EXTERNAL_PACKAGE_NAME.getRelative(pathB);
+ Artifact artifactB = new Artifact(pathB, root);
+ Artifact artifactExternalB = new Artifact(externalPathB, root);
+
+ Runfiles.ManifestBuilder builder = new Runfiles.ManifestBuilder(
+ PathFragment.EMPTY_FRAGMENT, false);
+
+ Map<PathFragment, Artifact> inputManifest = ImmutableMap.<PathFragment, Artifact>builder()
+ .put(pathB, artifactB)
+ .put(externalPathB, artifactExternalB)
+ .build();
+ Runfiles.ConflictChecker checker = new Runfiles.ConflictChecker(
+ Runfiles.ConflictPolicy.WARN, reporter, null);
+ builder.addUnderWorkspace(inputManifest, checker);
+
+ assertThat(builder.build().entrySet()).containsExactly(
+ Maps.immutableEntry(new PathFragment("repo/b"), artifactExternalB));
+ checkConflictWarning();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index f9daedb..2ea8887 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -38,7 +38,6 @@
import com.google.devtools.build.lib.buildtool.BuildRequest.BuildRequestOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
-import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.flags.InvocationPolicyEnforcer;
import com.google.devtools.build.lib.packages.PackageFactory;
@@ -190,8 +189,6 @@
loadingPhaseRunner = skyframeExecutor.getLoadingPhaseRunner(
pkgFactory.getRuleClassNames(), defaultFlags().contains(Flag.SKYFRAME_LOADING_PHASE));
buildView = new BuildView(directories, ruleClassProvider, skyframeExecutor, null);
- buildView.setArtifactRoots(
- ImmutableMap.of(PackageIdentifier.createInMainRepo(""), rootDirectory));
useConfiguration();
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
index 5e0cda6..d86d2be 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.testutil.ManualClock;
+import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -124,12 +125,7 @@
@Test
public void testDeleteTreesBelowNotPrefixed() throws IOException {
createTestDirectoryTree();
- SymlinkForest forest = SymlinkForest.builder()
- .setWorkspace(topDir)
- .setProductName("mock-name")
- .setPrefixes(new String[]{"file-"})
- .build();
- forest.deleteTreesBelowNotPrefixed();
+ SymlinkForest.deleteTreesBelowNotPrefixed(topDir, new String[]{"file-"});
assertTrue(file1.exists());
assertTrue(file2.exists());
assertFalse(aDir.exists());
@@ -191,13 +187,7 @@
Path linkRoot = fileSystem.getPath("/linkRoot");
createDirectoryAndParents(linkRoot);
- SymlinkForest.builder()
- .setLegacyExternalRunfiles(false)
- .setPackageRoots(packageRootMap)
- .setWorkspace(linkRoot)
- .setProductName("mock-name")
- .setWorkspaceName("wsname")
- .build()
+ new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, "wsname")
.plantSymlinkForest();
assertLinksTo(linkRoot, rootA, "pkgA");
@@ -225,13 +215,7 @@
.put(createPkg(rootX, rootY, "foo"), rootX)
.build();
- SymlinkForest.builder()
- .setLegacyExternalRunfiles(false)
- .setPackageRoots(packageRootMap)
- .setWorkspace(linkRoot)
- .setProductName("mock-name")
- .setWorkspaceName("wsname")
- .build()
+ new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, "wsname")
.plantSymlinkForest();
assertLinksTo(linkRoot, rootX, "file");
}
@@ -239,32 +223,24 @@
@Test
public void testRemotePackage() throws Exception {
Path outputBase = fileSystem.getPath("/ob");
- Path rootY = outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME).getRelative("y");
- Path rootZ = outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME).getRelative("z");
- Path rootW = outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME).getRelative("w");
+ Path rootY = outputBase.getRelative(Label.EXTERNAL_PATH_PREFIX).getRelative("y");
+ Path rootZ = outputBase.getRelative(Label.EXTERNAL_PATH_PREFIX).getRelative("z");
+ Path rootW = outputBase.getRelative(Label.EXTERNAL_PATH_PREFIX).getRelative("w");
createDirectoryAndParents(rootY);
- createDirectoryAndParents(rootZ);
- createDirectoryAndParents(rootW);
FileSystemUtils.createEmptyFile(rootY.getRelative("file"));
ImmutableMap<PackageIdentifier, Path> packageRootMap =
ImmutableMap.<PackageIdentifier, Path>builder()
// Remote repo without top-level package.
- .put(createPkg(outputBase, "y", "w"), rootY)
+ .put(createPkg(outputBase, "y", "w"), outputBase)
// Remote repo with and without top-level package.
- .put(createPkg(outputBase, "z", ""), rootZ)
- .put(createPkg(outputBase, "z", "a/b/c"), rootZ)
+ .put(createPkg(outputBase, "z", ""), outputBase)
+ .put(createPkg(outputBase, "z", "a/b/c"), outputBase)
// Only top-level pkg.
- .put(createPkg(outputBase, "w", ""), rootW)
+ .put(createPkg(outputBase, "w", ""), outputBase)
.build();
- SymlinkForest.builder()
- .setLegacyExternalRunfiles(false)
- .setPackageRoots(packageRootMap)
- .setWorkspace(linkRoot)
- .setProductName("mock-name")
- .setWorkspaceName("wsname")
- .build()
+ new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, "wsname")
.plantSymlinkForest();
assertFalse(linkRoot.getRelative(Label.EXTERNAL_PATH_PREFIX + "/y/file").exists());
assertLinksTo(
@@ -287,15 +263,9 @@
.put(Label.EXTERNAL_PACKAGE_IDENTIFIER, root)
.build();
- SymlinkForest.builder()
- .setLegacyExternalRunfiles(false)
- .setPackageRoots(packageRootMap)
- .setWorkspace(linkRoot)
- .setProductName("mock-name")
- .setWorkspaceName("wsname")
- .build()
+ new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, "wsname")
.plantSymlinkForest();
- assertThat(linkRoot.getRelative(Label.EXTERNAL_PACKAGE_NAME).exists()).isFalse();
+ assertThat(linkRoot.getRelative(Label.EXTERNAL_PATH_PREFIX).exists()).isFalse();
}
@Test
@@ -307,13 +277,7 @@
.put(createPkg(root, "y", "w"), root)
.build();
- SymlinkForest.builder()
- .setLegacyExternalRunfiles(true)
- .setPackageRoots(packageRootMap)
- .setWorkspace(linkRoot)
- .setProductName("mock-name")
- .setWorkspaceName("wsname")
- .build()
+ new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, "wsname")
.plantSymlinkForest();
assertThat(linkRoot.getRelative("../wsname").exists()).isTrue();
}
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
index 42642b8..a19ef4b 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
@@ -19,10 +19,12 @@
import static org.junit.Assert.assertSame;
import com.google.devtools.build.lib.vfs.PathFragment;
+
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
+
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -37,7 +39,8 @@
PackageIdentifier fooA = PackageIdentifier.parse("@foo//a");
assertThat(fooA.getRepository().strippedName()).isEqualTo("foo");
assertThat(fooA.getPackageFragment().getPathString()).isEqualTo("a");
- assertThat(fooA.getRepository().getSourceRoot()).isEqualTo(new PathFragment("external/foo"));
+ assertThat(fooA.getRepository().getSourceRoot()).isEqualTo(
+ new PathFragment("external/foo"));
PackageIdentifier absoluteA = PackageIdentifier.parse("//a");
assertThat(absoluteA.getRepository().strippedName()).isEqualTo("");
@@ -98,12 +101,10 @@
}
@Test
- public void testPathUnderExecRoot() throws Exception {
- assertThat(
- PackageIdentifier.create("@foo", new PathFragment("bar/baz")).getPathUnderExecRoot())
+ public void testRunfilesDir() throws Exception {
+ assertThat(PackageIdentifier.create("@foo", new PathFragment("bar/baz")).getRunfilesPath())
.isEqualTo(new PathFragment("../foo/bar/baz"));
- assertThat(
- PackageIdentifier.create("@", new PathFragment("bar/baz")).getPathUnderExecRoot())
+ assertThat(PackageIdentifier.create("@", new PathFragment("bar/baz")).getRunfilesPath())
.isEqualTo(new PathFragment("bar/baz"));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java
index 24a1917..9a39bbf 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java
@@ -58,12 +58,12 @@
}
@Test
- public void testPathUnderExecRoot() throws Exception {
- assertThat(RepositoryName.create("@foo").getPathUnderExecRoot())
+ public void testRunfilesDir() throws Exception {
+ assertThat(RepositoryName.create("@foo").getRunfilesPath())
.isEqualTo(new PathFragment("../foo"));
- assertThat(RepositoryName.create("@").getPathUnderExecRoot())
+ assertThat(RepositoryName.create("@").getRunfilesPath())
.isEqualTo(PathFragment.EMPTY_FRAGMENT);
- assertThat(RepositoryName.create("").getPathUnderExecRoot())
+ assertThat(RepositoryName.create("").getRunfilesPath())
.isEqualTo(PathFragment.EMPTY_FRAGMENT);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
index 5b3d20c..1fab14f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
@@ -696,7 +696,7 @@
checkError(
"test",
"bad_relative_include",
- "../.. references a path above the execution root (..).",
+ "Path references a path above the execution root.",
"cc_library(name='bad_relative_include', srcs=[], includes=['../..'])");
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
index e476138..2099df6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
@@ -1121,8 +1121,8 @@
@Test
public void testSystemIncludePathsOutsideExecutionRoot() throws Exception {
checkError("root", "a",
- "The include path '../../system' references a path outside of the execution root.",
- "cc_library(name='a', srcs=['a.cc'], copts=['-isystem../../system'])");
+ "The include path '../system' references a path outside of the execution root.",
+ "cc_library(name='a', srcs=['a.cc'], copts=['-isystem../system'])");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index af79883..00b27d2 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -340,7 +340,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("@r//:cclib");
assertContainsEvent(
- "/r/BUILD:2:10: Label '@r//:sub/my_sub_lib.h' crosses boundary of "
+ "/external/r/BUILD:2:10: Label '@r//:sub/my_sub_lib.h' crosses boundary of "
+ "subpackage '@r//sub' (perhaps you meant to put the colon here: "
+ "'@r//sub:my_sub_lib.h'?)");
}
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
index ba50706..8db1105 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
@@ -65,7 +65,7 @@
"com.google.devtools.build.lib.bazel.rules.BazelRulesModule";
public static final ImmutableList<String> IGNORED_MESSAGE_PREFIXES = ImmutableList.<String>of();
- public static final String GCC_INCLUDE_PATH = "../bazel_tools/tools/cpp/gcc3";
+ public static final String GCC_INCLUDE_PATH = "external/bazel_tools/tools/cpp/gcc3";
public static final String TOOLS_REPOSITORY = "@bazel_tools";
