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

Ignore Starlark options on commands with allowResidue = False #19363

Closed

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Aug 29, 2023

Ensures that bazel version does not fail when a common line in .bazelrc contains a Starlark option (similar for sync and shutdown). There is very little chance for users being confused about these commands accepting and silently ignoring Starlark flags.

Fixes #18130 (comment)

@fmeum fmeum force-pushed the allow-starlark-options-on-version branch from 83eb660 to af95305 Compare August 29, 2023 20:02
@fmeum fmeum changed the title Ignore Starlark options for bazel version Ignore Starlark options on commands with allowResidue = False Aug 29, 2023
@fmeum fmeum requested a review from haxorz August 29, 2023 20:03
@fmeum
Copy link
Collaborator Author

fmeum commented Aug 29, 2023

@haxorz Could you review this small follow-up to the common pseudo-command effort?

@haxorz haxorz requested review from justinhorvitz and removed request for haxorz August 29, 2023 20:45
src/test/py/bazel/options_test.py Outdated Show resolved Hide resolved
Ensures that `bazel version` does not fail when a `common` line in
`.bazelrc` contains a Starlark option (similar for `sync` and
`shutdown`). There is very little chance for users being confused about
these commands accepting and silently ignoring Starlark flags.
@fmeum fmeum force-pushed the allow-starlark-options-on-version branch from af95305 to 915c13c Compare August 30, 2023 07:51
@fmeum fmeum marked this pull request as ready for review August 30, 2023 07:51
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Aug 30, 2023
@iancha1992 iancha1992 added the team-Core Skyframe, bazel query, BEP, options parsing, bazelrc label Aug 30, 2023
@sgowroji
Copy link
Member

sgowroji commented Sep 1, 2023

Hi @justinhorvitz, Since I can see that this PR has been approved, please let me know whether I should proceed with importing it.

@justinhorvitz
Copy link
Contributor

Yes it is ready to import, thanks.

@fmeum
Copy link
Collaborator Author

fmeum commented Sep 4, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Sep 4, 2023
@iancha1992
Copy link
Member

@bazel-io fork 6.4.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Sep 5, 2023
@iancha1992 iancha1992 added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 5, 2023
@copybara-service copybara-service bot closed this in 954b4d9 Sep 6, 2023
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 6, 2023
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Sep 6, 2023
Ensures that `bazel version` does not fail when a `common` line in `.bazelrc` contains a Starlark option (similar for `sync` and `shutdown`). There is very little chance for users being confused about these commands accepting and silently ignoring Starlark flags.

Fixes bazelbuild#18130 (comment)

Closes bazelbuild#19363.

PiperOrigin-RevId: 562959337
Change-Id: Ifb4f44d63f1801f6c5a8c2f421138b4014c49a06
iancha1992 pushed a commit that referenced this pull request Sep 6, 2023
…e` (#19417)

Ensures that `bazel version` does not fail when a `common` line in
`.bazelrc` contains a Starlark option (similar for `sync` and
`shutdown`). There is very little chance for users being confused about
these commands accepting and silently ignoring Starlark flags.

Fixes
#18130 (comment)

Closes #19363.

Commit
954b4d9

PiperOrigin-RevId: 562959337
Change-Id: Ifb4f44d63f1801f6c5a8c2f421138b4014c49a06

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@fmeum fmeum deleted the allow-starlark-options-on-version branch September 6, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Core Skyframe, bazel query, BEP, options parsing, bazelrc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants