Proposal: use bazel's cquery in place of query for queryForSourceFiles checks

[josephperrott]

Currently ibazel uses bazel query to determine if the build graph has changed. However bazel query does not allow for configurations to be considered.

If you set a --define flag for your builds, every time ibazel runs bazel query, the analysis cache is invalidated in determining the query results only to have the results invalidated again by the next build.

If bazel cquery is used instead, the --define flag is now considered/valid and the correct graph is created.

cquery inherits all of its flags from build (as does run and test) which leads to all supported flags in ibazel being able to be forwarded to the "query" checks

---

Under the hood the commands that are called by ibazel in this instance look something like this:

Calls: bazel build //path/to/module:target
      ^ creates analysis cache using `--define` flag configuration

Change made to a file

Calls: bazel query //path/to/module:target
      ^ creates new analysis cache without `--define` flag, throwing out old cache
Calls: bazel build //path/to/module:target
      ^ creates new analysis cache using `--define` flag, throwing out old cache

with cquery it would instead be:

Calls: bazel build //path/to/module:target
      ^ creates analysis cache using `--define` flag configuration

Change made to a file

Calls: bazel cquery //path/to/module:target
      ^ uses analysis cache already created, updating as necessary
Calls: bazel build //path/to/module:target
      ^ uses analysis cache already created

[achew22]

@josephperrott, I think this would be a really really great feature to add. I've been doing some repo cleanup with the goal of engaging you on this issue. How could I help get you engaged?

All the code for controlling the query operation lives inside a small block inside the bazel lib.

bazel-watcher/bazel/bazel.go

Lines 216 to 244 in 7741ff3

	// Executes a query language expression over a specified subgraph of the
	// build dependency graph.
	//
	// For example, to show all C++ test rules in the strings package, use:
	//
	// res, err := b.Query('kind("cc_.*test", strings:*)')
	//
	// or to find all dependencies of //path/to/package:target, use:
	//
	// res, err := b.Query('deps(//path/to/package:target)')
	//
	// or to find a dependency path between //path/to/package:target and //dependency:
	//
	// res, err := b.Query('somepath(//path/to/package:target, //dependency)')
	func (b *bazel) Query(args ...string) (*blaze_query.QueryResult, error) {
	blazeArgs := append([]string(nil), "--output=proto", "--order_output=no", "--color=no")
	blazeArgs = append(blazeArgs, args...)
	b.WriteToStderr(true)
	b.WriteToStdout(false)
	stdoutBuffer, _ := b.newCommand("query", blazeArgs...)
	err := b.cmd.Run()
	if err != nil {
	return nil, err
	}
	return b.processQuery(stdoutBuffer.Bytes())
	}

It doesn't look to me like it would be that hard to add a new method cquery and then change all the callers over. Is this a thing I could interest you in?

I'm about to do a release that will add colors (YAY!) and fix a lot of the output formatting issues (YAY!) but would love to do a v0.11.1 as early as this afternoon if you're interested.

[github-actions]

Stale issue message

[IgorMinar]

Don't close this please.

…

On Sat, Feb 15, 2020, 4:14 PM github-actions[bot] ***@***.***> wrote: Stale issue message — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#305?email_source=notifications&email_token=AABUZ2EOYWP2BSJZZHN7EG3RDCAPFA5CNFSM4JU54OM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL32FGY#issuecomment-586654363>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABUZ2BQWL732LAMV4WOHU3RDCAPFANCNFSM4JU54OMQ> .

[github-actions]

Stale issue message

[IgorMinar]

not stale

[devversion]

@achew22 Thanks for working on this. I think unfortunately it's not improving things as there is still a bazel query call for determining the build files. This causes the analysis cache to be still discarded.

The goal would be that the analysis cache is not discarded. We could achieve this by using cquery for determining the build files, or by always using bazel query and allowing define variables.

cquery does not support buildfiles, but maybe we could make that work with upstream changes in bazel? Ideally though, bazel query would just support --define for the sake of not discarding analysis cache, unless I miss something here. This makes me wonder why the analysis cache is even discarded at all if standard bazel query is supposed to just run as part of the loading phase.

[achew22]

@devversion this sounds like a great issue to open upstream on the Bazel main repo. Unfortunately I don't have enough time to spin up on that particular issue, but I would be interested in seeing what they say. It could be something as simple as changing a Boolean value somewhere. That's something I would be happy to do.

[juliexxia]

Hey y'all, bazel engineer here. @devversion is right, query just runs the loading phase which is why it doesn't respect any build flags (like --define). That also means it shouldn't be invalidating the analysis cache because it doesn't run the analysis phase.

Out of curiosity, you're running two query commands one after another and you're seeing symptoms that looks like analysis cache invalidation? What are those symptoms?

As for bazel changes, adding --define respect to query isn't an option. cquery was created to be a query that respects build flags. But adding buildfiles() support to cquery seems possible to me. If I remember correctly, there wasn't any major technical reason this didn't happen, we just figured we'd implement when a need arose:
https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java;l=480

Here's the implementation in query:
https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java;l=410

Bazel team probably doesn't have the cycles to own this but If you wanted to investigate/make this change in the bazel repo I'd be happy to shepherd your PRs through.