From 7b525cd9ea8c456bdeb9f8a7e8cd063d70b5dfb2 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Fri, 22 May 2026 09:23:52 -0400 Subject: [PATCH] Hash only the owner execute bit on source files Match what git's index tracks (100644 vs 100755). Group and others execute bits don't affect the build and can flip between checkouts under different umasks, causing spurious hash differences. This mirrors the target-determinator fix in commit 5bfae36 ("only take user execute bits into account"). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/bazel_diff/hash/SourceFileHasher.kt | 34 ++++++----------- .../bazel_diff/hash/SourceFileHasherTest.kt | 37 +++++++++++++++++-- 2 files changed, 45 insertions(+), 26 deletions(-) diff --git a/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt b/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt index 384f4d5..f8fe311 100644 --- a/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt +++ b/cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt @@ -92,17 +92,7 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher { safePutBytes(contentHash.toByteArray()) putBytes(byteArrayOf(0x01)) val absoluteFilePath = workingDirectory.resolve(filenamePath) - val isExecutable = - try { - Files.getPosixFilePermissions(absoluteFilePath).let { perms -> - perms.contains(PosixFilePermission.OWNER_EXECUTE) || - perms.contains(PosixFilePermission.GROUP_EXECUTE) || - perms.contains(PosixFilePermission.OTHERS_EXECUTE) - } - } catch (_: Exception) { - absoluteFilePath.toFile().canExecute() - } - putBytes(byteArrayOf(if (isExecutable) 0x01 else 0x00)) + putBytes(byteArrayOf(if (isOwnerExecutable(absoluteFilePath)) 0x01 else 0x00)) } else { val absoluteFilePath = workingDirectory.resolve(filenamePath) val file = absoluteFilePath.toFile() @@ -115,17 +105,7 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher { } // Mark that file exists putBytes(byteArrayOf(0x01)) - val isExecutable = - try { - Files.getPosixFilePermissions(absoluteFilePath).let { perms -> - perms.contains(PosixFilePermission.OWNER_EXECUTE) || - perms.contains(PosixFilePermission.GROUP_EXECUTE) || - perms.contains(PosixFilePermission.OTHERS_EXECUTE) - } - } catch (_: Exception) { - file.canExecute() - } - putBytes(byteArrayOf(if (isExecutable) 0x01 else 0x00)) + putBytes(byteArrayOf(if (isOwnerExecutable(absoluteFilePath)) 0x01 else 0x00)) } } else { logger.w { "File $absoluteFilePath not found" } @@ -155,6 +135,16 @@ class SourceFileHasherImpl : KoinComponent, SourceFileHasher { return digest(sourceFileTarget, modifiedFilepaths) } + // Git's index only tracks the owner execute bit (100644 vs 100755); group/others bits don't + // affect the build and differ between checkouts under different umasks. Mirror that behavior so + // hashes don't churn when only group/others bits flip. + private fun isOwnerExecutable(absoluteFilePath: Path): Boolean = + try { + Files.getPosixFilePermissions(absoluteFilePath).contains(PosixFilePermission.OWNER_EXECUTE) + } catch (_: Exception) { + absoluteFilePath.toFile().canExecute() + } + private fun isMainRepo(name: String): Int { if (name.startsWith("//")) { return 2 diff --git a/cli/src/test/kotlin/com/bazel_diff/hash/SourceFileHasherTest.kt b/cli/src/test/kotlin/com/bazel_diff/hash/SourceFileHasherTest.kt index a8332c9..08890a4 100644 --- a/cli/src/test/kotlin/com/bazel_diff/hash/SourceFileHasherTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/hash/SourceFileHasherTest.kt @@ -240,18 +240,47 @@ internal class SourceFileHasherTest : KoinTest { val permsWithoutExec = Files.getPosixFilePermissions(filePath) - PosixFilePermission.OWNER_EXECUTE Files.setPosixFilePermissions(filePath, permsWithoutExec) - val nonExecutableHash = - hasher.digest(BazelSourceFileTarget(target, seed)).toHexString() + val nonExecutableHash = hasher.digest(BazelSourceFileTarget(target, seed)).toHexString() val permsWithExec = permsWithoutExec + PosixFilePermission.OWNER_EXECUTE Files.setPosixFilePermissions(filePath, permsWithExec) - val executableHash = - hasher.digest(BazelSourceFileTarget(target, seed)).toHexString() + val executableHash = hasher.digest(BazelSourceFileTarget(target, seed)).toHexString() // Hashes must differ when executable bit changes — currently fails (issue #325) assertThat(nonExecutableHash).isNotEqualTo(executableHash) } + // Only the OWNER execute bit matters — git's index only tracks 100644 vs 100755, so toggling the + // group/others execute bits should not produce a different hash (avoids churn from umask + // differences between checkouts). + @Test + fun testHashIgnoresGroupAndOthersExecuteBits() = + runBlocking { + val testDir = Files.createTempDirectory("group_other_exec_test") + val filePath = testDir.resolve("path/to/file.txt") + Files.createDirectories(filePath.parent) + Files.createFile(filePath) + filePath.toFile().writeText("contents") + + val target = "//path/to:file.txt" + val hasher = SourceFileHasherImpl(testDir, null, externalRepoResolver) + + val ownerOnly = setOf(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE) + Files.setPosixFilePermissions(filePath, ownerOnly) + val baselineHash = hasher.digest(BazelSourceFileTarget(target, seed)).toHexString() + + val withGroupExec = ownerOnly + PosixFilePermission.GROUP_EXECUTE + Files.setPosixFilePermissions(filePath, withGroupExec) + val groupExecHash = hasher.digest(BazelSourceFileTarget(target, seed)).toHexString() + + val withOthersExec = ownerOnly + PosixFilePermission.OTHERS_EXECUTE + Files.setPosixFilePermissions(filePath, withOthersExec) + val othersExecHash = hasher.digest(BazelSourceFileTarget(target, seed)).toHexString() + + assertThat(groupExecHash).isEqualTo(baselineHash) + assertThat(othersExecHash).isEqualTo(baselineHash) + } + @Test fun testHashEmptyFileVsDeletedFile() = runBlocking {