Filtering

[purkhusid]

Hey, this project looks great! Fills a much needed gap in the Bazel ecosystem. I was looking over the code and I was wondering if there is some nice way to filter targets using bazel-diff?

My usecase is that I need to find all our deployment targets that changed by rule kind and also our artifact targets by specific tags.
Is this somehow possible with the current release?

[tinder-maxwellelliott]

Hello there @purkhusid! As of right now there is no direct support for this use case yet! My best guidance would be to actually parse the impacted targets output of the bazel-diff, create a query out of them and write that query to a file. This should be straightforward since the targets are simply newline separated. Then you can run bazel query --query_file=MY_CUSTOM_QUERY_FILE to run your query (you would want to use --query_file to protect yourself against a command line argument limit)

What kind of API would you like to see for this kind of feature?

[purkhusid]

@tinder-maxwellelliott I've been testing bazel-diff some more and maybe it just makes sense to keep this outside of bazel-diff? Like you said I can use bazel query to do this. By introducing an API to bazel-diff I think it would have to bring something to the table that makes it better than using bazel query be it in performance or features. I will see if I hit some pain points when I try introducing this in our codebase.

[purkhusid]

@tinder-maxwellelliott Also one thing that I noticed when using bazel-diff is that it does not output the reverse dependencies of the changed targets. Is this a explicit design decision? It's not a big problem but it requires you to run a bazel query rdeps(...) query if you want to find out what targets depend on the changed target.

[tinder-maxwellelliott]

I am thinking about this more and I think you are right, I am going to update impacted-tests to be impacted-targets with a custom option to support querying for tests.

I want to handle this internally since these queries can be massive and we already handle this.

[TimothySeah]

@tinder-maxwellelliott thanks for making this! I saw your comment on the BazelCon selective testing talk (https://youtu.be/9Dk7mtIm7_A?t=1635) and this implementation is exactly what I was looking for.

However, I have a question about this issue. @purkhusid could you explain which reverse dependencies bazel-diff does not output? Look at the pseudocode at this timestamp of the selective testing talk (https://youtu.be/9Dk7mtIm7_A?t=1635). I think https://github.com/Tinder/bazel-diff/blob/master/src/main/java/com/bazel_diff/TargetHashingClient.java#L48 can just return impactedTargets rather than returning its rdeps to implement that pseudocode correctly. The reason is that each hash already recursively encapsulates all the dependencies (https://github.com/Tinder/bazel-diff/blob/master/src/main/java/com/bazel_diff/TargetHashingClient.java#L89).

Would it be possible to simply return impactedTargets without calculating its rdeps? I tried this on my repo and it worked as intended (i.e. I received a list of all the targets my file touched, including its recursive reverse dependencies). This has at least two advantages:

1. Saves a lot of time because calculating rdeps is expensive.
2. Works on more repos because rdeps does not always work on complicated repo setups.

[tinder-maxwellelliott]

I am open to moving away from the rdeps additional query, but want to hear more from @purkhusid

[purkhusid]

If I remember correctly then the output was only outputting changed targets using the files that are listed by modified-filepaths. I think we could skip rdeps if you use the -a flag that was added not too long ago: #52

I wonder if it might just be best to add a flag that disables the rdeps query? And maybe just disable the rdeps query when using the -a flag?

[tinder-maxwellelliott]

#64 is up