Only hash executable bit

pkg/hash_cache.go

@@ -614,13 +614,16 @@ func (hc *fileHashCache) Hash(path string) ([]byte, error) {
 		defer file.Close()
 		hasher := sha256.New()
 
-		// Hash the file mode.
-		// This is used to detect change such as file exec bit changing.
+		// Hash the file's u+x bit.
+		// Git doesn't track any of the other file permissions.
+		// If you chmod a file in your working tree in any other bit, and then run in worktree mode
+		// (because you have local untracked changes), the other permission bits may not match,
+		// and we want to ignore this.
 		info, err := file.Stat()
 		if err != nil {
 			return nil, err
 		}
-		if _, err := fmt.Fprintf(hasher, info.Mode().String()); err != nil {
+		if _, err := fmt.Fprintf(hasher, (info.Mode() & 0100).String()); err != nil {
 			return nil, err
 		}
 

tests/integration/java/com/github/bazel_contrib/target_determinator/integration/TargetDeterminatorIntegrationTest.java

@@ -2,8 +2,11 @@
 
 import com.github.bazel_contrib.target_determinator.label.Label;
 import org.hamcrest.CoreMatchers;
+import org.junit.Test;
 
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.attribute.PosixFilePermissions;
 import java.util.Set;
 
 import static junit.framework.TestCase.fail;
@@ -115,4 +118,26 @@ public void ignoredPlatformSpecificDepChanged() throws Exception {
     allowOverBuilds("Platform-specific narrowing is disabled due to https://github.com/bazelbuild/bazel/issues/17916");
     super.ignoredPlatformSpecificDepChanged();
   }
+
+  // We have previously seen a bug here when a worktree needed to be created incorrect permissions in the working tree would differ from the worktree (which clones things with the "correct" permissions).
+  // It turns out git doesn't actually track permissions other than u+x, so we shouldn't be diffing more than that.
+  @Test
+  public void testWrongGroupWritablePermissionsWhenUsingWorktree() throws Exception {
+    // Create an unignored file to force use of a worktree.
+    Path temporaryFileToForceWorktree = testDir.resolve("unadded-but-not-ignored-file-to-force-worktree-use");
+    Files.createFile(temporaryFileToForceWorktree);
+    try {
+      gitCheckout(Commits.ONE_SH_TEST);
+
+      Files.setPosixFilePermissions(testDir.resolve("sh").resolve("sh_test.sh"), PosixFilePermissions.fromString("rwxrwxr-x"));
+
+      Set<Label> targets = getTargets(Commits.ONE_SH_TEST, Commits.ONE_SH_TEST);
+      Util.assertTargetsMatch(
+              targets, Set.of(), Set.of("//sh:sh_test"), isAllowOverBuilds());
+
+    } finally {
+      Files.setPosixFilePermissions(testDir.resolve("sh").resolve("sh_test.sh"), PosixFilePermissions.fromString("rwxr-xr-x"));
+      Files.deleteIfExists(temporaryFileToForceWorktree);
+    }
+  }
 }

tests/integration/java/com/github/bazel_contrib/target_determinator/integration/Tests.java

@@ -53,7 +53,7 @@ abstract Set<Label> getTargets(Path workspace, String commitBefore, String commi
 
   @Rule public TestName name = new TestName();
 
-  private Set<Label> getTargets(String commitBefore, String commitAfter)
+  protected Set<Label> getTargets(String commitBefore, String commitAfter)
       throws TargetComputationErrorException {
     return getTargets(testDir, commitBefore, commitAfter);
   }
@@ -62,6 +62,10 @@ protected void allowOverBuilds(String reason) {
     this.allowOverBuilds = true;
   }
 
+  protected boolean isAllowOverBuilds() {
+    return allowOverBuilds;
+  }
+
   protected boolean supportsIgnoredUnaddedFiles() {
     return false;
   }
@@ -591,7 +595,7 @@ private void gitCheckoutBranch(final String branch) throws Exception {
     TestdataRepo.gitCheckoutBranch(testDir, branch);
   }
 
-  private void gitCheckout(final String commit) throws Exception {
+  protected void gitCheckout(final String commit) throws Exception {
     TestdataRepo.gitCheckout(testDir, commit);
   }