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

fix: Fix cquery targets filtering #251

Merged

Conversation

honnix
Copy link
Contributor

@honnix honnix commented Oct 23, 2024

Fixes #218

Convert Build.Target into BazelTarget first before applying the filtering. Doing so will make it easy to filter on name.

Note that e2e tests need to be fixed. Before I go fix those, I wanted to make sure this solution looks good. @tinder-maxwellelliott Please let me know what you think. Thanks.

Or maybe a more basic question: is this by design that cquery discarding source and generated targets, which query doesn't do?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Collaborator

@tinder-maxwellelliott tinder-maxwellelliott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

@@ -1,3 +1,11 @@
@@//src/main/java/com/integration:GuavaUserAndroid.java
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's surprising this source file appears here, but it is not filtered out by the starlark function. From my observation, it does have providers:

{"VisibilityProvider": <input file target //src/main/java/com/integration:GuavaUserAndroid.java>, "LicensesProvider": <input file target //src/main/java/com/integration:GuavaUserAndroid.java>, "FileProvider": , "FilesToRunProvider": <unknown object com.google.devtools.build.lib.analysis.FilesToRunProvider$SingleExecutableFilesToRunProvider>}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted that comment in a later commit

@honnix honnix marked this pull request as ready for review October 24, 2024 08:45
@tinder-maxwellelliott tinder-maxwellelliott merged commit ca357ff into Tinder:master Oct 24, 2024
8 checks passed
@honnix honnix deleted the fix-cquery-missing-targets branch October 24, 2024 12:04
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 this pull request may close these issues.

--useCquery probably incorrectly handles SOURCE_FILE's
3 participants