Hash only the owner execute bit on source files#362
Merged
Conversation
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SourceFileHasherto check onlyPosixFilePermission.OWNER_EXECUTEinstead of OR'ing owner/group/others. Pulled into a privateisOwnerExecutablehelper to avoid duplicating the try/catch at both call sites.testHashIgnoresGroupAndOthersExecuteBitsthat hashes the same file with moderw-------,rw---x---, andrw------xand asserts all three hashes are identical.testHashExecutableBitChange(issue Missing bazel-diff support for change executable bit of files #325) still passes since it only togglesOWNER_EXECUTE.Why
Cross-referencing how a sibling tool handles this: bazel-contrib/target-determinator shipped commit
5bfae36("fix(hash_cache): only take user execute bits into account") for the same reason:Git's index only records
100644vs100755— the owner execute bit. Group/others execute bits aren't represented and can flip between checkouts under different umasks. Folding them into the hash adds noise without buying any signal that the build cares about.The current OR-of-three logic was introduced in #331 (fix for #325). #325's example use case (toggling the owner execute bit on a script under a
filegroup) is preserved by this change.Test plan
bazel test //cli:SourceFileHasherTestpasses locally (testHashExecutableBitChangeand the newtestHashIgnoresGroupAndOthersExecuteBitsboth green).bazel test //cli:BuildGraphHasherTest //cli:TargetHashTestpasses locally — no broader regressions.🤖 Generated with Claude Code