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.

Sign up for GitHub

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

Jump to bottom

cquery inherits from test not build #13491

Closed
wants to merge 1 commit into from
Closed

cquery inherits from test not build #13491

wants to merge 1 commit into from

Conversation

illicitonion
Copy link
Contributor

@illicitonion illicitonion commented May 19, 2021 •
edited
Loading

This makes directives like test --test_arg=foo present in .bazelrc files be
factored into the configuration hash for test targets.

See #13428 for extensive
context.

@google-cla google-cla bot added the cla: yes label May 19, 2021
@illicitonion illicitonion mentioned this pull request May 19, 2021
Closed
@jin jin added the team-Configurability platforms, toolchains, cquery, select(), config transitions label May 24, 2021
@gregestren gregestren self-requested a review June 22, 2021 19:06
@gregestren
Copy link
Contributor

Just want to confirm, after #13428 (comment), that this is the route you want to go.

Also a nitpick on the description: this means cquery picks up .bazelrc directives of the form:

test --some_flag

right?

For example, I expect cquery already picks up

build --test_arg=foo

but not

test --cpu=bar

gregestren
gregestren reviewed Jun 22, 2021
View reviewed changes
Copy link
Contributor

@gregestren gregestren left a comment

Choose a reason for hiding this comment

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

Can we add a quick test to block regression? https://github.com/bazelbuild/bazel/blob/080ef6eba08e8e5329c7135d39378d5841ee417f/src/test/shell/integration/configured_query_test.sh looks like the best place to me (I can't find any dedicated bazelrc tests).

src/main/java/com/google/devtools/build/lib/runtime/commands/CqueryCommand.java Show resolved Hide resolved
@illicitonion
cquery inherits from test not build
91a9443 
This makes flags like `--test_arg` present in `.bazelrc` files be
factored into the configuration hash for test targets.

See bazelbuild#13428 for extensive
context.
@bazel-io bazel-io closed this in 20aff82 Jun 29, 2021
illicitonion added a commit to illicitonion/bazel that referenced this pull request Jul 13, 2021
@illicitonion
cquery inherits from test not build
1e1a478 
This makes directives like `test --test_arg=foo` present in `.bazelrc` files be
factored into the configuration hash for test targets.

See bazelbuild#13428 for extensive
context.

Closes bazelbuild#13491.

PiperOrigin-RevId: 382143334
@illicitonion illicitonion mentioned this pull request Jul 13, 2021
Merged
illicitonion added a commit to illicitonion/bazel that referenced this pull request Jul 13, 2021
@illicitonion
cquery inherits from test not build
7920ffe 
This makes directives like `test --test_arg=foo` present in `.bazelrc` files be
factored into the configuration hash for test targets.

See bazelbuild#13428 for extensive
context.

Closes bazelbuild#13491.

PiperOrigin-RevId: 382143334
@dhalperi
Copy link
Contributor

This seems to have broken ibazel, with repro and descriptions in #13904. Maybe this is the intended behavior, and we/ibazel are now wrong about how to use cquery?

@illicitonion illicitonion deleted the cquery-inherits-test branch August 26, 2021 09:40
@illicitonion
Copy link
Contributor Author

This seems to have broken ibazel, with repro and descriptions in #13904. Maybe this is the intended behavior, and we/ibazel are now wrong about how to use cquery?

Sorry for the breakge! I don't think this is expected - let's follow up on that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregestren gregestren gregestren approved these changes

Assignees

@gregestren gregestren

Labels
cla: yes team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@illicitonion @gregestren @dhalperi @jin