diff options
author | Dmitry Lomov <dslomov@google.com> | 2018-05-30 04:34:08 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-05-30 04:35:42 -0700 |
commit | 5b1ce4d5d7568ecacf02c63c30a9cc7ce7ef24d3 (patch) | |
tree | eee571e4bf5d2dfc304084808d449febce5f2b8a /src/test/java/com | |
parent | 1973be49ca38a17e5272e8af1d0ba6b00e442d1f (diff) |
Fix `equals()` and `hashCode()` for artifacts: artifacts of different classes are not equal.
Also validate that there are no tree and file artifacts with the same exec path.
Fixes #4668.
Closes #5284.
Change-Id: Id97c0407a476a5bfc697b4ca7b858e3d0c0f8c75
PiperOrigin-RevId: 198540425
Diffstat (limited to 'src/test/java/com')
5 files changed, 82 insertions, 3 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java index a86066d1a5..51c0fcf6e3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java @@ -194,6 +194,11 @@ public final class AnalysisTestUtil { } @Override + public ImmutableSet<Artifact> getTreeArtifactsConflictingWithFiles() { + return original.getTreeArtifactsConflictingWithFiles(); + } + + @Override public ActionKeyContext getActionKeyContext() { return original.getActionKeyContext(); } @@ -398,7 +403,12 @@ public final class AnalysisTestUtil { @Override public ImmutableSet<Artifact> getOrphanArtifacts() { - return ImmutableSet.<Artifact>of(); + return ImmutableSet.of(); + } + + @Override + public ImmutableSet<Artifact> getTreeArtifactsConflictingWithFiles() { + return ImmutableSet.of(); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index e687bc260f..360717f5fa 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -1142,6 +1142,32 @@ public abstract class BuildViewTestCase extends FoundationTestCase { } /** + * Gets a tree artifact, creating it if necessary. {@code ArtifactOwner} should be a genuine + * {@link ConfiguredTargetKey} corresponding to a {@link ConfiguredTarget}. If called from a test + * that does not exercise the analysis phase, the convenience methods {@link + * #getBinArtifactWithNoOwner} or {@link #getGenfilesArtifactWithNoOwner} should be used instead. + */ + protected Artifact getTreeArtifact( + PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) { + return view.getArtifactFactory().getTreeArtifact(rootRelativePath, root, owner); + } + + /** + * Gets a Tree Artifact for testing in the subdirectory of the {@link + * BuildConfiguration#getBinDirectory} corresponding to the package of {@code owner}. So to + * specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should just be + * "foo.o". + */ + protected final Artifact getTreeArtifact(String packageRelativePath, ConfiguredTarget owner) { + return getPackageRelativeTreeArtifact( + packageRelativePath, + getConfiguration(owner).getBinDirectory(RepositoryName.MAIN), + ConfiguredTargetKey.of( + owner, skyframeExecutor.getConfiguration(reporter, owner.getConfigurationKey()))); + } + + + /** * Gets a derived Artifact for testing with path of the form * root/owner.getPackageFragment()/packageRelativePath. * @@ -1155,6 +1181,20 @@ public abstract class BuildViewTestCase extends FoundationTestCase { } /** + * Gets a tree Artifact for testing with path of the form + * root/owner.getPackageFragment()/packageRelativePath. + * + * @see #getDerivedArtifact(PathFragment, ArtifactRoot, ArtifactOwner) + */ + private Artifact getPackageRelativeTreeArtifact( + String packageRelativePath, ArtifactRoot root, ArtifactOwner owner) { + return getTreeArtifact( + owner.getLabel().getPackageFragment().getRelative(packageRelativePath), + root, owner); + } + + + /** * Gets a derived Artifact for testing in the {@link BuildConfiguration#getBinDirectory}. This * method should only be used for tests that do no analysis, and so there is no ConfiguredTarget * to own this artifact. If the test runs the analysis phase, {@link #getBinArtifact(String, @@ -1922,6 +1962,11 @@ public abstract class BuildViewTestCase extends FoundationTestCase { } @Override + public ImmutableSet<Artifact> getTreeArtifactsConflictingWithFiles() { + throw new UnsupportedOperationException(); + } + + @Override public ActionKeyContext getActionKeyContext() { return actionKeyContext; } diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java index c82b2aeb25..b9b2f4c33c 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java @@ -894,7 +894,7 @@ public class RuleClassTest extends PackageLoadingTestCase { supportsConstraintChecking, /*requiredToolchains=*/ ImmutableSet.<Label>of(), /*supportsPlatforms=*/ true, - ImmutableList.copyOf(attributes)); + OutputFile.Kind.FILE, ImmutableList.copyOf(attributes)); } private static RuleClass createParentRuleClass() { diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java index 7d41301892..f894e4d32b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java @@ -705,7 +705,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { assertNoEvents(); SpawnAction splitAction = - getGeneratingSpawnAction(getBinArtifact("dexsplits/top", topTarget)); + getGeneratingSpawnAction(getTreeArtifact("dexsplits/top", topTarget)); assertThat(splitAction.getArguments()).contains("--main-dex-list"); assertThat(splitAction.getArguments()).contains("--minimal-main-dex"); assertThat(ActionsTestUtil.baseArtifactNames(getNonToolInputs(splitAction))) diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java index 51fa9d33f0..877b1a3f24 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java @@ -1658,6 +1658,30 @@ public class SkylarkIntegrationTest extends BuildViewTestCase { + " rule 'r' should be created by the same rule."); } + @Test + public void testFileAndDirectory() throws Exception { + scratch.file( + "ext.bzl", + "def _extrule(ctx):", + " dir = ctx.actions.declare_directory('foo/bar/baz')", + " ctx.actions.run_shell(", + " outputs = [dir],", + " command = 'mkdir -p ' + dir.path + ' && echo wtf > ' + dir.path + '/wtf.txt')", + "", + "extrule = rule(", + " _extrule,", + " outputs = {", + " 'out': 'foo/bar/baz',", + " },", + ")"); + scratch.file("BUILD", "load(':ext.bzl', 'extrule')", "", "extrule(", " name = 'test'", ")"); + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//:test"); + assertContainsEvent("ERROR /workspace/BUILD:3:1: in extrule rule //:test:"); + assertContainsEvent("he following directories were also declared as files:"); + assertContainsEvent("foo/bar/baz"); + } + /** * Skylark integration test that forces inlining. */ |