Make getxattr not throw an exception when attribute does not exist on Mac.
This matches the current behavior on Linux. When an extended attribute is not present on a file, getxattr on Linux returns ENODATA whereas getxattr on Mac returns ENOATTR. Previously, we were special casing ENODATA to not throw an exception but not ENOATTR. Now we treat them the same.
RELNOTES: None
PiperOrigin-RevId: 185157964
diff --git a/src/main/native/unix_jni.cc b/src/main/native/unix_jni.cc
index 7615c67..98686af 100644
--- a/src/main/native/unix_jni.cc
+++ b/src/main/native/unix_jni.cc
@@ -847,7 +847,7 @@
// 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 @@
// 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 f4a1600..19d5b3f 100644
--- a/src/main/native/unix_jni.h
+++ b/src/main/native/unix_jni.h
@@ -83,13 +83,17 @@
// 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 02aaa98..06fbf443 100644
--- a/src/main/native/unix_jni_darwin.cc
+++ b/src/main/native/unix_jni_darwin.cc
@@ -104,13 +104,17 @@
}
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 772b400..f66de38 100644
--- a/src/main/native/unix_jni_linux.cc
+++ b/src/main/native/unix_jni_linux.cc
@@ -74,13 +74,17 @@
}
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 ea5039d..ddad8aa 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 @@
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"));
+ }
}
