From 4f647e859203dc3a2f26ab088b95ef1a58c2e3ea Mon Sep 17 00:00:00 2001 From: ulfjack Date: Tue, 9 May 2017 07:45:40 -0400 Subject: Change FileContentsProxy to use ctime instead of mtime This gives us better reliability for detecting file changes; especially in cases where tools intentionally do not update mtime. Fixes #1525. PiperOrigin-RevId: 155490849 --- .../build/lib/skyframe/FileContentsProxyTest.java | 100 +++++++++++++++++++++ .../build/lib/skyframe/FileFunctionTest.java | 6 +- .../build/lib/skyframe/TreeArtifactBuildTest.java | 7 +- 3 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/skyframe/FileContentsProxyTest.java (limited to 'src/test/java/com/google/devtools/build/lib/skyframe') diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileContentsProxyTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileContentsProxyTest.java new file mode 100644 index 0000000000..0ca3da5f80 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileContentsProxyTest.java @@ -0,0 +1,100 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.testing.EqualsTester; +import com.google.devtools.build.lib.vfs.FileStatus; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link FileContentsProxy}. */ +@RunWith(JUnit4.class) +public class FileContentsProxyTest { + /** A simple implementation of FileStatus for testing. */ + public static final class InjectedStat implements FileStatus { + private final long mtime; + private final long ctime; + private final long size; + private final long nodeId; + + public InjectedStat(long mtime, long ctime, long size, long nodeId) { + this.mtime = mtime; + this.ctime = ctime; + this.size = size; + this.nodeId = nodeId; + } + + @Override + public boolean isFile() { + return true; + } + + @Override + public boolean isSpecialFile() { + return false; + } + + @Override + public boolean isDirectory() { + return false; + } + + @Override + public boolean isSymbolicLink() { + return false; + } + + @Override + public long getSize() { + return size; + } + + @Override + public long getLastModifiedTime() { + return mtime; + } + + @Override + public long getLastChangeTime() { + return ctime; + } + + @Override + public long getNodeId() { + return nodeId; + } + } + + @Test + public void equalsAndHashCode() { + new EqualsTester() + .addEqualityGroup(new FileContentsProxy(1L, 2L), new FileContentsProxy(1L, 2L)) + .addEqualityGroup(new FileContentsProxy(1L, 4L)) + .addEqualityGroup(new FileContentsProxy(3L, 4L)) + .addEqualityGroup(new FileContentsProxy(-1L, -1L)) + .testEquals(); + } + + @Test + public void fromStat() throws Exception { + FileContentsProxy p1 = + FileContentsProxy.create( + new InjectedStat(/*mtime=*/1, /*ctime=*/2, /*size=*/3, /*nodeId=*/4)); + assertThat(p1.getCTime()).isEqualTo(2); + assertThat(p1.getNodeId()).isEqualTo(4); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java index 830b91c786..8e1ff34c53 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java @@ -558,13 +558,15 @@ public class FileFunctionTest { } @Test - public void testSymlinkTargetContentsChangeModTime() throws Exception { + public void testSymlinkTargetContentsChangeCTime() throws Exception { fastDigest = false; Path fooPath = file("foo"); FileSystemUtils.writeContentAsLatin1(fooPath, "foo"); Path p = symlink("symlink", "foo"); FileValue a = valueForPath(p); - fooPath.setLastModifiedTime(88); + manualClock.advanceMillis(1); + fooPath.chmod(0555); + manualClock.advanceMillis(1); FileValue b = valueForPath(p); assertThat(b).isNotEqualTo(a); } 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 c682340282..a3853e6540 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 @@ -42,7 +42,6 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.BuildFailedException; import com.google.devtools.build.lib.actions.Root; -import com.google.devtools.build.lib.actions.cache.InjectedStat; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.actions.util.TestAction; @@ -687,13 +686,11 @@ public class TreeArtifactBuildTest extends TimestampBuilderTestCase { MetadataHandler md = actionExecutionContext.getMetadataHandler(); FileStatus stat = outOneFileOne.getPath().stat(Symlinks.NOFOLLOW); md.injectDigest(outOneFileOne, - new InjectedStat(stat.getLastModifiedTime(), stat.getSize(), stat.getNodeId()), - Hashing.md5().hashString("one", Charset.forName("UTF-8")).asBytes()); + stat, Hashing.md5().hashString("one", Charset.forName("UTF-8")).asBytes()); stat = outOneFileTwo.getPath().stat(Symlinks.NOFOLLOW); md.injectDigest(outOneFileTwo, - new InjectedStat(stat.getLastModifiedTime(), stat.getSize(), stat.getNodeId()), - Hashing.md5().hashString("two", Charset.forName("UTF-8")).asBytes()); + stat, Hashing.md5().hashString("two", Charset.forName("UTF-8")).asBytes()); } catch (Exception e) { throw new RuntimeException(e); } -- cgit v1.2.3