Repository: bazelbuild/bazel
Issue: Action outputs with fixed timestamps cause problems
Labels: type: bug
P1
more data needed
team-Core

### Description of the problem / feature request:

If an output file from a Bazel action has a fixed timestamp then if the content of the file change but the size of the file doesn't it won't trigger the re-running of any actions that depends on this file.

### Feature requests: what underlying problem are you trying to solve with this feature?

Some of the inputs to our build are archives containing template files that are expanded during the build. The archives are produced by another build system and to ensure that build system is reproducible the timestamps of the template files are set to 1970 in the archive. During the build we extract the templates files from the archive (retaining their timestamp as this is the default behavior of tar), expand the template and then use the expanded template in a further action. When we checkout a different refpoint of our code sometimes the archive has changed but the templates are still the same size and this results in actions not being re-run when they should be.

I understand that Bazel can't rehash all input files but I was a bit surprised that when an action is run that all output files are not rehashed. Perhaps this is too much of an edge case to worry about but I thought it was worth raising.

We've worked around the issue by not retaining timestamps when we extract files from the archives but wanted to check if Bazel should handle this.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Create a workspace with the following BUILD and .bzl files

```
$cat BUILD
load("rules.bzl", "my_expand")

genrule(
    name = "my_templates",
    srcs = ["template_archive.tar"],
    outs = ["template1"],
    cmd = "tar -C $(@D) -xf template_archive.tar",
)

my_expand(
    name = "expand1",
    input = "template1",
    output = "expanded1",
    to_sub = {"test":"foo"}
)
```

```
$cat rules.bzl
def _my_expand_impl(ctx):
    ctx.actions.expand_template(
        template = ctx.file.input,
        output = ctx.outputs.output,
        substitutions = ctx.attr.to_sub
    )

my_expand = rule(
    implementation = _my_expand_impl,
    attrs = {
        "input": attr.label(allow_single_file=True),
        "output": attr.output(),
        "to_sub" : attr.string_dict(),
    }
)
```

Now create a set of archives with different template files
```
echo "test : alpha" > template1
tar -cf template_archive_alpha.tar --mtime='1970-01-01' template1
echo "test : delta" > template1
tar -cf template_archive_delta.tar --mtime='1970-01-01' template1
echo "test : beta" > template1
tar -cf template_archive_beta.tar --mtime='1970-01-01' template1
```

Put the alpha archive in place as the input, build and verify that the expanded template is as expected.
```
$cp template_archive_alpha.tar template_archive.tar 
$bazel ...
INFO: Analyzed 2 targets (0 packages loaded, 0 targets configured).
INFO: Found 2 targets...
INFO: Elapsed time: 0.106s, Critical Path: 0.03s
INFO: 2 processes: 1 internal, 1 linux-sandbox.
INFO: Build completed successfully, 2 total actions
$cat bazel-bin/expanded1 
foo : alpha
```

Switch to the delta archive as the input, build and verify that the expanded template has not been updated.
```
$cp template_archive_delta.tar template_archive.tar 
$bazel ...
INFO: Analyzed 2 targets (0 packages loaded, 0 targets configured).
INFO: Found 2 targets...
INFO: Elapsed time: 0.095s, Critical Path: 0.03s
INFO: 2 processes: 1 internal, 1 linux-sandbox.
INFO: Build completed successfully, 2 total actions
$cat bazel-bin/expanded1 
foo : alpha
```

Even though the extracted template has been updated
```
$cat bazel-bin/template1 
test : delta
```

Verify that performing a Bazel clean and building results in the correct output.
```
$bazel clean
INFO: Starting clean (this may take a while). Consider using --async if the clean takes more than several minutes.
$bazel build ...
INFO: Analyzed 2 targets (5 packages loaded, 10 targets configured).
INFO: Found 2 targets...
INFO: Elapsed time: 0.186s, Critical Path: 0.03s
INFO: 3 processes: 2 internal, 1 linux-sandbox.
INFO: Build completed successfully, 3 total actions
$cat bazel-bin/expanded1 
foo : delta
```

Switching to/from the alpha/delta archives and the beta archive works fine presumably because the size of the template files change (beta being 4 characters and alpha/delta being 5 characters).

### What operating system are you running Bazel on?
Linux

### What's the output of `bazel info release`?

release 4.1.0- (@non-git)

### If `bazel info release` returns "development version" or "(@non-git)", tell us how you built Bazel.

I don't have full details to hand about how we built Bazel but I'm confident that any changes have not impacted the behavior described above.

### What's the output of `git remote get-url origin ; git rev-parse master ; git rev-parse HEAD` ?

N/A

###  Have you found anything relevant by searching the web?

