diff --git a/src/main/native/unix_jni.cc b/src/main/native/unix_jni.cc index 7615c67da4fdac..98686af3493554 100644 --- a/src/main/native/unix_jni.cc +++ b/src/main/native/unix_jni.cc @@ -847,7 +847,7 @@ Java_com_google_devtools_build_lib_unix_NativePosixFiles_mkfifo(JNIEnv *env, // Linux extended file attributes typedef ssize_t getxattr_func(const char *path, const char *name, - void *value, size_t size); + void *value, size_t size, bool *attr_not_found); static jbyteArray getxattr_common(JNIEnv *env, jstring path, @@ -859,9 +859,11 @@ static jbyteArray getxattr_common(JNIEnv *env, // TODO(bazel-team): on ERANGE, try again with larger buffer. jbyte value[4096]; jbyteArray result = NULL; - ssize_t size = getxattr(path_chars, name_chars, value, arraysize(value)); + bool attr_not_found = false; + ssize_t size = getxattr(path_chars, name_chars, value, arraysize(value), + &attr_not_found); if (size == -1) { - if (errno != ENODATA) { + if (!attr_not_found) { ::PostFileException(env, errno, path_chars); } } else { diff --git a/src/main/native/unix_jni.h b/src/main/native/unix_jni.h index f4a160016147c5..19d5b3f1f83ee2 100644 --- a/src/main/native/unix_jni.h +++ b/src/main/native/unix_jni.h @@ -83,13 +83,17 @@ int StatSeconds(const portable_stat_struct &statbuf, StatTimes t); // Returns nanoseconds from a stat buffer. int StatNanoSeconds(const portable_stat_struct &statbuf, StatTimes t); -// Runs getxattr(2), if available. If not, sets errno to ENOSYS. +// Runs getxattr(2). If the attribute is not found, returns -1 and sets +// attr_not_found to true. For all other errors, returns -1, sets attr_not_found +// to false and leaves errno set to the error code returned by the system. ssize_t portable_getxattr(const char *path, const char *name, void *value, - size_t size); + size_t size, bool *attr_not_found); -// Run lgetxattr(2), if available. If not, sets errno to ENOSYS. +// Runs lgetxattr(2). If the attribute is not found, returns -1 and sets +// attr_not_found to true. For all other errors, returns -1, sets attr_not_found +// to false and leaves errno set to the error code returned by the system. ssize_t portable_lgetxattr(const char *path, const char *name, void *value, - size_t size); + size_t size, bool *attr_not_found); // Run sysctlbyname(3), only available on darwin int portable_sysctlbyname(const char *name_chars, long *mibp, size_t *sizep); diff --git a/src/main/native/unix_jni_darwin.cc b/src/main/native/unix_jni_darwin.cc index 02aaa98a7f341c..06fbf443c9c7ce 100644 --- a/src/main/native/unix_jni_darwin.cc +++ b/src/main/native/unix_jni_darwin.cc @@ -104,13 +104,17 @@ int StatNanoSeconds(const portable_stat_struct &statbuf, StatTimes t) { } ssize_t portable_getxattr(const char *path, const char *name, void *value, - size_t size) { - return getxattr(path, name, value, size, 0, 0); + size_t size, bool *attr_not_found) { + ssize_t result = getxattr(path, name, value, size, 0, 0); + *attr_not_found = (errno == ENOATTR); + return result; } ssize_t portable_lgetxattr(const char *path, const char *name, void *value, - size_t size) { - return getxattr(path, name, value, size, 0, XATTR_NOFOLLOW); + size_t size, bool *attr_not_found) { + ssize_t result = getxattr(path, name, value, size, 0, XATTR_NOFOLLOW); + *attr_not_found = (errno == ENOATTR); + return result; } int portable_sysctlbyname(const char *name_chars, long *mibp, size_t *sizep) { diff --git a/src/main/native/unix_jni_linux.cc b/src/main/native/unix_jni_linux.cc index 772b4007162f43..f66de383f1c52d 100644 --- a/src/main/native/unix_jni_linux.cc +++ b/src/main/native/unix_jni_linux.cc @@ -74,13 +74,17 @@ int StatNanoSeconds(const portable_stat_struct &statbuf, StatTimes t) { } ssize_t portable_getxattr(const char *path, const char *name, void *value, - size_t size) { - return ::getxattr(path, name, value, size); + size_t size, bool *attr_not_found) { + ssize_t result = ::getxattr(path, name, value, size); + *attr_not_found = (errno == ENODATA); + return result; } ssize_t portable_lgetxattr(const char *path, const char *name, void *value, - size_t size) { - return ::lgetxattr(path, name, value, size); + size_t size, bool *attr_not_found) { + ssize_t result = ::lgetxattr(path, name, value, size); + *attr_not_found = (errno == ENODATA); + return result; } int portable_sysctlbyname(const char *name_chars, long *mibp, size_t *sizep) { 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 ea5039d89f362e..ddad8aa1594199 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 @@ -14,11 +14,15 @@ package com.google.devtools.build.lib.unix; import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; import com.google.common.collect.ImmutableMap; import com.google.common.hash.HashCode; import com.google.devtools.build.lib.testutil.TestUtils; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileAccessException; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -26,6 +30,7 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.nio.file.Files; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -111,4 +116,43 @@ public void throwsFilePermissionException() throws Exception { assertThat(e).hasMessage(foo + " (Read-only file system)"); } } + + /** Skips the test if the file system does not support extended attributes. */ + private static void assumeXattrsSupported() throws Exception { + // The standard file systems on macOS support extended attributes by default, so we can assume + // that the test will work on that platform. For other systems, we currently don't have a + // mechanism to validate this so the tests are skipped unconditionally. + assumeTrue(OS.getCurrent() == OS.DARWIN); + } + + @Test + public void testGetxattr_AttributeFound() throws Exception { + assumeXattrsSupported(); + + String myfile = Files.createTempFile("getxattrtest", null).toString(); + Runtime.getRuntime().exec("xattr -w foo bar " + myfile).waitFor(); + + assertThat(new String(NativePosixFiles.getxattr(myfile, "foo"), UTF_8)).isEqualTo("bar"); + assertThat(new String(NativePosixFiles.lgetxattr(myfile, "foo"), UTF_8)).isEqualTo("bar"); + } + + @Test + public void testGetxattr_AttributeNotFoundReturnsNull() throws Exception { + assumeXattrsSupported(); + + String myfile = Files.createTempFile("getxattrtest", null).toString(); + + assertThat(NativePosixFiles.getxattr(myfile, "foo")).isNull(); + assertThat(NativePosixFiles.lgetxattr(myfile, "foo")).isNull(); + } + + @Test + public void testGetxattr_FileNotFound() throws Exception { + String nonexistentFile = workingDir.getChild("nonexistent").toString(); + + assertThrows( + FileNotFoundException.class, () -> NativePosixFiles.getxattr(nonexistentFile, "foo")); + assertThrows( + FileNotFoundException.class, () -> NativePosixFiles.lgetxattr(nonexistentFile, "foo")); + } }