Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add
lib/compatibility.bzl
#381New 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
base: main
Are you sure you want to change the base?
Add
lib/compatibility.bzl
#381Changes from 12 commits
fa9f398
e86f542
b6a5584
52b2e62
8190b2e
b3d4463
977a070
63d9712
e202c2b
4b34b67
6abd9ad
dc846a0
73b0ed8
5b4c97c
b77d66e
7568466
bf4a2cd
ee916b0
7939c78
c333bc4
d935d67
ded484c
8d07e16
0ee8b41
04caadf
6765506
76a22f4
0d87298
9eb896e
d27b58d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity: Is there a particular reason for why
target_compatible_with
can't deduplicate it's value before processing it? That won't help simplify this for existing Bazel versions though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that the reason is that there's some fairly generic logic somewhere that complains about duplicate labels in
label_list
attributes. E.g. I remember that adding duplicate deps topy_library
also throws a bazel error. In other words, I suspect that this behaviour cannot be changed without affecting the same logic for all other attributes. Unless we hard-code something fortarget_compatible_with
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. It's pretty easy to hit this error when programmatically adding dependencies from a macro.
@gregestren Can you perhaps shed some light on why Bazel errors out rather than stably deduping the label list? AFAIK buildifier already handles non-macro duplications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@philsc and I followed up on this on another issue/PR (which was that?). I assume the historic logic is to encourage tidy BUILD curation. I ran this by the devs most concerned about API consistency and didn't hear strong opinion on keeping it this way.