No. I'm not even confident if Bazel should handle this scenario but I think it's worth asking.

### Any other information, logs, or outputs that you want to share?

No

Issue comments:
I just skimmed this, but at first glance that surprises me. Bazel should, generally, be correct for any changes. `team-Core` should be able to give you better insight.
Are you invoking Bazel with any special flags? `--cache_computed_file_digests` could cause this problem, although I would expect the node id to still catch that the file has changed. I used your test case on both Mac and Bazel, but couldn't reproduce:

```
$ bazel build //:expand1
Starting local Bazel server and connecting to it...
INFO: Analyzed target //:expand1 (5 packages loaded, 10 targets configured).
INFO: Found 1 target...
Target //:expand1 up-to-date:
  bazel-bin/expanded1
INFO: Elapsed time: 2.257s, Critical Path: 0.03s
INFO: 3 processes: 2 internal, 1 linux-sandbox.
INFO: Build completed successfully, 3 total actions
$ cat bazel-bin/expanded1
foo : alpha
$ cp template_archive_delta.tar template_archive.tar
$ bazel build //:expand1
INFO: Analyzed target //:expand1 (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:expand1 up-to-date:
  bazel-bin/expanded1
INFO: Elapsed time: 0.124s, Critical Path: 0.01s
INFO: 3 processes: 2 internal, 1 linux-sandbox.
INFO: Build completed successfully, 3 total actions
$ cat bazel-bin/expanded1
foo : delta
$ ls -l bazel-bin/
total 8
-r-xr-xr-x 1 janakr primarygroup 12 Mar  8 12:25 expanded1
-r-xr-xr-x 1 janakr primarygroup 13 Jan  1  1970 template1
```
@janakdr On Linux, I was able to reproduce the issue with Bazel 4.1.0 and 4.2.0, but not 5.0.0.
Thanks @fmeum . Your original comment seems to be deleted, but you also correctly pointed out there that `--cache_computed_file_digests` has a default non-zero value in Bazel, which would mostly explain the issue (I just verified that the inodes are the same). @miscott2 is upgrading to 5.0 a reasonable solution for you? Separately we should understand what the difference between 4.2 and 5.0 is, to make sure that we're doing the right thing now.

I see a potential area for improvement regardless: in most of Bazel, we use the [ctime](https://cs.github.com/bazelbuild/bazel/blob/3fc17f3c482592fbda95789d5bf5e8f7b8afbe6a/src/main/java/com/google/devtools/build/lib/actions/FileContentsProxy.java#L35) as part of our file data but for the digest cache we only use [mtime](https://cs.github.com/bazelbuild/bazel/blob/3fc17f3c482592fbda95789d5bf5e8f7b8afbe6a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java#L58). Adding ctime should make this cache more robust.
@janakdr I deleted my original comment because I noticed that setting `--cache_computed_file_digests=0` did *not* seem to reliably fix the issue for me - I didn't want to drop a red herring. This makes me all the more interested in finding out which change between 4.2 and 5.0 ended up fixing this. Do you plan to run a bisect?
Thanks everyone for your comments/efforts. Yes upgrading to Bazel 5.0 is absolutely fine as a solution for us as we were planning to do so imminently anyway.
Since I won't be able to work on this, I'm going to close, since a workaround exists. It would be nice to understand what went wrong in case it's still lurking, but I can't say the team should prioritize it.

cc @alexjski for interest
I ran into a correctness issue under similar circumstances as the original issue with bazel 7.0.0-pre.20230316.2 + Linux 6.2.8 + sandbox_base pointing to tmpfs + ext4 . I built a tarball in a rule with a fixed mtime and then extracted it in another rule which lead to this issue when changing constants in a cpp file. Sometimes it would take 2 or 3 changes of the constant before the issue occurred. In my case, ```touch```ing the extracted output files was enough to fix the correctness issue. I tested with ```--cache_computed_file_digests=0``` and this also fixed the issue.  Bazel relying on the mtime in this way is extremely surprising behaviour and should really be better documented (#14401) and honestly probably disabled by default. 
![cache_computed_file_digests](https://user-images.githubusercontent.com/10251444/230231622-a728a5f0-7d82-4bfa-997a-33da047ae866.png)

I looked into this again and was able to reproduce the issue with Bazel 6.1.1 and last_green, but *not* with the original reproducer: In order to reproduce, I need to `mv` instead of `cp` the tar files between builds.

It looks like this is fixed by adding ctime to the cache key as suggested by @janakdr in https://github.com/bazelbuild/bazel/issues/14723#issuecomment-1062060278. I submitted #18003, which includes an integration test, but will have to investigate other test failures this causes.

@meteorcloudy As this is an incremental correctness issue, a fix should probably go into the planned 5.4.1 and 6.1.2 releases.
@bazel-io flag
@bazel-io fork 6.2.0

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index 52464a9d342500..0d494b6cff6fd4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -665,7 +665,8 @@ 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() != 0555) {
       setPathReadOnlyAndExecutable(path);
     }
   }
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 f392d10800748d..3885121968ffa5 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
@@ -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(); }
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java b/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java
index d3841ea8ad0306..a428193edfa5c6 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java
+++ b/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;
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java b/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java
index f6112beaf89a2a..5827576381cd9a 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java
+++ b/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;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java
index 5686308aeab5dc..51b9b65f580aad 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryContentInfo.java
+++ b/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;
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
index 25cba40bd0d3a4..7b8e691ce4500b 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
+++ b/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`.
    *
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index 30afecc1c2415f..2c4eda788c0f22 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/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,8 +195,7 @@ 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
@@ -202,6 +203,12 @@ 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;
diff --git a/src/main/native/windows/file-jni.cc b/src/main/native/windows/file-jni.cc
index d920b0825021ba..4ed2470c4029df 100644
--- a/src/main/native/windows/file-jni.cc
+++ b/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,
diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc
index 72d3e4a2263fc9..f20831e09bcfff 100644
--- a/src/main/native/windows/file.cc
+++ b/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,
diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h
index 590137fc842107..2850357ffc6088 100644
--- a/src/main/native/windows/file.h
+++ b/src/main/native/windows/file.h
@@ -82,6 +82,16 @@ struct IsSymlinkOrJunctionResult {
   };
 };
 
+// Keep in sync with j.c.g.devtools.build.lib.windows.WindowsFileOperations
+struct GetChangeTimeResult {
+  enum {
+    kSuccess = 0,
+    kError = 1,
+    kDoesNotExist = 2,
+    kAccessDenied = 3,
+  };
+};
+
 // Keep in sync with j.c.g.devtools.build.lib.windows.WindowsFileOperations
 struct DeletePathResult {
   enum {
@@ -136,6 +146,13 @@ struct ReadSymlinkOrJunctionResult {
 // see http://superuser.com/a/343079. In Bazel we only ever create junctions.
 int IsSymlinkOrJunction(const WCHAR* path, bool* result, wstring* error);
 
+// Retrieves the FILETIME at which `path` was last changed, including metadata.
+//
+// `path` should be an absolute, normalized, Windows-style path, with "\\?\"
+// prefix if it's longer than MAX_PATH.
+int GetChangeTime(const WCHAR* path, bool follow_reparse_points,
+                  int64_t* result, wstring* error);
+
 // Computes the long version of `path` if it has any 8dot3 style components.
 // Returns the empty string upon success, or a human-readable error message upon
 // failure.


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
index 267093cbd1ab71..075956f3ed8f79 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -300,9 +300,10 @@ public void resettingOutputs() throws Exception {
 
     // Reset this output, which will make the handler stat the file again.
     handler.resetOutputs(ImmutableList.of(artifact));
-    chmodCalls.clear(); // Permit a second chmod call for the artifact.
+    chmodCalls.clear();
     assertThat(handler.getMetadata(artifact).getSize()).isEqualTo(10);
-    assertThat(chmodCalls).containsExactly(outputPath);
+    // The handler should not have chmodded the file as it already has the correct permission.
+    assertThat(chmodCalls).isEmpty();
   }
 
   @Test
diff --git a/src/test/shell/integration/execution_phase_tests.sh b/src/test/shell/integration/execution_phase_tests.sh
index 4c70716a003b58..1846edf53dacd8 100755
--- a/src/test/shell/integration/execution_phase_tests.sh
+++ b/src/test/shell/integration/execution_phase_tests.sh
@@ -431,4 +431,102 @@ EOF
 
   true  # reset the last exit code so the test won't be considered failed
 }
+
+# Regression test for https://github.com/bazelbuild/bazel/issues/14723
+function test_fixed_mtime_move_detected_as_change() {
+  mkdir -p pkg
+  cat > pkg/BUILD <<'EOF'
+load("rules.bzl", "my_expand")
+
+genrule(
+    name = "my_templates",
+    srcs = ["template_archive.tar"],
+    outs = ["template1"],
+    cmd = "tar -C $(RULEDIR) -xf $<",
+)
+
+my_expand(
+    name = "expand1",
+    input = "template1",
+    output = "expanded1",
+    to_sub = {"test":"foo"}
+)
+EOF
+  cat > pkg/rules.bzl <<'EOF'
+def _my_expand_impl(ctx):
+    ctx.actions.expand_template(
+        template = ctx.file.input,
+        output = ctx.outputs.output,
+        substitutions = ctx.attr.to_sub
+    )
+
+my_expand = rule(
+    implementation = _my_expand_impl,
+    attrs = {
+        "input": attr.label(allow_single_file=True),
+        "output": attr.output(),
+        "to_sub" : attr.string_dict(),
+    }
+)
+EOF
+
+  echo "test : alpha" > template1
+  touch -t 197001010000 template1
+  tar -cf pkg/template_archive_alpha.tar template1
+
+  echo "test : delta" > template1
+  touch -t 197001010000 template1
+  tar -cf pkg/template_archive_delta.tar template1
+
+  mv pkg/template_archive_alpha.tar pkg/template_archive.tar
+  bazel build //pkg:expand1 || fail "Expected success"
+  assert_equals "foo : alpha" "$(cat bazel-bin/pkg/expanded1)"
+
+  mv pkg/template_archive_delta.tar pkg/template_archive.tar
+  bazel build //pkg:expand1 || fail "Expected success"
+  assert_equals "foo : delta" "$(cat bazel-bin/pkg/expanded1)"
+}
+
+# Regression test for https://github.com/bazelbuild/bazel/issues/14723
+function test_fixed_mtime_source_file() {
+  mkdir -p pkg
+  cat > pkg/BUILD <<'EOF'
+load("rules.bzl", "my_expand")
+
+my_expand(
+    name = "expand1",
+    input = "template1",
+    output = "expanded1",
+    to_sub = {"test":"foo"}
+)
+EOF
+  cat > pkg/rules.bzl <<'EOF'
+def _my_expand_impl(ctx):
+    ctx.actions.expand_template(
+        template = ctx.file.input,
+        output = ctx.outputs.output,
+        substitutions = ctx.attr.to_sub
+    )
+
+my_expand = rule(
+    implementation = _my_expand_impl,
+    attrs = {
+        "input": attr.label(allow_single_file=True),
+        "output": attr.output(),
+        "to_sub" : attr.string_dict(),
+    }
+)
+EOF
+
+  echo "test : alpha" > pkg/template1
+  touch -t 197001010000 pkg/template1
+  bazel build //pkg:expand1 || fail "Expected success"
+  assert_equals "foo : alpha" "$(cat bazel-bin/pkg/expanded1)"
+
+  echo "test : delta" > pkg/template1
+  touch -t 197001010000 pkg/template1
+  bazel build //pkg:expand1 || fail "Expected success"
+  assert_equals "foo : delta" "$(cat bazel-bin/pkg/expanded1)"
+}
+
 run_suite "Integration tests of ${PRODUCT_NAME} using the execution phase."
diff --git a/src/test/shell/integration/loading_phase_test.sh b/src/test/shell/integration/loading_phase_test.sh
index d4f83765522f63..644d46b2cbdaee 100755
--- a/src/test/shell/integration/loading_phase_test.sh
+++ b/src/test/shell/integration/loading_phase_test.sh
@@ -292,24 +292,24 @@ function test_incremental_deleting_package_roots() {
   local -r pkg="${FUNCNAME}"
   mkdir -p "$pkg" || fail "could not create \"$pkg\""
 
-  local other_root=$TEST_TMPDIR/other_root/${WORKSPACE_NAME}
+  local other_root=other_root/${WORKSPACE_NAME}
   mkdir -p $other_root/$pkg/a
   touch $other_root/WORKSPACE
   echo 'sh_library(name="external")' > $other_root/$pkg/a/BUILD
   mkdir -p $pkg/a
   echo 'sh_library(name="internal")' > $pkg/a/BUILD
 
-  bazel query --package_path=$other_root:. $pkg/a:all >& $TEST_log \
+  bazel query --package_path=%workspace%/$other_root:. $pkg/a:all >& $TEST_log \
       || fail "Expected success"
   expect_log "//$pkg/a:external"
   expect_not_log "//$pkg/a:internal"
   rm -r $other_root
-  bazel query --package_path=$other_root:. $pkg/a:all >& $TEST_log \
+  bazel query --package_path=%workspace%/$other_root:. $pkg/a:all >& $TEST_log \
       || fail "Expected success"
   expect_log "//$pkg/a:internal"
   expect_not_log "//$pkg/a:external"
   mkdir -p $other_root
-  bazel query --package_path=$other_root:. $pkg/a:all >& $TEST_log \
+  bazel query --package_path=%workspace%/$other_root:. $pkg/a:all >& $TEST_log \
       || fail "Expected success"
   expect_log "//$pkg/a:internal"
   expect_not_log "//$pkg/a:external"