From 3fa149ab9cbcf5246d01530977df4a4e81e3ff62 Mon Sep 17 00:00:00 2001 From: Ed Schouten Date: Thu, 18 Jun 2020 18:55:05 +0200 Subject: [PATCH] UnixFileSystem: read cached hashes from extended attributes There are certain workloads where Bazel's running time gets dominated by checksum computation. Examples include: - People adding local_repository()s to their project that point to networked file shares. - The use of repositories that contain very large input files. When using remote execution, we need to compute digests to be able to place such files in input roots. In many cases, a centralized CAS will already contain these files. It would be nice if Bazel could efficiently check for existence of such objects without needing to scan the file locally. This change extends UnixFileSystem to call getxattr() on an attribute prior to falling back to reading file contents. The name of the extended attribute that is used is configurable through a command line flag. Using extended attributes to store this information also seems to be a fairly common approach. Apparently it is also used within Google itself: https://groups.google.com/g/bazel-discuss/c/6VmjSOLySnY/m/v2dpwt8jBgAJ So far no code has been added to let Bazel write these attributes to disk. The main goal so far is to speed up access to read-only corpora, where the maintainers have spent the effort adding these attributes. --- src/main/cpp/blaze.cc | 3 + src/main/cpp/startup_options.cc | 5 + src/main/cpp/startup_options.h | 2 + .../lib/runtime/BazelFileSystemModule.java | 3 +- .../build/lib/runtime/BlazeRuntime.java | 2 +- .../runtime/BlazeServerStartupOptions.java | 13 +++ .../build/lib/unix/UnixFileSystem.java | 16 ++- .../google/devtools/build/lib/vfs/Path.java | 4 +- .../java/com/google/devtools/build/lib/BUILD | 3 + .../repository/TestArchiveDescriptor.java | 2 +- .../buildeventstream/LastBuildEventTest.java | 2 +- .../buildtool/InconsistentFilesystemTest.java | 2 +- .../build/lib/buildtool/IoHookTestCase.java | 2 +- .../lib/buildtool/ProgressReportingTest.java | 2 +- .../lib/exec/local/LocalSpawnRunnerTest.java | 4 +- .../shell/CommandUsingLinuxSandboxTest.java | 2 +- .../shell/CommandUsingProcessWrapperTest.java | 2 +- .../build/lib/unix/NativePosixFilesTest.java | 2 +- .../unix/UnixDigestHashAttributeNameTest.java | 62 ++++++++++++ .../build/lib/unix/UnixFileSystemTest.java | 2 +- .../build/lib/unix/UnixPathEqualityTest.java | 4 +- .../build/lib/vfs/util/FileSystems.java | 4 +- src/test/shell/bazel/BUILD | 8 ++ src/test/shell/bazel/remote/BUILD | 13 +++ .../remote_execution_with_xattr_test.sh | 99 +++++++++++++++++++ .../unix_digest_hash_attribute_name_test.sh | 72 ++++++++++++++ 26 files changed, 315 insertions(+), 20 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java create mode 100755 src/test/shell/bazel/remote/remote_execution_with_xattr_test.sh create mode 100755 src/test/shell/bazel/unix_digest_hash_attribute_name_test.sh diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 19a399520e9243..9275bfe29ea4f2 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -485,6 +485,9 @@ static vector GetServerExeArgs(const blaze_util::Path &jvm_path, // being "null" to set the programmatic default in the server. result.push_back("--digest_function=" + startup_options.digest_function); } + if (!startup_options.unix_digest_hash_attribute_name.empty()) { + result.push_back("--unix_digest_hash_attribute_name=" + startup_options.unix_digest_hash_attribute_name); + } if (startup_options.idle_server_tasks) { result.push_back("--idle_server_tasks"); } else { diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index f9b2fda40180ea..af9cf2da4cf8a3 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -150,6 +150,7 @@ StartupOptions::StartupOptions(const string &product_name, RegisterUnaryStartupFlag("command_port"); RegisterUnaryStartupFlag("connect_timeout_secs"); RegisterUnaryStartupFlag("digest_function"); + RegisterUnaryStartupFlag("unix_digest_hash_attribute_name"); RegisterUnaryStartupFlag("server_javabase"); RegisterUnaryStartupFlag("host_jvm_args"); RegisterUnaryStartupFlag("host_jvm_profile"); @@ -359,6 +360,10 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( NULL) { digest_function = value; option_sources["digest_function"] = rcfile; + } else if ((value = GetUnaryOption(arg, next_arg, "--unix_digest_hash_attribute_name")) != + NULL) { + unix_digest_hash_attribute_name = value; + option_sources["unix_digest_hash_attribute_name"] = rcfile; } else if ((value = GetUnaryOption(arg, next_arg, "--command_port")) != NULL) { if (!blaze_util::safe_strto32(value, &command_port) || diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h index cc9ca667811b72..9532df5804228e 100644 --- a/src/main/cpp/startup_options.h +++ b/src/main/cpp/startup_options.h @@ -254,6 +254,8 @@ class StartupOptions { // The hash function to use when computing file digests. std::string digest_function; + std::string unix_digest_hash_attribute_name; + bool idle_server_tasks; // The startup options as received from the user and rc files, tagged with diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java index 5c4eaff522f146..65e2912aa49556 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java @@ -90,6 +90,7 @@ public ModuleFileSystem getFileSystem( throws DefaultHashFunctionNotSetException { BlazeServerStartupOptions options = startupOptions.getOptions(BlazeServerStartupOptions.class); boolean enableSymLinks = options != null && options.enableWindowsSymlinks; + String unixDigestHashAttributeName = options != null ? options.unixDigestHashAttributeName : ""; if ("0".equals(System.getProperty("io.bazel.EnableJni"))) { // Ignore UnixFileSystem, to be used for bootstrapping. return ModuleFileSystem.create( @@ -101,6 +102,6 @@ public ModuleFileSystem getFileSystem( return ModuleFileSystem.create( OS.getCurrent() == OS.WINDOWS ? new WindowsFileSystem(enableSymLinks) - : new UnixFileSystem()); + : new UnixFileSystem(unixDigestHashAttributeName)); } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index 3ffeb66c46ce5b..88d29a6a8a2c23 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -1130,7 +1130,7 @@ private static FileSystem defaultFileSystemImplementation( // The JNI-based UnixFileSystem is faster, but on Windows it is not available. return OS.getCurrent() == OS.WINDOWS ? new WindowsFileSystem(startupOptions.enableWindowsSymlinks) - : new UnixFileSystem(); + : new UnixFileSystem(startupOptions.unixDigestHashAttributeName); } private static SubprocessFactory subprocessFactoryImplementation() { diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java index cf0b6a2a0dd3e3..4b455740cae28b 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java @@ -482,4 +482,17 @@ public String getTypeDescription() { + "Requires Windows developer mode to be enabled and Windows 10 version 1703 or " + "greater.") public boolean enableWindowsSymlinks; + + @Option( + name = "unix_digest_hash_attribute_name", + defaultValue = "", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.LOSES_INCREMENTAL_STATE}, + help = + "The name of an extended attribute that can be placed on files to store a precomputed " + + "copy of the file's hash, corresponding with --digest_function. This option " + + "can be used to reduce disk I/O and CPU load caused by hash computation. This " + + "extended attribute is checked on all source files and output files, meaning " + + "that it causes a significant number of invocations of the getxattr() system call.") + public String unixDigestHashAttributeName; } diff --git a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java index 497025c605bc52..5e84de360b75ea 100644 --- a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java @@ -46,10 +46,15 @@ @ThreadSafe public class UnixFileSystem extends AbstractFileSystemWithCustomStat { - public UnixFileSystem() throws DefaultHashFunctionNotSetException {} + private final String hashAttributeName; - public UnixFileSystem(DigestHashFunction hashFunction) { + public UnixFileSystem(String hashAttributeName) throws DefaultHashFunctionNotSetException { + this.hashAttributeName = hashAttributeName; + } + + public UnixFileSystem(DigestHashFunction hashFunction, String hashAttributeName) { super(hashFunction); + this.hashAttributeName = hashAttributeName; } /** @@ -407,6 +412,13 @@ public byte[] getxattr(Path path, String name, boolean followSymlinks) throws IO } } + @Override + protected byte[] getFastDigest(Path path) throws IOException { + // Attempt to obtain the digest from an extended attribute attached to the file. This prevents + // the checksum from being recomputed unnecessarily. + return hashAttributeName.isEmpty() ? null : getxattr(path, hashAttributeName, true); + } + @Override protected byte[] getDigest(Path path) throws IOException { String name = path.toString(); diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java index 58bb000e5d4692..1b12caea7ea46c 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java @@ -766,7 +766,9 @@ public byte[] getFastDigest() throws IOException { } /** - * Returns the digest of the file denoted by the current path, following symbolic links. + * Returns the digest of the file denoted by the current path, following symbolic links. This + * function should not be called directly. Use + * {@link com.google.devtools.build.lib.actions.cache.DigestUtils} instead. * * @return a new byte array containing the file's digest * @throws IOException if the digest could not be computed for any reason diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 6669ff327f9f61..b0288e0bdbc3f6 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -197,12 +197,15 @@ java_test( test_class = "com.google.devtools.build.lib.AllTests", deps = [ ":AllTests", + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/unix", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/util:string", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/test/java/com/google/devtools/build/lib/vfs:testutil", "//src/test/java/com/google/devtools/build/lib/events:testutil", "//src/test/java/com/google/devtools/build/lib/testutil", "//src/test/java/com/google/devtools/build/lib/testutil:JunitUtils", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/TestArchiveDescriptor.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/TestArchiveDescriptor.java index 01278f8428a10c..d6af2530cc740f 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/TestArchiveDescriptor.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/TestArchiveDescriptor.java @@ -69,7 +69,7 @@ DecompressorDescriptor.Builder createDescriptorBuilder() throws IOException { FileSystem testFS = OS.getCurrent() == OS.WINDOWS ? new JavaIoFileSystem(DigestHashFunction.getDefaultUnchecked()) - : new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); + : new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); // do not rely on TestConstants.JAVATESTS_ROOT end with slash, but ensure separators // are not duplicated diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/LastBuildEventTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/LastBuildEventTest.java index 2062c571a0e6e2..4badd6c71bc3ac 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventstream/LastBuildEventTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/LastBuildEventTest.java @@ -33,7 +33,7 @@ public class LastBuildEventTest { @Test public void testForwardsReferencedLocalFilesCall() { - FileSystem fs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); + FileSystem fs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); LocalFile localFile = new LocalFile(fs.getPath("/some/file"), LocalFileType.FAILED_TEST_OUTPUT); LastBuildEvent event = new LastBuildEvent(new BuildEvent() { @Override diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/InconsistentFilesystemTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/InconsistentFilesystemTest.java index ed6674573189c7..902a878c4782cf 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/InconsistentFilesystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/InconsistentFilesystemTest.java @@ -38,7 +38,7 @@ protected boolean realFileSystem() { @Override protected FileSystem createFileSystem() { - return new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()) { + return new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), "") { boolean threwException = false; @Override diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/IoHookTestCase.java b/src/test/java/com/google/devtools/build/lib/buildtool/IoHookTestCase.java index 31d65ab462c3d4..8e7dccd27ff5c7 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/IoHookTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/IoHookTestCase.java @@ -63,7 +63,7 @@ protected void setListener(FileListener listener) { @Override protected FileSystem createFileSystem() { setListener(DUMMY_LISTENER); - return new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()) { + return new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), "") { @Override protected void chmod(Path path, int chmod) throws IOException { listener.get().handle(PathOp.CHMOD, path); diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/ProgressReportingTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/ProgressReportingTest.java index 7262caff20ba78..33b9931fa2740a 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/ProgressReportingTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/ProgressReportingTest.java @@ -64,7 +64,7 @@ protected boolean realFileSystem() { @Override protected FileSystem createFileSystem() { - return new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()) { + return new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), "") { private void recordAccess(PathOp op, Path path) { if (receiver != null) { diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java index a1ea5799ad4902..d0ba16995b8c7f 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java @@ -882,7 +882,7 @@ public void hasExecutionStatistics_whenOptionIsEnabled() throws Exception { // TODO(b/62588075) Currently no process-wrapper or execution statistics support in Windows. assumeTrue(OS.getCurrent() != OS.WINDOWS); - FileSystem fs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); + FileSystem fs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); LocalExecutionOptions options = Options.getDefaults(LocalExecutionOptions.class); options.collectLocalExecutionStatistics = true; @@ -956,7 +956,7 @@ public void hasNoExecutionStatistics_whenOptionIsDisabled() throws Exception { // TODO(b/62588075) Currently no process-wrapper or execution statistics support in Windows. assumeTrue(OS.getCurrent() != OS.WINDOWS); - FileSystem fs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); + FileSystem fs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); LocalExecutionOptions options = Options.getDefaults(LocalExecutionOptions.class); options.collectLocalExecutionStatistics = false; diff --git a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java index 2c8d2e464abad3..0ea509930c25f7 100644 --- a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java +++ b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingLinuxSandboxTest.java @@ -43,7 +43,7 @@ public final class CommandUsingLinuxSandboxTest { @Before public final void createFileSystem() throws Exception { - testFS = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); + testFS = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); runfilesDir = testFS.getPath(BlazeTestUtils.runfilesDir()); } diff --git a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java index 31c44ee910e1ce..7929a621d5d4f7 100644 --- a/src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java +++ b/src/test/java/com/google/devtools/build/lib/shell/CommandUsingProcessWrapperTest.java @@ -40,7 +40,7 @@ public final class CommandUsingProcessWrapperTest { @Before public final void createFileSystem() throws Exception { - testFS = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); + testFS = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); } private ProcessWrapper getProcessWrapper() { diff --git a/src/test/java/com/google/devtools/build/lib/unix/NativePosixFilesTest.java b/src/test/java/com/google/devtools/build/lib/unix/NativePosixFilesTest.java index 7fc0c6ae173250..fc1e7755d7a6c1 100644 --- a/src/test/java/com/google/devtools/build/lib/unix/NativePosixFilesTest.java +++ b/src/test/java/com/google/devtools/build/lib/unix/NativePosixFilesTest.java @@ -42,7 +42,7 @@ public class NativePosixFilesTest { @Before public final void createFileSystem() throws Exception { - testFS = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); + testFS = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); workingDir = testFS.getPath(new File(TestUtils.tmpDir()).getCanonicalPath()); testFile = workingDir.getRelative("test"); } diff --git a/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java b/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java new file mode 100644 index 00000000000000..e526de19274b8f --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/unix/UnixDigestHashAttributeNameTest.java @@ -0,0 +1,62 @@ +// Copyright 2014 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.unix; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.lib.actions.cache.DigestUtils; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystemTest; +import com.google.devtools.build.lib.vfs.Path; +import java.io.IOException; +import org.junit.Test; + +/** Test for {@link com.google.devtools.build.lib.unix.UnixFileSystem#getFastDigest()}. */ +public class UnixDigestHashAttributeNameTest extends FileSystemTest { + private static final byte FAKE_DIGEST[] = { + 0x18, 0x5f, 0x3d, 0x33, 0x22, 0x71, 0x7e, 0x25, + 0x55, 0x61, 0x26, 0x0c, 0x03, 0x6b, 0x2e, 0x26, + 0x43, 0x06, 0x7c, 0x30, 0x4e, 0x3a, 0x51, 0x20, + 0x07, 0x71, 0x76, 0x48, 0x26, 0x38, 0x19, 0x69, + }; + + @Override + protected FileSystem getFreshFileSystem(DigestHashFunction digestHashFunction) { + return new FakeAttributeFileSystem(digestHashFunction); + } + + @Test + public void testFoo() throws Exception { + // Instead of actually trying to access this file, a call to getxattr() should be made. We + // intercept this call and return a fake extended attribute value, thereby causing the checksum + // computation to be skipped entirely. + assertThat(DigestUtils.getDigestWithManualFallback(absolutize("myfile"), 123)) + .isEqualTo(FAKE_DIGEST); + } + + private class FakeAttributeFileSystem extends UnixFileSystem { + public FakeAttributeFileSystem(DigestHashFunction hashFunction) { + super(hashFunction, "user.checksum.sha256"); + } + + @Override + public byte[] getxattr(Path path, String name, boolean followSymlinks) throws IOException { + assertThat(path).isEqualTo(absolutize("myfile")); + assertThat(name).isEqualTo("user.checksum.sha256"); + assertThat(followSymlinks).isEqualTo(true); + return FAKE_DIGEST; + } + } +} diff --git a/src/test/java/com/google/devtools/build/lib/unix/UnixFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/unix/UnixFileSystemTest.java index 6ee304f1baa503..084daa0bc7f839 100644 --- a/src/test/java/com/google/devtools/build/lib/unix/UnixFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/unix/UnixFileSystemTest.java @@ -30,7 +30,7 @@ public class UnixFileSystemTest extends SymlinkAwareFileSystemTest { @Override protected FileSystem getFreshFileSystem(DigestHashFunction digestHashFunction) { - return new UnixFileSystem(digestHashFunction); + return new UnixFileSystem(digestHashFunction, ""); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/unix/UnixPathEqualityTest.java b/src/test/java/com/google/devtools/build/lib/unix/UnixPathEqualityTest.java index 7a7690e6fba47f..55a1d07a2c4ffa 100644 --- a/src/test/java/com/google/devtools/build/lib/unix/UnixPathEqualityTest.java +++ b/src/test/java/com/google/devtools/build/lib/unix/UnixPathEqualityTest.java @@ -37,8 +37,8 @@ public class UnixPathEqualityTest { @Before public final void initializeFileSystem() throws Exception { - unixFs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); - otherUnixFs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked()); + unixFs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); + otherUnixFs = new UnixFileSystem(DigestHashFunction.getDefaultUnchecked(), ""); assertThat(unixFs != otherUnixFs).isTrue(); } diff --git a/src/test/java/com/google/devtools/build/lib/vfs/util/FileSystems.java b/src/test/java/com/google/devtools/build/lib/vfs/util/FileSystems.java index c4778ebfddf9ee..1d81b4fec71b06 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/util/FileSystems.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/util/FileSystems.java @@ -34,8 +34,8 @@ public static FileSystem getNativeFileSystem() { try { return Class.forName(TestConstants.TEST_REAL_UNIX_FILE_SYSTEM) .asSubclass(FileSystem.class) - .getDeclaredConstructor(DigestHashFunction.class) - .newInstance(DigestHashFunction.getDefaultUnchecked()); + .getDeclaredConstructor(DigestHashFunction.class, String.class) + .newInstance(DigestHashFunction.getDefaultUnchecked(), ""); } catch (Exception e) { throw new IllegalStateException(e); } diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 6fa630566cda67..0ea97cf973a2d3 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -1319,3 +1319,11 @@ sh_test( data = [":test-deps"], deps = ["@bazel_tools//tools/bash/runfiles"], ) + +sh_test( + name = "unix_digest_hash_attribute_name_test", + srcs = ["unix_digest_hash_attribute_name_test.sh"], + data = [":test-deps"], + deps = ["@bazel_tools//tools/bash/runfiles"], + tags = ["no_windows"], +) diff --git a/src/test/shell/bazel/remote/BUILD b/src/test/shell/bazel/remote/BUILD index 9158c5d8a5944c..b4d07d39cdf79d 100644 --- a/src/test/shell/bazel/remote/BUILD +++ b/src/test/shell/bazel/remote/BUILD @@ -88,3 +88,16 @@ sh_test( "@bazel_tools//tools/bash/runfiles", ], ) + +sh_test( + name = "remote_execution_with_xattr_test", + size = "large", + timeout = "eternal", + srcs = ["remote_execution_with_xattr_test.sh"], + data = [ + ":remote_utils", + "//src/test/shell/bazel:test-deps", + "//src/tools/remote:worker", + "@bazel_tools//tools/bash/runfiles", + ], +) diff --git a/src/test/shell/bazel/remote/remote_execution_with_xattr_test.sh b/src/test/shell/bazel/remote/remote_execution_with_xattr_test.sh new file mode 100755 index 00000000000000..b15d96688ffd3d --- /dev/null +++ b/src/test/shell/bazel/remote/remote_execution_with_xattr_test.sh @@ -0,0 +1,99 @@ +#!/bin/bash +# +# Copyright 2020 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. +# +# Tests remote execution and caching. + +set -euo pipefail + +# --- begin runfiles.bash initialization --- +if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + if [[ -f "$0.runfiles_manifest" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" + elif [[ -f "$0.runfiles/MANIFEST" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + fi +fi +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" +elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ + "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" +else + echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash" + exit 1 +fi +# --- end runfiles.bash initialization --- + +source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } +source "$(rlocation "io_bazel/src/test/shell/bazel/remote/remote_utils.sh")" \ + || { echo "remote_utils.sh not found!" >&2; exit 1; } + +function set_up() { + start_worker +} + +function tear_down() { + bazel clean >& $TEST_log + stop_worker +} + +function test_remote_grpc_cache_with_xattr() { + # Create a simple build action that depends on two input files. + echo hello > file1 + echo world > file2 + cat > BUILD << 'EOF' +genrule( + name = "nothing", + srcs = ["file1", "file2"], + outs = ["nothing.txt"], + cmd = "echo foo > $@", +) +EOF + + # Place an extended attribute on one of the input files that contains + # the file's hash. This will allow Bazel to compute the file's digest + # without reading the actual file contents. + build_file_hash="$(openssl sha256 file1 | cut -d ' ' -f 2)" + if type xattr 2> /dev/null; then + xattr -wx user.checksum.sha256 "${build_file_hash}" file1 + elif type setfattr 2> /dev/null; then + setfattr -n user.checksum.sha256 -v "0x${build_file_hash}" file1 + else + # Skip this test, as this platform doesn't provide any known utility + # for setting extended attributes. + return 0 + fi + + # Run the build action with remote caching. We should not see 'VFS md5' + # profiling events for file1, as getxattr() should be called instead. + # We should see an entry for file2, though. + bazel \ + --unix_digest_hash_attribute_name=user.checksum.sha256 \ + build \ + --remote_cache=grpc://localhost:${worker_port} \ + --profile=profile_log \ + --record_full_profiler_data \ + //:nothing || fail "Build failed" + grep -q "VFS md5.*file1" profile_log && \ + fail "Bazel should not have computed a digest for file1" + grep -q "VFS md5.*file2" profile_log || \ + fail "Bazel should have computed a digest for file2" +} + +run_suite "Remote execution with extended attributes enabled" diff --git a/src/test/shell/bazel/unix_digest_hash_attribute_name_test.sh b/src/test/shell/bazel/unix_digest_hash_attribute_name_test.sh new file mode 100755 index 00000000000000..750307ee7c1521 --- /dev/null +++ b/src/test/shell/bazel/unix_digest_hash_attribute_name_test.sh @@ -0,0 +1,72 @@ +#!/bin/bash +# +# Copyright 2020 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. + +# --- begin runfiles.bash initialization --- +# Copy-pasted from Bazel's Bash runfiles library (tools/bash/runfiles/runfiles.bash). +set -euo pipefail +if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + if [[ -f "$0.runfiles_manifest" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" + elif [[ -f "$0.runfiles/MANIFEST" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + fi +fi +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" +elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ + "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" +else + echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash" + exit 1 +fi +# --- end runfiles.bash initialization --- + +source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +function test_xattr_operations_in_profile_log { + touch WORKSPACE + cat > BUILD << 'EOF' +genrule( + name = "foo", + outs = ["foo.out"], + cmd = "touch $@", +) +EOF + # Place an extended attribute with the checksum on the BUILD file. + # macOS ships with the xattr command line tool, while Linux generally + # ships with setfattr. Both have different styles of invocation. + build_file_hash="$(openssl sha256 BUILD | cut -d ' ' -f 2)" + if type xattr 2> /dev/null; then + xattr -wx user.checksum.sha256 "${build_file_hash}" BUILD + elif type setfattr 2> /dev/null; then + setfattr -n user.checksum.sha256 -v "0x${build_file_hash}" BUILD + fi + + bazel \ + --unix_digest_hash_attribute_name=user.checksum.sha256 \ + build \ + --profile=profile_log \ + --record_full_profiler_data \ + //:foo || fail "Build failed" + grep -q "VFS xattr.*BUILD" profile_log || \ + fail "Bazel did not perform getxattr() calls" +} + +run_suite "Integration tests for --unix_digest_hash_attribute_name"