bazel-contrib / target-determinator

Determines which Bazel targets were affected between two git commits.
Apache License 2.0
144 stars 22 forks source link

Filter incompatible targets #47

Closed alokpr closed 1 year ago

alokpr commented 1 year ago

For some reason, cquery returns incompatible targets that need to be filtered using the config operator.

alokpr commented 1 year ago

Commit 1 produces correct result but may be too slow for large projects since it calls bazel for each configured target. I will try an approach described here - https://bazel.build/extending/platforms#cquery-incompatible-target-detection

alokpr commented 1 year ago

@illicitonion Please review the latest commit. It performs much better than the first commit since it makes only one extra bazel-cquery call.

alokpr commented 1 year ago

Thanks for the help Dave. Are the tests failing due to this change? It did not look related to me.

illicitonion commented 1 year ago

Ish :) The remaining tests are failing because bazel 3.7.1 didn't support starlark cqueries, but we shouldn't be running tests against bazel 3.7.1 any more, so I'm updating to make sure we only test our supported range of Bazel versions.

alokpr commented 1 year ago

Ah, I see. So I guess the testdata repository needs to be updated?

illicitonion commented 1 year ago

We should be able to merge this after we merge https://github.com/bazel-contrib/target-determinator/pull/50 :) I just pushed a cherry-pick to this branch so we can see CI hopefully go green 🤞

Thanks again for the contribution (and patience!)

illicitonion commented 1 year ago

Sorry this has taken so long - thanks so much for the contribution!

alokpr commented 1 year ago

Thanks for shepherding it through