From 39dbc9805f9d60acc9a98685826d2a5bb2dd0908 Mon Sep 17 00:00:00 2001 From: Ola Rozenfeld Date: Thu, 17 Nov 2016 20:14:56 +0000 Subject: Adding an option to set the digest function that everything uses. Minor refactoring: enabling potential fast digest computation of more than one digest function type. Usage: bazel --host_jvm_args="-Dbazel.DigestFunction=SHA1" build ... Ugliness: using a system property (a static non-final variable), because the better way to do it (a flag) would result in a much, much larger refactoring. More ugliness: I have updated the minimal amount of tests. A lot of tests are still relying on the default value of MD5. Ideally, they need to be updated as well. -- MOS_MIGRATED_REVID=139490836 --- .../lib/skyframe/ArtifactFunctionTestCase.java | 10 ++---- .../build/lib/skyframe/FileFunctionTest.java | 41 +++++++++------------- 2 files changed, 19 insertions(+), 32 deletions(-) (limited to 'src/test/java/com/google/devtools/build/lib/skyframe') diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java index e3c6a97a43..0ccaeaf559 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -158,13 +159,8 @@ abstract class ArtifactFunctionTestCase { /** InMemoryFileSystem that can pretend to do a fast digest. */ protected class CustomInMemoryFs extends InMemoryFileSystem { @Override - protected String getFastDigestFunctionType(Path path) { - return fastDigest ? "MD5" : null; - } - - @Override - protected byte[] getFastDigest(Path path) throws IOException { - return fastDigest ? getMD5Digest(path) : null; + protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException { + return fastDigest ? getDigest(path, hashFunction) : null; } } } 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 91dbc6c4ae..5eb13b326d 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 @@ -52,6 +52,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.FileStatus; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -95,13 +96,13 @@ public class FileFunctionTest { private Path pkgRoot; private Path outputBase; private PathPackageLocator pkgLocator; - private boolean fastMd5; + private boolean fastDigest; private ManualClock manualClock; private RecordingDifferencer differencer; @Before public final void createFsAndRoot() throws Exception { - fastMd5 = true; + fastDigest = true; manualClock = new ManualClock(); createFsAndRoot(new CustomInMemoryFs(manualClock)); } @@ -433,12 +434,7 @@ public class FileFunctionTest { createFsAndRoot( new CustomInMemoryFs(manualClock) { @Override - protected String getFastDigestFunctionType(Path path) { - return "magic"; - } - - @Override - protected byte[] getFastDigest(Path path) throws IOException { + protected byte[] getFastDigest(Path path, HashFunction hf) throws IOException { return digest; } }); @@ -479,7 +475,7 @@ public class FileFunctionTest { createFsAndRoot( new CustomInMemoryFs(manualClock) { @Override - protected byte[] getFastDigest(Path path) { + protected byte[] getFastDigest(Path path, HashFunction hf) { return path.getBaseName().equals("unreadable") ? expectedDigest : null; } }); @@ -494,7 +490,7 @@ public class FileFunctionTest { @Test public void testFileModificationModTime() throws Exception { - fastMd5 = false; + fastDigest = false; Path p = file("file"); FileValue a = valueForPath(p); p.setLastModifiedTime(42); @@ -504,7 +500,7 @@ public class FileFunctionTest { @Test public void testFileModificationDigest() throws Exception { - fastMd5 = true; + fastDigest = true; Path p = file("file"); FileValue a = valueForPath(p); FileSystemUtils.writeContentAsLatin1(p, "goop"); @@ -516,9 +512,9 @@ public class FileFunctionTest { public void testModTimeVsDigest() throws Exception { Path p = file("somefile", "fizzley"); - fastMd5 = true; + fastDigest = true; FileValue aMd5 = valueForPath(p); - fastMd5 = false; + fastDigest = false; FileValue aModTime = valueForPath(p); assertThat(aModTime).isNotEqualTo(aMd5); new EqualsTester().addEqualityGroup(aMd5).addEqualityGroup(aModTime).testEquals(); @@ -560,7 +556,7 @@ public class FileFunctionTest { @Test public void testSymlinkTargetContentsChangeModTime() throws Exception { - fastMd5 = false; + fastDigest = false; Path fooPath = file("foo"); FileSystemUtils.writeContentAsLatin1(fooPath, "foo"); Path p = symlink("symlink", "foo"); @@ -572,7 +568,7 @@ public class FileFunctionTest { @Test public void testSymlinkTargetContentsChangeDigest() throws Exception { - fastMd5 = true; + fastDigest = true; Path fooPath = file("foo"); FileSystemUtils.writeContentAsLatin1(fooPath, "foo"); Path p = symlink("symlink", "foo"); @@ -830,14 +826,14 @@ public class FileFunctionTest { assertArrayEquals(digest, value.getDigest()); // Digest is cached -- no filesystem access. assertEquals(expectedCalls, digestCalls.get()); - fastMd5 = false; + fastDigest = false; digestCalls.set(0); value = valueForPath(file); // No new digest calls. assertEquals(0, digestCalls.get()); assertNull(value.getDigest()); assertEquals(0, digestCalls.get()); - fastMd5 = true; + fastDigest = true; Path dir = directory("directory"); try { assertNull(valueForPath(dir).getDigest()); @@ -932,7 +928,7 @@ public class FileFunctionTest { fs.stubStat(path("a"), inconsistentParentFileStatus); // Disable fast-path md5 so that we don't try try to md5 the "a" (since it actually physically // is a directory). - fastMd5 = false; + fastDigest = false; SequentialBuildDriver driver = makeDriver(); SkyKey skyKey = skyKey("a/b"); EvaluationResult result = @@ -1624,21 +1620,16 @@ public class FileFunctionTest { super(manualClock); } - @Override - protected String getFastDigestFunctionType(Path path) { - return fastMd5 ? "MD5" : null; - } - public void stubFastDigestError(Path path, IOException error) { stubbedFastDigestErrors.put(path, error); } @Override - protected byte[] getFastDigest(Path path) throws IOException { + protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException { if (stubbedFastDigestErrors.containsKey(path)) { throw stubbedFastDigestErrors.get(path); } - return fastMd5 ? getMD5Digest(path) : null; + return fastDigest ? getDigest(path) : null; } public void stubStat(Path path, @Nullable FileStatus stubbedResult) { -- cgit v1.2.3