Use millisecond precision when setting the last modified time on Unix.
Currently, we truncate to second precision. Instead, make use of utimensat(2), which can accommodate up to nanosecond precision, is part of POSIX, and is widely available (Linux, Darwin, OpenBSD, FreeBSD and NetBSD).
PiperOrigin-RevId: 678185275
Change-Id: I307dbc75c8f594e6d70b43ee0bde2226b3015611
diff --git a/src/main/java/com/google/devtools/build/lib/unix/NativePosixFiles.java b/src/main/java/com/google/devtools/build/lib/unix/NativePosixFiles.java
index 3761ceb..6f58450 100644
--- a/src/main/java/com/google/devtools/build/lib/unix/NativePosixFiles.java
+++ b/src/main/java/com/google/devtools/build/lib/unix/NativePosixFiles.java
@@ -129,16 +129,17 @@
public static native ErrnoFileStatus errnoLstat(String path);
/**
- * Native wrapper around POSIX utime(2) syscall.
+ * Native wrapper around POSIX utimensat(2) syscall.
*
- * Note: negative file times are interpreted as unsigned time_t.
+ * <p>Note that, even though utimensat(2) supports up to nanosecond precision, this interface only
+ * allows millisecond precision, which is what Bazel uses internally.
*
- * @param path the file whose times to change.
- * @param now if true, ignore actime/modtime parameters and use current time.
- * @param modtime the file modification time in seconds since the UNIX epoch.
- * @throws IOException if the utime() syscall failed.
+ * @param path the file whose modification time should be changed.
+ * @param now if true, ignore {@code epochMilli} and use the current time.
+ * @param epochMilli the file modification time in milliseconds since the UNIX epoch.
+ * @throws IOException if the operation failed.
*/
- public static native void utime(String path, boolean now, int modtime) throws IOException;
+ public static native void utimensat(String path, boolean now, long epochMilli) throws IOException;
/**
* Native wrapper around POSIX mkdir(2) syscall.
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 1345a94..01a423f 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
@@ -455,13 +455,7 @@
public void setLastModifiedTime(PathFragment path, long newTime) throws IOException {
var comp = Blocker.begin();
try {
- if (newTime == Path.NOW_SENTINEL_TIME) {
- NativePosixFiles.utime(path.toString(), true, 0);
- } else {
- // newTime > MAX_INT => -ve unixTime
- int unixTime = (int) (newTime / 1000);
- NativePosixFiles.utime(path.toString(), false, unixTime);
- }
+ NativePosixFiles.utimensat(path.toString(), newTime == Path.NOW_SENTINEL_TIME, newTime);
} finally {
Blocker.end(comp);
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
index 71955e7..cb74d3f 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.vfs;
-import com.google.common.annotations.VisibleForTesting;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.clock.JavaClock;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -63,12 +62,6 @@
this.clock = new JavaClock();
}
- @VisibleForTesting
- JavaIoFileSystem(Clock clock, DigestHashFunction hashFunction) {
- super(hashFunction);
- this.clock = clock;
- }
-
protected File getIoFile(PathFragment path) {
return new File(path.toString());
}
diff --git a/src/main/native/unix_jni.cc b/src/main/native/unix_jni.cc
index acdbef5..d97ea7b 100644
--- a/src/main/native/unix_jni.cc
+++ b/src/main/native/unix_jni.cc
@@ -22,6 +22,7 @@
#include <limits.h>
#include <netdb.h>
#include <netinet/in.h>
+#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <sys/resource.h>
@@ -30,6 +31,7 @@
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/types.h>
+#include <time.h>
#include <unistd.h>
#include <utime.h>
@@ -489,32 +491,25 @@
/*
* Class: com.google.devtools.build.lib.unix.NativePosixFiles
- * Method: utime
- * Signature: (Ljava/lang/String;ZII)V
+ * Method: utimensat
+ * Signature: (Ljava/lang/String;ZJ)V
* Throws: java.io.IOException
*/
extern "C" JNIEXPORT void JNICALL
-Java_com_google_devtools_build_lib_unix_NativePosixFiles_utime(JNIEnv *env,
- jclass clazz,
- jstring path,
- jboolean now,
- jint modtime) {
+Java_com_google_devtools_build_lib_unix_NativePosixFiles_utimensat(
+ JNIEnv *env, jclass clazz, jstring path, jboolean now, jlong millis) {
const char *path_chars = GetStringLatin1Chars(env, path);
-#ifdef __linux
- struct timespec spec[2] = {{0, UTIME_OMIT}, {modtime, now ? UTIME_NOW : 0}};
+ int64_t sec = millis / 1000;
+ int32_t nsec = (millis % 1000) * 1000000;
+ struct timespec spec[2] = {
+ // Do not set atime.
+ {0, UTIME_OMIT},
+ // Set mtime to now if `now` is true, otherwise to the specified time.
+ {sec, now ? UTIME_NOW : nsec},
+ };
if (::utimensat(AT_FDCWD, path_chars, spec, 0) == -1) {
PostException(env, errno, path_chars);
}
-#else
- struct utimbuf buf = { modtime, modtime };
- struct utimbuf *bufptr = now ? nullptr : &buf;
- if (::utime(path_chars, bufptr) == -1) {
- // EACCES ENOENT EMULTIHOP ELOOP EINTR
- // ENOTDIR ENOLINK EPERM EROFS -> IOException
- // EFAULT ENAMETOOLONG -> RuntimeException
- PostException(env, errno, path_chars);
- }
-#endif
ReleaseStringLatin1Chars(path_chars);
}
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
index eb7c202..a9d911b 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
@@ -43,6 +43,7 @@
import java.nio.channels.SeekableByteChannel;
import java.nio.channels.WritableByteChannel;
import java.nio.file.FileAlreadyExistsException;
+import java.time.Instant;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Matcher;
@@ -1168,6 +1169,31 @@
}
// Test the date functions
+
+ @Test
+ public void testSetLastModifiedTime() throws Exception {
+ Path file = absolutize("file");
+ FileSystemUtils.createEmptyFile(file);
+
+ file.setLastModifiedTime(1234567890L);
+ assertThat(file.getLastModifiedTime()).isEqualTo(1234567890L);
+ }
+
+ @Test
+ public void testSetLastModifiedTimeWithSentinel() throws Exception {
+ Path file = absolutize("file");
+ FileSystemUtils.createEmptyFile(file);
+
+ // To avoid sleeping, first set the modification time to the past.
+ long pastTime = Instant.now().minusSeconds(1).toEpochMilli();
+ file.setLastModifiedTime(pastTime);
+
+ // Even if we get the system time before the setLastModifiedTime call, getLastModifiedTime may
+ // return a time which is slightly behind. Simply check that it's greater than the past time.
+ file.setLastModifiedTime(Path.NOW_SENTINEL_TIME);
+ assertThat(file.getLastModifiedTime()).isGreaterThan(pastTime);
+ }
+
@Test
public void testCreateFileChangesTimeOfDirectory() throws Exception {
storeReferenceTime(workingDir.getLastModifiedTime());
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java
index ab5b9d3..b8ca6f6 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/JavaIoFileSystemTest.java
@@ -13,14 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.vfs;
-import static com.google.common.truth.Truth.assertThat;
-
import com.google.common.collect.Iterables;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
-import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.TestUtils;
import java.io.IOException;
import java.nio.file.Files;
@@ -42,12 +39,9 @@
*/
public class JavaIoFileSystemTest extends SymlinkAwareFileSystemTest {
- private ManualClock clock;
-
@Override
public FileSystem getFreshFileSystem(DigestHashFunction digestHashFunction) {
- clock = new ManualClock();
- return new JavaIoFileSystem(clock, digestHashFunction);
+ return new JavaIoFileSystem(digestHashFunction);
}
// Tests are inherited from the FileSystemTest
@@ -58,21 +52,6 @@
@Test
public void testBadPermissionsThrowsExceptionOnStatIfFound() {}
- @Test
- public void testSetLastModifiedTime() throws Exception {
- Path file = xEmptyDirectory.getChild("new-file");
- FileSystemUtils.createEmptyFile(file);
-
- file.setLastModifiedTime(1000L);
- assertThat(file.getLastModifiedTime()).isEqualTo(1000L);
- file.setLastModifiedTime(0L);
- assertThat(file.getLastModifiedTime()).isEqualTo(0L);
-
- clock.advanceMillis(42000L);
- file.setLastModifiedTime(-1L);
- assertThat(file.getLastModifiedTime()).isEqualTo(42000L);
- }
-
@Override
protected boolean isHardLinked(Path a, Path b) throws IOException {
return Files.readAttributes(
