Add flag to use cquery #178
Conversation
| } else { | ||
| // Unfortunately, cquery does not support streamed_proto yet. | ||
| // See https://github.com/bazelbuild/bazel/issues/17743. This poses an issue for large monorepos. | ||
| add("proto") |
There was a problem hiding this comment.
Bummer here but seems unavoidable for now
There was a problem hiding this comment.
Yeah :(. BTW, I realized there is a bug with the first commit so I have pushed another update with one more test covering the basic source code change case.
It seems that the implementation can be cleaner if source code hashing and rule target hashing are done uniformly with a deps query to include everything (rather than separate queries on source and rule targets). But that change would be pretty big so I didn't attempt it.
This PR adds two flags `--useCquery` and `--cqueryCommandOptions` which enables using `bazel cquery` instead of `bazel query` when computing build graph. This creates more accurate build graph and hence hash computation is more accurate with less false positives. A e2e test is added to demonstrate this change.
fix comment
| queryService.query(repoTargetsQuery.joinToString(" + ") { "'$it'" })) | ||
| .distinctBy { it.rule.name } | ||
| } else { | ||
| val buildTargetsQuery = listOf("//...:all-targets") + fineGrainedHashExternalRepos.map { "@$it//...:all-targets" } |
There was a problem hiding this comment.
What was the reason removing //external:all-targets from query branch? Without those repo generation targets, how would "coarse grained" hashes get computed?
There was a problem hiding this comment.
#263 is a related change where I removed rule input transformation for query.
|
👋 Just discovered this, thanks! Would you maybe consider making this the default, given the higher accuracy? |
This PR adds two flags
--useCqueryand--cqueryCommandOptionswhich enables usingbazel cqueryinstead ofbazel querywhen computing build graph. This creates more accurate build graph and hence hash computation is more accurate with less false positives.A e2e test is added to demonstrate this change.