diff options
author | Laszlo Csomor <laszlocsomor@google.com> | 2016-09-20 15:40:42 +0000 |
---|---|---|
committer | Laszlo Csomor <laszlocsomor@google.com> | 2016-09-21 07:06:32 +0000 |
commit | 8539a1215bb58211c7c643005d2389ecafa6f580 (patch) | |
tree | 87783de1c37833261051d4c540e52ec81fcbdbfe /src/test/java | |
parent | 63010255876a81cf2b0bc4fc5d95a0e1a99df58d (diff) |
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
Diffstat (limited to 'src/test/java')
9 files changed, 68 insertions, 77 deletions
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 734f7d4f09..f8bacacf9a 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.Iterables; 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 @@ public class RunfilesTest extends FoundationTestCase { @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 @@ public class RunfilesTest extends FoundationTestCase { 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 f9daedb462..2ea8887392 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.analysis.config.ConfigurationFactory; 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 @@ public abstract class AnalysisTestCase extends FoundationTestCase { 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 5e0cda6d61..d86d2be261 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.LabelSyntaxException; 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 @@ public class SymlinkForestTest { @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 @@ public class SymlinkForestTest { 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 @@ public class SymlinkForestTest { .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 @@ public class SymlinkForestTest { @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 @@ public class SymlinkForestTest { .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 @@ public class SymlinkForestTest { .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 42642b8851..a19ef4b7a3 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.assertEquals; 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 @@ public class PackageIdentifierTest { 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 @@ public class PackageIdentifierTest { } @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 24a19172c7..9a39bbfda1 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 @@ public class RepositoryNameTest { } @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 5b3d20ca95..1fab14fa9b 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 @@ public class CcCommonTest extends BuildViewTestCase { 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 e476138dbe..2099df68a1 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 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase { @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 af79883d2a..00b27d247e 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 @@ public class SkylarkRuleContextTest extends SkylarkTestCase { 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 ba50706f2e..8db1105771 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 @@ public class TestConstants { "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"; |