Target was indirectly impacted, but has no impacted dependencies

[agustinmista]

Hey!

We are evaluating using target distance metrics to implement better job avoidance in our setup and started hitting this exception when making changes in some of our autogenerated targets.

We are currently investigating the issue, but are concerned about bazel-diff becoming a blocker if it crashes in CI upon changes in problematic targets we might not be aware of. With this in mind, would it make sense to turn this error into a warning and defaulting the computed distance to a conservative value like 0?

[tinder-maxwellelliott]

@alex-torok What is the reasoning behind this being exceptional? I believe I have also seen this fail before for us. It might make sense to add a comment around this logic and allow for a warning instead?

[alex-torok]

For a target to be indirectly changed, but not have any of its dependencies be changed, something must have gone wrong in the tracking of dependencies between targets or the computation of directly changed targets.

If a target wasn't directly changed and it has no changed dependencies, then what changed it???

The exception could be turned into a warning, but it likely means that there is a bug or failing edge condition hiding in the bazel-diff code for determining changes. Having a reliable reproduction would help for fixing the issue and getting a testcase in the repo.

[agustinmista]

Thanks both @tinder-maxwellelliott and @alex-torok for the quick reply! 😃

If a target wasn't directly changed and it has no changed dependencies, then what changed it???

Indeed a good question, I wonder if this is triggered by one of our rules breaking an assumption made by the distance analysis.

The exception could be turned into a warning, but it likely means that there is a bug or failing edge condition hiding in the bazel-diff code for determining changes. Having a reliable reproduction would help for fixing the issue and getting a testcase in the repo.

I unfortunately can't provide a reproducer at this time (the only datapoint we have comes from a rather gigantic rule) but, if we could turn this exception into a warning, that could unblock us to deploy this and collect any warning generated in the process. Hopefully with more data there will be some common pattern emerging that we could look into.

[alex-torok]

Something that would tell you exactly where the issue is:

You could add a verbose logging mode in the code that hashes targets so that it prints each piece of data that is being added to the hash digests.

If you run this twice on the base and compare revision, then diff the output, you should be able to figure out what is going on.

We have this in our internal implementation of target diffing. It lets you answer tough questions and look inside the target hashing logic, which is a black box that typically only gives you final hash digests.

[agustinmista]

We think we found the issue: computing distance metrics seems to be incompatible with filtering generated hashes with --targetType. We were filtering targets by --targetType=Rule, and the missing one in the dependency chain was a GeneratedFile. Removing the flag fixed the problem.

I think it would be good to make bazel-diff print a warning saying that filtering target types is incompatible with distance metrics if the user enables both --targetType and --depsFile.

Not sure if this is a limitation one could easily overcome, unless the filtering is done after the hash files are generated, i.e., moving --targetType from generate-hashes to get-impacted-targets, and always computing hashes for all kinds of targets by default.

[tinder-maxwellelliott]

I think it would be good to make bazel-diff print a warning saying that filtering target types is incompatible with distance metrics if the user enables both --targetType and --depsFile.

This looks like a great idea to gate this experience from others, open to the latter suggestion around moving --targetType but that will be a breaking change