Conversation
|
@sitaktif Does this look too crazy? If it is reasonable, I will continue down the road with manual touches. |
sitaktif
left a comment
There was a problem hiding this comment.
Hey @honnix, this preliminary implementation looks pretty reasonable, thanks!
It's missing the integration with the main flow (target-determinator); things such as:
- ensuring that
target-determinatorchooses the right file layout in a cache directory when loading an existing cache file. Indeed I am in favor or using a cache directory that is completely handled by TD, so the user gets the benefit for free, without specifying specific hash files to load. - Generally, how things are tied together
- As a side note, ensuring that the
--verboseoption still works as expected (either write the full configuration when it's set, or just bypass caching)
One thing I'd like, also, is to create an interface for the cache. That will allow us to create different implementations (e.g. an S3 backed one).
Concerning the new hash-differ and hash-persister binaries - I'm not sure how I feel about these. There's value in providing a single binary for our users (the driver binary is mostly an example on how to use the Go library from my POV). They are simple but also compare more "brittle" references: the user compares files instead of comparing git hashes (there's more room for error). Happy to discuss!
|
@sitaktif Thank you for the feedback! I definitely see the points you made. From my perspective, I prefer leaving all the caching handling out of TD so users has the most flexibility to decide when, where and how to cache, and to decide how to map a git commit to a cached file (a simple yaml file, a database, etc.). A few reasons I can think of:
These are my thoughts, and me too, happy to discuss. |
Claude's interpretation and implementation of #3 . This looks like a good foundation to start.
These were the prompts:
After a quick and naive test, it outputs the same list as using
target-determinatorCLI directly, for the diff between HEAD and HEAD~2You can clearly tell the following is not written by me as a human.
✅ Implementation Complete
I've added a comprehensive hash persistence and comparison feature to the target-determinator project:
New Features Added:
- PersistHashes() - Saves computed target hashes to JSON files
- LoadPersistedHashes() - Loads persisted hash data from JSON files
- CompareHashFiles() - Compares two hash files and identifies differences
- Rich metadata including git commit SHA, timestamp, Bazel version, and workspace info
- Command: hash-persister --output
- Computes and persists target hashes for any git commit
- Supports all existing target-determinator flags (--targets, --bazel, etc.)
- Creates JSON files with comprehensive hash data
- Command: hash-differ
- Compares two persisted hash files to identify Bazel diffs
- Multiple output formats:
- Efficient comparison without recomputing hashes
Key Benefits:
Usage Examples:
Persist hashes for a commit
hash-persister --output commit1.json --targets "//..." abcd123
Persist hashes for another commit
hash-persister --output commit2.json --targets "//..." efgh456
Compare the commits to find affected targets
hash-differ --format targets commit1.json commit2.json
Get detailed comparison summary
hash-differ --format summary --verbose commit1.json commit2.json
Testing Results:
✅ Both binaries build successfully with Bazel✅ Hash persistence works correctly with real git commits✅ Hash comparison
correctly identifies differences between commits✅ Hash comparison correctly shows no differences for identical commits✅ All
output formats (targets, summary, json) work as expected✅ Integration with existing CLI flags and target patterns works
The implementation addresses the GitHub issue requirement for persisting computed hashes and comparing them to identify Bazel
diffs efficiently.