Skip to content

Allow using requires_none and requires_not_none at the same time#647

Closed
keith wants to merge 2 commits intobazelbuild:mainfrom
keith:ks/allow-using-requires_none-and-requires_not_none-at-the-same-time
Closed

Allow using requires_none and requires_not_none at the same time#647
keith wants to merge 2 commits intobazelbuild:mainfrom
keith:ks/allow-using-requires_none-and-requires_not_none-at-the-same-time

Conversation

@keith
Copy link
Copy Markdown
Member

@keith keith commented Mar 21, 2026

This combination can be useful during migrations from old bazel build variables
to new ones. You add your args as requiring the new feature to be not
none, and the old feature to be none.

@keith keith force-pushed the ks/allow-using-requires_none-and-requires_not_none-at-the-same-time branch from 5b81c97 to 23d63c8 Compare March 21, 2026 00:56
@armandomontanez armandomontanez added type: feature request Request for new, generally useful functionality that is missing P2 We'll consider working on this in future. (Assignee optional) category: toolchains labels Mar 24, 2026
@lilygorsheneva
Copy link
Copy Markdown
Collaborator

I don't think I understand why these combinations were forbidden in the first place, or why we only want to allow specific ones now.

@keith
Copy link
Copy Markdown
Member Author

keith commented Apr 20, 2026

maybe @armandomontanez remembers, i just assume we were being conservative and hadn't tested the full combos

@armandomontanez
Copy link
Copy Markdown
Collaborator

The original reason these were restricted to one per rule was to eliminate confusion around logical behavior of specifying multiple (e.g. OR vs AND). I'm sympathetic to the desire to reduce complexity of some rules by compacting these, but I'd rather that be done as a clearer proposal for how we can prioritize communication of expected behavior to users of the API.

The specific request is:

  • Make some test cases to ensure edge cases are understood (e.g. what if requires_none and requires_not_none for the same build variable are set?). I don't remember which of these Bazel provides correctness checks for.
  • Propose a way to ensure that users can clearly read the intent as they're parsing the instantiation of these rules in build files.

@hvadehra hvadehra removed their request for review April 21, 2026 04:42
@keith keith force-pushed the ks/allow-using-requires_none-and-requires_not_none-at-the-same-time branch from 23d63c8 to 86ac297 Compare April 23, 2026 23:55
@keith
Copy link
Copy Markdown
Member Author

keith commented Apr 23, 2026

I updated the logic to make sure you don't pass the same var to both, I didn't check what the toolchain logic verifies but i think we'll have a nicer error either way.

I think requires_none + requires_not_none is pretty clear when they can't be the same variable, since it doesn't matter what order anything is evaluated in

keith added 2 commits May 4, 2026 17:34
This combination can be useful during migrations from old bazel features
to new ones. You add your args as requiring the new feature to be not
none, and the old feature to be none.
@keith keith force-pushed the ks/allow-using-requires_none-and-requires_not_none-at-the-same-time branch from 86ac297 to 6d583b1 Compare May 5, 2026 00:34
@copybara-service copybara-service Bot closed this in 8daf978 May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: toolchains P2 We'll consider working on this in future. (Assignee optional) type: feature request Request for new, generally useful functionality that is missing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants