Implement target distance metrics #230
Conversation
The new fields in TargetHash and TargetDigest are to track the direct srcs/attrs hash separately from the overall hash. Currently, the directHash is not used at all.
Add e2e test for dump distances TODO: It would be nice if there was a better e2e test here -- Ask Maxwell how he generates the integration workspace data.
|
|
cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt
Outdated
Show resolved
Hide resolved
cli/src/test/resources/fixture/impacted_targets_distances-1-2.txt
Outdated
Show resolved
Hide resolved
alex-torok
changed the title
alex-torok
changed the title
| Take a look at the following bazelcon talks to learn more about `bazel-diff`: | ||
|
|
||
| * [BazelCon 2023: Improving CI efficiency with Bazel querying and bazel-diff](https://www.youtube.com/watch?v=QYAbmE_1fSo) | ||
| * BazelCon 2024: Not Going the Distance: Filtering Tests by Build Graph Distance: Coming Soon |
cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt
Outdated
Show resolved
Hide resolved
cli/src/test/resources/fixture/impacted_targets_distances-1-2.txt
Outdated
Show resolved
Hide resolved
1. Output detailed hash data as json 2. Use a different serialization format for targethashes to avoid ambiguity
alex-torok
force-pushed
the
add-direct-hash-to-rule-hasher
branch
from
f121539
to
02efb3e
Compare
|
@tinder-maxwellelliott - I added a new E2E test that better showcases target / package distances with a new test workspace. I don't think there is anything else I need to do here. |
alex-torok
force-pushed
the
add-direct-hash-to-rule-hasher
branch
from
8adcef4
to
e8eef2a
Compare
alex-torok
force-pushed
the
add-direct-hash-to-rule-hasher
branch
from
e8eef2a
to
a60e006
Compare
|
@alex-torok There have been some changes in main for BCR support, once those are resolved I can merge this |
|
@tinder-maxwellelliott Let me know if there is anything else I can do to help land this. |
Looks like there are some test failures |
|
I think I fixed it, but won't be able to run it locally and see until later today. |
|
@tinder-maxwellelliott - should be passing now (at least it is on my machine) |
|
I don't know if it was an intentional side-effect, but the format changes mean hashes generated pre-8.0.1 will fail parsing on 8.0.1+. We noticed since we store digests per commit on CI to speed everything up. It's pretty transient on our part because we fall back to no work avoidance when bazel-diff fails, but others might be a little surprised. |
We are having the same problem. This change is indeed surprising. I'm wondering what could be a migration path. |
|
This PR changed how hashes are calculated -- you'll get a different hash value for an old commit on the new version of If you version bazel-diff outside of your repo (i.e use a new version of bazel-diff for calculate-hashes on older commits), then you'd hit cache consistency issues. Versioning it in the repo would just mean that when you bump it, you'd create an 'all targets changed' commit. If you want the backwards compatibility, you'd need to update the We discussed this, but I think I just missed getting it in with a testcase. |
Add target distance metrics to measure how far away an impacted target is from a directly impacted target. I tried to keep the commits relatively concise, so reviewing them one-by-one from the beginning may be easier than looking at all of the changes at once.
Fixes #223