diff options
author | 2018-01-15 15:46:45 -0800 | |
---|---|---|
committer | 2018-01-15 15:48:22 -0800 | |
commit | 15b532652e446472d6a61af1ba73c8aca170500d (patch) | |
tree | 19d76d505842667cc4bcc2fa2a0da948282a0ef6 /src | |
parent | 7c439b9f6f67939c57fe321958878972869e12e0 (diff) |
Remove test methods from ArtifactFactory and Root that violate root invariants.
In this case, the invariant violated is creating derived roots at the exec root.
PiperOrigin-RevId: 181994080
Diffstat (limited to 'src')
4 files changed, 4 insertions, 48 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java index 569d236b18..37bd6f48b6 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java @@ -157,24 +157,6 @@ public class ArtifactFactory implements ArtifactResolver { return getSourceArtifact(execPath, root, ArtifactOwner.NULL_OWNER); } - /** - * Only for use by BinTools! Returns an artifact for a tool at the given path - * fragment, relative to the exec root, creating it if not found. This method - * only works for normalized, relative paths. - */ - public Artifact getDerivedArtifact(PathFragment execPath, Path execRoot) { - Preconditions.checkArgument(!execPath.isAbsolute(), execPath); - Preconditions.checkArgument(execPath.isNormalized(), execPath); - // TODO(bazel-team): Check that either BinTools do not change over the life of the Blaze server, - // or require that a legitimate ArtifactOwner be passed in here to allow for ownership. - return getArtifact( - execRoot.getRelative(execPath), - Root.execRootAsDerivedRoot(execRoot), - execPath, - ArtifactOwner.NULL_OWNER, - null); - } - private void validatePath(PathFragment rootRelativePath, Root root) { Preconditions.checkArgument(!root.isSourceRoot()); Preconditions.checkArgument(!rootRelativePath.isAbsolute(), rootRelativePath); diff --git a/src/main/java/com/google/devtools/build/lib/actions/Root.java b/src/main/java/com/google/devtools/build/lib/actions/Root.java index e6451c9e41..5718478327 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Root.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Root.java @@ -102,15 +102,6 @@ public final class Root implements Comparable<Root>, Serializable, SkylarkValue return new Root(execRoot, root, true); } - /** - * Returns the exec root as a derived root. The exec root should never be treated as a derived - * root, but this is currently allowed. Do not add any further uses besides the ones that already - * exist! - */ - static Root execRootAsDerivedRoot(Path execRoot) { - return new Root(execRoot, execRoot); - } - @Nullable private final Path execRoot; private final Path path; private final boolean isMiddlemanRoot; diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionLookupValueTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionLookupValueTest.java index 3bcf87b68b..5ae6c377e5 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionLookupValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ActionLookupValueTest.java @@ -21,6 +21,7 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import org.junit.Before; @@ -55,7 +56,9 @@ public class ActionLookupValueTest { @Test public void testActionNotPresentAfterEvaluation() { - Root root = Root.asDerivedRoot(fs.getRootDirectory()); + Path execRoot = fs.getPath("/execroot"); + Path outputRootPath = execRoot.getRelative("blaze-out"); + Root root = Root.asDerivedRoot(execRoot, outputRootPath); Action normalAction = mock(Action.class); Artifact normalArtifact = new Artifact(PathFragment.create("normal"), root); when(normalAction.getOutputs()).thenReturn(ImmutableSet.of(normalArtifact)); diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java index a3b6f6c1e2..5c6005d1cb 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java @@ -210,26 +210,6 @@ public class ArtifactFactoryTest { } } - @Test - public void testGetDerivedArtifact() throws Exception { - PathFragment toolPath = PathFragment.create("_bin/tool"); - Artifact artifact = artifactFactory.getDerivedArtifact(toolPath, execRoot); - assertThat(artifact.getExecPath()).isEqualTo(toolPath); - assertThat(artifact.getRoot()).isEqualTo(Root.asDerivedRoot(execRoot)); - assertThat(artifact.getPath()).isEqualTo(execRoot.getRelative(toolPath)); - assertThat(artifact.getOwner()).isNull(); - } - - @Test - public void testGetDerivedArtifactFailsForAbsolutePath() throws Exception { - try { - artifactFactory.getDerivedArtifact(PathFragment.create("/_bin/b"), execRoot); - fail(); - } catch (IllegalArgumentException e) { - // Expected exception - } - } - private static class MockPackageRootResolver implements PackageRootResolver { private Map<PathFragment, Root> packageRoots = Maps.newHashMap(); |