Add the parents attribute to the platform rule. #6486
Conversation
Fixes bazelbuild#6218. Change-Id: I7dcf430fbcf745654058113cf68df18a88e44c94
|
|
||
| if (parentPlatforms.size() > 1) { | ||
| throw ruleContext.throwWithAttributeError( | ||
| PlatformRule.PARENTS_PLATFORM_ATTR, "parents attribute must have a single value"); |
There was a problem hiding this comment.
Optional nit: replace "parents" with PlatformRule.PARENTS_PLATFORM_ATTR to further consolidate the source of truth?
| /* <!-- #BLAZE_RULE(platform).ATTRIBUTE(parents) --> | ||
| The label of a <code>platform</code> target that this platform should inherit from. Although | ||
| the attribute takes a list, there should be no more than one platform present. Any | ||
| constraint_settings not set directly on this platform will be found in the parent platform. |
There was a problem hiding this comment.
Did we have API reference to the precise semantics? (what happens if both platforms define the constraint_setting, {PARENT_REMOTE_EXECUTION_PROPERTIES}, etc)?
There was a problem hiding this comment.
This is the API reference, I can add more detail if anything is unclear.
There was a problem hiding this comment.
For the sake of precision, can we clarify that the child value overrides the parent value if both are set (even if that's intuitive anyway)? Is it worth mentioning the use of {PARENT_REMOTE_EXECUTION_PROPERTIES} here?
This seems like the right place o be pedantically detailed.
There was a problem hiding this comment.
Added more documentation, adding @spomorski for docs review.
| /* <!-- #BLAZE_RULE(platform).ATTRIBUTE(parents) --> | ||
| The label of a <code>platform</code> target that this platform should inherit from. Although | ||
| the attribute takes a list, there should be no more than one platform present. Any | ||
| constraint_settings not set directly on this platform will be found in the parent platform. |
Fixes #6218.