[7.4.0] Use millisecond precision when setting the last modified time…

… on Unix. (bazelbuild#23740)

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

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.
+   * <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.

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

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());
   }

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 @@ 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);
 }
 

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());

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(