Add support for building queried targets in 1 command#27058
Add support for building queried targets in 1 command#27058
Conversation
keith
force-pushed
the
ks/add-support-for-build-query
branch
from
decad34 to
4307f8c
Compare
| } catch (QueryException | InterruptedException | IOException e) { | ||
| throw new TargetPatternsHelperException( | ||
| "Error executing query: " + e.getMessage(), | ||
| TargetPatterns.Code.TARGET_PATTERNS_UNKNOWN); |
There was a problem hiding this comment.
idk what exit codes i should really use here, i didn't want to force all callers to handle this same logic, so I kept only this 1 exception type that they're already handling
This allows combining the common pattern of build and then query into a single build command that executes the passed query (or query_file) Fixes #26938 RELNOTES[inc]: Add 'build --query' flag for building the result of a bazel query in a single command
keith
force-pushed
the
ks/add-support-for-build-query
branch
from
4307f8c to
fbb7c00
Compare
keith
force-pushed
the
ks/add-support-for-build-query
branch
from
3d6f761 to
2d3e949
Compare
keith
changed the title
|
RELNOTES[NEW] seems more appropriate for a new flag, it's not an incompatible change. |
src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/runtime/commands/TargetPatternsHelper.java
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/runtime/commands/TargetPatternsHelper.java
Show resolved
Hide resolved
| if (!targets.isEmpty() && !buildRequestOptions.targetPatternFile.isEmpty()) { | ||
|
|
||
| int optionCount = 0; | ||
| if (!targets.isEmpty()) optionCount++; |
There was a problem hiding this comment.
Is there a design reason that residue targets can't be mixed with a query?
I am specifically interested in how it interacts with --remote_download_outputs=toplevel, as that is one place where we use a query like this to download only certain files, while still building everything:
bazel test --remote_download_outputs=top-level //... $(bazel query 'attr(tags, ci-download-outputs, //...)')I haven't figured out if there is an existing technical limitation that would make supporting that difficult.
There was a problem hiding this comment.
nothing inherent, i inherited this logic from targetPatternFile and I haven't traced back why it was done for that case
There was a problem hiding this comment.
i tried to look back at why it was done for that case and didn't find anything conclusive. i assumed it was because it would be complicated with negations, but didn't find that discussed. Specifically I was thinking:
# foo.txt
//bar/...
bazel build --target_patterns_file=foo.txt -- -//bar:baz
seems potentially confusing, but maybe it would be fine. im going to try to add support for it for this feature either way
|
Thank you for contributing to the Bazel repository! This pull request has been marked as stale since it has not had any activity in the last 30 days. It will be closed in the next 30 days unless any other activity occurs. If you think this PR is still relevant and should stay open, please post any comment here and the PR will no longer be marked as stale. |
github-actions
bot
added
the
stale
|
@keith This would be awesome to land. |
github-actions
bot
removed
the
stale
|
somehow i submitted this with a branch on this repo, which i don't think i was supposed to have access to do, re-opened over here #28996 |
4 participants
This allows combining the common pattern of build and then query into a
single build command that executes the passed query (or query_file)
Fixes #26938
RELNOTES[NEW]: Add 'build --target_query' flag for building the result of a
bazel query in a single command