[5.4.1] Use ctime in file digest cache key

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

@@ -74,6 +74,7 @@ java_library(
  ":common",
  "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
  "//src/main/java/com/google/devtools/build/lib/events",
+ "//src/main/java/com/google/devtools/build/lib/util:os",
  "//third_party:gson",
  "//third_party:guava",
  ],

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java

@@ -22,6 +22,8 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.gson.FieldNamingPolicy;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
@@ -94,15 +96,18 @@ public Optional<byte[]> getModuleFile(ModuleKey key, ExtendedEventHandler eventH
  /** Represents fields available in {@code bazel_registry.json} for the registry. */
  private static class BazelRegistryJson {
  String[] mirrors;
+ String moduleBasePath;
  }
 
  /** Represents fields available in {@code source.json} for each version of a module. */
  private static class SourceJson {
+ String type = "archive";
  URL url;
  String integrity;
  String stripPrefix;
  Map<String, String> patches;
  int patchStrip;
+ String path;
  }
 
  /**
@@ -142,6 +147,60 @@ public RepoSpec getRepoSpec(ModuleKey key, String repoName, ExtendedEventHandler
  throw new FileNotFoundException(
  String.format("Module %s's source information not found in registry %s", key, getUrl()));
  }
+
+ String type = sourceJson.get().type;
+ switch (type) {
+ case "archive":
+ return createArchiveRepoSpec(sourceJson, bazelRegistryJson, key, repoName);
+ case "local_path":
+ return createLocalPathRepoSpec(sourceJson, bazelRegistryJson, key, repoName);
+ default:
+ throw new IOException(String.format("Invalid source type for module %s", key));
+ }
+ }
+
+ private RepoSpec createLocalPathRepoSpec(
+ Optional<SourceJson> sourceJson,
+ Optional<BazelRegistryJson> bazelRegistryJson,
+ ModuleKey key,
+ RepositoryName repoName)
+ throws IOException {
+ String path = sourceJson.get().path;
+ if (!PathFragment.isAbsolute(path)) {
+ String moduleBase = bazelRegistryJson.get().moduleBasePath;
+ path = moduleBase + "/" + path;
+ if (!PathFragment.isAbsolute(moduleBase)) {
+ if (uri.getScheme().equals("file")) {
+ if (uri.getPath().isEmpty() || !uri.getPath().startsWith("/")) {
+ throw new IOException(
+ String.format(
+ "Provided non absolute local registry path for module %s: %s",
+ key, uri.getPath()));
+ }
+ // Unix: file:///tmp --> /tmp
+ // Windows: file:///C:/tmp --> C:/tmp
+ path = uri.getPath().substring(OS.getCurrent() == OS.WINDOWS ? 1 : 0) + "/" + path;
+ } else {
+ throw new IOException(String.format("Provided non local registry for module %s", key));
+ }
+ }
+ }
+
+ return RepoSpec.builder()
+ .setRuleClassName("local_repository")
+ .setAttributes(
+ AttributeValues.create(
+ ImmutableMap.of(
+ "name", repoName.getName(), "path", PathFragment.create(path).toString())))
+ .build();
+ }
+
+ private RepoSpec createArchiveRepoSpec(
+ Optional<SourceJson> sourceJson,
+ Optional<BazelRegistryJson> bazelRegistryJson,
+ ModuleKey key,
+ RepositoryName repoName)
+ throws IOException {
  URL sourceUrl = sourceJson.get().url;
  if (sourceUrl == null) {
  throw new IOException(String.format("Missing source URL for module %s", key));

src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java

@@ -665,7 +665,10 @@ private static FileArtifactValue fileArtifactValueFromStat(
  }
 
  private static void setPathReadOnlyAndExecutableIfFile(Path path) throws IOException {
- if (path.isFile(Symlinks.NOFOLLOW)) {
+ FileStatus stat = path.statIfFound(Symlinks.NOFOLLOW);
+ if (stat != null
+ && stat.isFile()
+ && stat.getPermissions() != outputPermissions.getPermissionsMode()) {
  setPathReadOnlyAndExecutable(path);
  }
  }

src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java

@@ -98,7 +98,10 @@ public long getNodeId() {
  return status.getInodeNumber();
  }
 
- int getPermissions() { return status.getPermissions(); }
+ @Override
+ public int getPermissions() {
+ return status.getPermissions();
+ }
 
  @Override
  public String toString() { return status.toString(); }

src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java

@@ -54,6 +54,9 @@ private static class CacheKey {
  /** File system identifier of the file (typically the inode number). */
  private final long nodeId;
 
+ /** Last change time of the file. */
+ private final long changeTime;
+
  /** Last modification time of the file. */
  private final long modifiedTime;
 
@@ -70,6 +73,7 @@ private static class CacheKey {
  public CacheKey(Path path, FileStatus status) throws IOException {
  this.path = path.asFragment();
  this.nodeId = status.getNodeId();
+ this.changeTime = status.getLastChangeTime();
  this.modifiedTime = status.getLastModifiedTime();
  this.size = status.getSize();
  }
@@ -84,6 +88,7 @@ public boolean equals(Object object) {
  CacheKey key = (CacheKey) object;
  return path.equals(key.path)
  && nodeId == key.nodeId
+ && changeTime == key.changeTime
  && modifiedTime == key.modifiedTime
  && size == key.size;
  }
@@ -94,6 +99,7 @@ public int hashCode() {
  int result = 17;
  result = 31 * result + path.hashCode();
  result = 31 * result + Longs.hashCode(nodeId);
+ result = 31 * result + Longs.hashCode(changeTime);
  result = 31 * result + Longs.hashCode(modifiedTime);
  result = 31 * result + Longs.hashCode(size);
  return result;

src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java

@@ -85,4 +85,16 @@ public interface FileStatus {
  * ought to cause the node ID of b to change, but appending / modifying b should not.
  */
  long getNodeId() throws IOException;
+
+ /**
+ * Returns the file's permissions in POSIX format (e.g. 0755) if possible without performing
+ * additional IO, otherwise (or if unsupported by the file system) returns -1.
+ *
+ * <p>If accurate group and other permissions aren't available, the returned value should attempt
+ * to mimic a umask of 022 (i.e. read and execute permissions extend to group and other, write
+ * does not).
+ */
+ default int getPermissions() {
+ return -1;
+ }
 }

src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java

@@ -120,6 +120,22 @@ public long getNodeId() {
  return System.identityHashCode(this);
  }
 
+ @Override
+ public final int getPermissions() {
+ int permissions = 0;
+ // Emulate the default umask of 022.
+ if (isReadable) {
+ permissions |= 0444;
+ }
+ if (isWritable) {
+ permissions |= 0200;
+ }
+ if (isExecutable) {
+ permissions |= 0111;
+ }
+ return permissions;
+ }
+
  @Override
  public final InMemoryContentInfo inode() {
  return this;

src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java

@@ -15,7 +15,9 @@
 package com.google.devtools.build.lib.windows;
 
 import com.google.devtools.build.lib.jni.JniLoader;
+import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.nio.file.AccessDeniedException;
 
 /** File operations on Windows. */
 public class WindowsFileOperations {
@@ -82,6 +84,12 @@ public Status getStatus() {
  // IS_SYMLINK_OR_JUNCTION_ERROR = 1;
  private static final int IS_SYMLINK_OR_JUNCTION_DOES_NOT_EXIST = 2;
 
+ // Keep GET_CHANGE_TIME_* values in sync with src/main/native/windows/file.cc.
+ private static final int GET_CHANGE_TIME_SUCCESS = 0;
+ // private static final int GET_CHANGE_TIME_ERROR = 1;
+ private static final int GET_CHANGE_TIME_DOES_NOT_EXIST = 2;
+ private static final int GET_CHANGE_TIME_ACCESS_DENIED = 3;
+
  // Keep CREATE_JUNCTION_* values in sync with src/main/native/windows/file.h.
  private static final int CREATE_JUNCTION_SUCCESS = 0;
  // CREATE_JUNCTION_ERROR = 1;
@@ -114,6 +122,9 @@ public Status getStatus() {
  private static native int nativeIsSymlinkOrJunction(
  String path, boolean[] result, String[] error);
 
+ private static native int nativeGetChangeTime(
+ String path, boolean followReparsePoints, long[] result, String[] error);
+
  private static native boolean nativeGetLongPath(String path, String[] result, String[] error);
 
  private static native int nativeCreateJunction(String name, String target, String[] error);
@@ -143,6 +154,25 @@ public static boolean isSymlinkOrJunction(String path) throws IOException {
  throw new IOException(String.format("Cannot tell if '%s' is link: %s", path, error[0]));
  }
 
+ /** Returns the time at which the file was last changed, including metadata changes. */
+ public static long getLastChangeTime(String path, boolean followReparsePoints)
+ throws IOException {
+ long[] result = new long[] {0};
+ String[] error = new String[] {null};
+ switch (nativeGetChangeTime(asLongPath(path), followReparsePoints, result, error)) {
+ case GET_CHANGE_TIME_SUCCESS:
+ return result[0];
+ case GET_CHANGE_TIME_DOES_NOT_EXIST:
+ throw new FileNotFoundException(path);
+ case GET_CHANGE_TIME_ACCESS_DENIED:
+ throw new AccessDeniedException(path);
+ default:
+ // This is GET_CHANGE_TIME_ERROR (1). The JNI code puts a custom message in 'error[0]'.
+ break;
+ }
+ throw new IOException(String.format("Cannot get last change time of '%s': %s", path, error[0]));
+ }
+
  /**
  * Returns the long path associated with the input `path`.
  *

src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java

@@ -156,6 +156,8 @@ protected FileStatus stat(PathFragment path, boolean followSymlinks) throws IOEx
  }
 
  final boolean isSymbolicLink = !followSymlinks && fileIsSymbolicLink(file);
+ final long lastChangeTime =
+ WindowsFileOperations.getLastChangeTime(getNioPath(path).toString(), followSymlinks);
  FileStatus status =
  new FileStatus() {
  @Override
@@ -193,15 +195,20 @@ public long getLastModifiedTime() throws IOException {
 
  @Override
  public long getLastChangeTime() {
- // This is the best we can do with Java NIO...
- return attributes.lastModifiedTime().toMillis();
+ return lastChangeTime;
  }
 
  @Override
  public long getNodeId() {
  // TODO(bazel-team): Consider making use of attributes.fileKey().
  return -1;
  }
+
+ @Override
+ public int getPermissions() {
+ // Files on Windows are implicitly readable and executable.
+ return 0555 | (attributes.isReadOnly() ? 0 : 0200);
+ }
  };
 
  return status;

src/main/native/windows/file-jni.cc

@@ -62,6 +62,29 @@ Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeIsSymlink
  return static_cast<jint>(result);
 }
 
+extern "C" JNIEXPORT jint JNICALL
+Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeGetChangeTime(
+ JNIEnv* env, jclass clazz, jstring path, jboolean follow_reparse_points,
+ jlongArray result_holder, jobjectArray error_msg_holder) {
+ std::wstring wpath(bazel::windows::GetJavaWstring(env, path));
+ std::wstring error;
+ jlong ctime = 0;
+ int result =
+ bazel::windows::GetChangeTime(wpath.c_str(), follow_reparse_points,
+ reinterpret_cast<int64_t*>(&ctime), &error);
+ if (result == bazel::windows::GetChangeTimeResult::kSuccess) {
+ env->SetLongArrayRegion(result_holder, 0, 1, &ctime);
+ } else {
+ if (!error.empty() && CanReportError(env, error_msg_holder)) {
+ ReportLastError(
+ bazel::windows::MakeErrorMessage(
+ WSTR(__FILE__), __LINE__, L"nativeGetChangeTime", wpath, error),
+ env, error_msg_holder);
+ }
+ }
+ return static_cast<jint>(result);
+}
+
 extern "C" JNIEXPORT jboolean JNICALL
 Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeGetLongPath(
  JNIEnv* env, jclass clazz, jstring path, jobjectArray result_holder,

src/main/native/windows/file.cc

@@ -21,6 +21,7 @@
 #include <WinIoCtl.h>
 #include <stdint.h> // uint8_t
 #include <versionhelpers.h>
+#include <winbase.h>
 #include <windows.h>
 
 #include <memory>
@@ -119,6 +120,54 @@ int IsSymlinkOrJunction(const WCHAR* path, bool* result, wstring* error) {
  }
 }
 
+int GetChangeTime(const WCHAR* path, bool follow_reparse_points,
+ int64_t* result, wstring* error) {
+ if (!IsAbsoluteNormalizedWindowsPath(path)) {
+ if (error) {
+ *error = MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetChangeTime",
+ path, L"expected an absolute Windows path");
+ }
+ return GetChangeTimeResult::kError;
+ }
+
+ AutoHandle handle;
+ DWORD flags = FILE_FLAG_BACKUP_SEMANTICS;
+ if (!follow_reparse_points) {
+ flags |= FILE_FLAG_OPEN_REPARSE_POINT;
+ }
+ handle = CreateFileW(path, 0,
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+ nullptr, OPEN_EXISTING, flags, nullptr);
+
+ if (!handle.IsValid()) {
+ DWORD err = GetLastError();
+ if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
+ return GetChangeTimeResult::kDoesNotExist;
+ } else if (err == ERROR_ACCESS_DENIED) {
+ return GetChangeTimeResult::kAccessDenied;
+ }
+ if (error) {
+ *error =
+ MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateFileW", path, err);
+ }
+ return GetChangeTimeResult::kError;
+ }
+
+ FILE_BASIC_INFO info;
+ if (!GetFileInformationByHandleEx(handle, FileBasicInfo, (LPVOID)&info,
+ sizeof(FILE_BASIC_INFO))) {
+ DWORD err = GetLastError();
+ if (error) {
+ *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+ L"GetFileInformationByHandleEx", path, err);
+ }
+ return GetChangeTimeResult::kError;
+ }
+
+ *result = info.ChangeTime.QuadPart;
+ return GetChangeTimeResult::kSuccess;
+}
+
 wstring GetLongPath(const WCHAR* path, unique_ptr<WCHAR[]>* result) {
  if (!IsAbsoluteNormalizedWindowsPath(path)) {
  return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetLongPath", path,