Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--useCquery probably incorrectly handles SOURCE_FILE's #218

Closed
Asdprom opened this issue Jun 7, 2024 · 5 comments · Fixed by #251
Closed

--useCquery probably incorrectly handles SOURCE_FILE's #218

Asdprom opened this issue Jun 7, 2024 · 5 comments · Fixed by #251

Comments

@Asdprom
Copy link

Asdprom commented Jun 7, 2024

Hi!

I noticed a difference in behavior between the cquery and query implementations - on the same repository (bazel-diff-sample.zip), we get different results.

The example is quite simple: target B depends on the source file A.cc through the sample_repo local repository and target A.

//:B -> @sample_repo//:A -> @sample_repo//:A.cc

Query returns significantly more targets than cquery (with the --fineGrainedHashExternalRepos=sample_repo flag specified in both cases), when A.cc is modified.

cquery:

//sample_repo:A

query:

//sample_repo:A
@sample_repo//:A.cc
@sample_repo//:A
//sample_repo:A.cc
//:B

It is possible that the tool with the --useCquery flag is not working correctly. My suspicion falls on these two (1, 2) code fragments, where almost all sources are implicitly filtered out from the target list because SOURCE_FILE has empty rule.name. Which leads to warnings like:

[Warning] Unable to calculate digest for input @sample_repo//:A.cc for rule @sample_repo//:A

And there is no similar filtration performed when the tool is run in query mode. Sorry in advance if I somehow misunderstood this code or idea behind it, but results seem a little bit strange.

@honnix
Copy link
Contributor

honnix commented Sep 16, 2024

Maybe due to

if providers(target) == None:
# skip printing non-target results. That is, source files and generated files won't be
# printed
return ""
, which looks like by design.

@honnix
Copy link
Contributor

honnix commented Oct 22, 2024

@tinder-maxwellelliott I think this might be indeed an issue. Not only source files, but also generated files are filter out because of empty rule.name.

@honnix
Copy link
Contributor

honnix commented Oct 22, 2024

I think we might just convert Build.Target into BazelTarget already in BazelQueryService so we can call .name. But would that break the initial design (BazelQueryService should strictly dealing with Bazel proto objects)?

@tinder-maxwellelliott
Copy link
Collaborator

@honnix Id be open to a PR for this to see what the impact is

@honnix
Copy link
Contributor

honnix commented Oct 23, 2024

I put up something quickly in #251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants