Don't treat empty files specially with respect to mtime/digest.
RELNOTES: Bazel no longer regards an empty file as changed if its mtime has changed.
--
MOS_MIGRATED_REVID=127328552
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
index 789e2b1..84e455c 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java
@@ -263,8 +263,7 @@
assertEquals(1L, value.getModifiedTime());
}
- // Empty files need to store their mtimes, so touching an empty file
- // can be used to trigger rebuilds.
+ // Empty files are the same as normal files -- mtime is not stored.
@Test
public void testEmptyFile() throws Exception {
Artifact artifact = createDerivedArtifact("empty");
@@ -273,7 +272,7 @@
path.setLastModifiedTime(1L);
FileArtifactValue value = create(artifact);
assertArrayEquals(path.getMD5Digest(), value.getDigest());
- assertEquals(1L, value.getModifiedTime());
+ assertEquals(-1L, value.getModifiedTime());
assertEquals(0L, value.getSize());
}
@@ -322,8 +321,7 @@
EqualsTester equalsTester = new EqualsTester();
equalsTester
.addEqualityGroup(create(artifact1), create(artifact2), create(diffMtime))
- .addEqualityGroup(create(empty1))
- .addEqualityGroup(create(empty2), create(empty3))
+ .addEqualityGroup(create(empty1), create(empty2), create(empty3))
.addEqualityGroup(create(dir1))
.addEqualityGroup(create(dir2), create(dir3))
.testEquals();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderMediumTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderMediumTest.java
index dba91d6..3c274f9 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderMediumTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderMediumTest.java
@@ -134,7 +134,8 @@
buildArtifacts(persistentBuilder(cache), goodbye);
assertFalse(button.pressed); // not rebuilt
- FileSystemUtils.touchFile(hello.getPath());
+ hello.getPath().setWritable(true);
+ FileSystemUtils.writeContentAsLatin1(hello.getPath(), "new content");
button.pressed = false;
buildArtifacts(persistentBuilder(cache), goodbye);
@@ -364,7 +365,8 @@
buildArtifacts(persistentBuilder(cache), hello);
assertFalse(button.pressed); // not rebuilt
- BlazeTestUtils.changeModtime(hello.getPath());
+ hello.getPath().setWritable(true);
+ FileSystemUtils.writeContentAsLatin1(hello.getPath(), "new content");
button.pressed = false;
buildArtifacts(persistentBuilder(cache), hello);
@@ -394,7 +396,8 @@
buildArtifacts(persistentBuilder(cache), hello);
assertFalse(button.pressed); // not rebuilt
- BlazeTestUtils.changeModtime(hello.getPath());
+ hello.getPath().setWritable(true);
+ FileSystemUtils.writeContentAsLatin1(hello.getPath(), "new content");
button.pressed = false;
buildArtifacts(persistentBuilder(cache), hello);
@@ -440,7 +443,8 @@
buildArtifacts(persistentBuilder(cache), hello);
assertFalse(button.pressed); // not rebuilt
- BlazeTestUtils.changeModtime(hello.getPath());
+ hello.getPath().setWritable(true);
+ FileSystemUtils.writeContentAsLatin1(hello.getPath(), "new content");
button.pressed = false;
buildArtifacts(persistentBuilder(cache), hello);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTest.java
index 55f1784..ea10d33 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTest.java
@@ -169,8 +169,8 @@
buildArtifacts(cachingBuilder(), goodbye);
assertFalse(button.pressed); // not rebuilt
- // inMemoryMetadataCache.useFileDigest is false, so new timestamp is enough to force a rebuild.
- FileSystemUtils.touchFile(hello.getPath());
+ hello.getPath().setWritable(true);
+ FileSystemUtils.writeContentAsLatin1(hello.getPath(), "new content");
button.pressed = false;
buildArtifacts(cachingBuilder(), goodbye);
@@ -231,10 +231,10 @@
buildArtifacts(cachingBuilder(), hello);
assertFalse(button.pressed); // not rebuilt
- // Touching the *output* file 'hello' causes 'action' to re-execute, to
- // make things consistent again; this is not what Make would do, but it is
- // correct according to this Builder.
- BlazeTestUtils.changeModtime(hello.getPath());
+ // Changing the *output* file 'hello' causes 'action' to re-execute, to make things consistent
+ // again.
+ hello.getPath().setWritable(true);
+ FileSystemUtils.writeContentAsLatin1(hello.getPath(), "new content");
button.pressed = false;
buildArtifacts(cachingBuilder(), hello);
@@ -251,7 +251,8 @@
Artifact hello = createSourceArtifact("hello");
BlazeTestUtils.makeEmptyFile(hello.getPath());
Artifact wazuup = createDerivedArtifact("wazuup");
- Button button1 = createActionButton(Sets.newHashSet(hello), Sets.newHashSet(wazuup));
+ Button button1 = new Button();
+ registerAction(new CopyingAction(button1, hello, wazuup));
Artifact goodbye = createDerivedArtifact("goodbye");
Button button2 = createActionButton(Sets.newHashSet(wazuup), Sets.newHashSet(goodbye));
@@ -275,7 +276,8 @@
assertFalse(button1.pressed); // wazuup not rebuilt
assertFalse(button2.pressed); // goodbye not rebuilt
- FileSystemUtils.touchFile(hello.getPath());
+ hello.getPath().setWritable(true);
+ FileSystemUtils.writeContentAsLatin1(hello.getPath(), "new content");
button1.pressed = button2.pressed = false;
buildArtifacts(cachingBuilder(), goodbye);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index 51fa9cfa..2c1b7e6 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -19,6 +19,7 @@
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Range;
import com.google.common.collect.Sets;
@@ -26,6 +27,8 @@
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionCacheChecker;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionLogBufferPathGenerator;
import com.google.devtools.build.lib.actions.Artifact;
@@ -54,6 +57,7 @@
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileSystem;
+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 com.google.devtools.build.skyframe.CycleInfo;
@@ -71,6 +75,7 @@
import org.junit.Before;
+import java.io.IOException;
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Collection;
@@ -357,6 +362,26 @@
}
}
+ /** {@link TestAction} that copies its single input to its single output. */
+ protected static class CopyingAction extends TestAction {
+ CopyingAction(Runnable effect, Artifact input, Artifact output) {
+ super(effect, ImmutableSet.of(input), ImmutableSet.of(output));
+ }
+
+ @Override
+ public void execute(ActionExecutionContext actionExecutionContext)
+ throws ActionExecutionException {
+ super.execute(actionExecutionContext);
+ try {
+ FileSystemUtils.copyFile(
+ Iterables.getOnlyElement(getInputs()).getPath(),
+ Iterables.getOnlyElement(getOutputs()).getPath());
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+ }
+
protected static class InMemoryActionCache implements ActionCache {
private final Map<String, Entry> actionCache = new HashMap<>();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index 0d6d172..b68a86c 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -260,11 +260,7 @@
assertTrue(buttonTwo.pressed);
}
- /**
- * TreeArtifacts don't care about mtime, even when the file is empty.
- * However, actions taking input non-Tree artifacts still care about mtime
- * (although this behavior should go away).
- */
+ /** TreeArtifacts don't care about mtime, even when the file is empty. */
@Test
public void testMTimeForTreeArtifactsDoesNotMatter() throws Exception {
// For this test, we only touch the input file.
@@ -291,9 +287,8 @@
buttonOne.pressed = buttonTwo.pressed = false;
touchFile(in);
buildArtifact(outTwo);
- // Per existing behavior, mtime matters for empty file Artifacts.
- assertTrue(buttonOne.pressed);
- // But this should be cached.
+ // mtime does not matter.
+ assertFalse(buttonOne.pressed);
assertFalse(buttonTwo.pressed);
// None of the below following should result in anything being built.
