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 {