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 d29e63739331c1..3b28f056402b55 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 @@ -128,16 +128,17 @@ public static native void symlink(String oldpath, String newpath) 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. + *

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 c16c8164f39f01..e71f7e9f37a1f4 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 @@ -407,13 +407,7 @@ protected long getLastModifiedTime(PathFragment path, boolean followSymlinks) th @Override public void setLastModifiedTime(PathFragment path, long newTime) throws IOException { - 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); } @Override 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 71955e744c0342..cb74d3f22d2682 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 @@ public JavaIoFileSystem(DigestHashFunction hashFunction) { 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 acdbef5c83d597..d97ea7b5bd244d 100644 --- a/src/main/native/unix_jni.cc +++ b/src/main/native/unix_jni.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -30,6 +31,7 @@ #include #include #include +#include #include #include @@ -489,32 +491,25 @@ Java_com_google_devtools_build_lib_unix_NativePosixFiles_errnoLstat(JNIEnv *env, /* * 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 382502351f2934..58e05a2b35571b 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.WritableByteChannel; import java.nio.charset.StandardCharsets; 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; @@ -1177,6 +1178,31 @@ public void testDeleteTreesBelowFailsGracefullyIfContentsGoMissing() throws Exce } // 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 ab5b9d308f48a7..b8ca6f69d2d050 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 @@ public FileSystem getFreshFileSystem(DigestHashFunction digestHashFunction) { @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(