Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.4.0] Use millisecond precision when setting the last modified time on Unix. #23740

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
33 changes: 14 additions & 19 deletions src/main/native/unix_jni.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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(
Expand Down
Loading