-
Notifications
You must be signed in to change notification settings - Fork 179
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
Create common build settings #154
Conversation
Create common simple build settings for people to use so they don't recreate these rules over and over again. This fulfills part of the SBC design doc: https://docs.google.com/document/d/1vc8v-kXjvgZOdQdnxPTaV0rrLxtP2XwnD2tAZlYJOqw/edit#bookmark=id.iiumwic0jphr
Part of work for bazelbuild/bazel#5577 |
rules/common_settings.bzl
Outdated
build_setting = config.bool(), | ||
) | ||
|
||
string_flag = rule( |
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.
Really excited to see this landing!
Drive-by comment (possibly for a future PR): it would be nice if the string_*
flags took a list of acceptable values as an attribute, like attr.string
does, where the implementation function would verify that the value is permitted. This would handle a common case without forcing rule authors who want this logic to write their own rule/validation.
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.
Done!
+ link to full documentation + docstrings for rules/provider + "values" allowed values attr added to the string typed settings
fix missing :
rules/common_settings.bzl
Outdated
""" | ||
|
||
BuildSettingInfo = provider( | ||
doc = """A singleton provider that contains the raw value of a build setting""", |
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.
nit: Triple quotes unnecessary if it's only a single line :)
(there are multiple such instances in this file)
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.
Done.
rules/common_settings.bzl
Outdated
|
||
def _impl(ctx): | ||
return BuildSettingInfo(value = ctx.build_setting_value) | ||
|
||
int_flag = rule( | ||
implementation = _impl, | ||
build_setting = config.int(flag = True), | ||
doc = """An int-typed build setting that is user settable""", |
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.
"user-settable" is a bit confusing, how about "{may, may not} be set on the command line" ?
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.
Done.
This reverts commit 4345517.
Create common simple build settings for people to use so they don't recreate these rules over and over again.
This fulfills part of the SBC design doc: https://docs.google.com/document/d/1vc8v-kXjvgZOdQdnxPTaV0rrLxtP2XwnD2tAZlYJOqw/edit#bookmark=id.iiumwic0jphr