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

Expose int- and string-valued build settings as Make variables #440

Merged
merged 2 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions docs/common_settings_doc.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ A bool-typed build setting that cannot be set on the command line
## int_flag

<pre>
int_flag(<a href="#int_flag-name">name</a>)
int_flag(<a href="#int_flag-name">name</a>, <a href="#int_flag-make_variable">make_variable</a>)
</pre>

An int-typed build setting that can be set on the command line
Expand All @@ -61,14 +61,15 @@ An int-typed build setting that can be set on the command line
| Name | Description | Type | Mandatory | Default |
| :------------- | :------------- | :------------- | :------------- | :------------- |
| <a id="int_flag-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | |
| <a id="int_flag-make_variable"></a>make_variable | If set, the build setting's value will be available as a Make variable with this name in the attributes of rules that list this build setting in their 'toolchains' attribute. | String | optional | <code>""</code> |


<a id="int_setting"></a>

## int_setting

<pre>
int_setting(<a href="#int_setting-name">name</a>)
int_setting(<a href="#int_setting-name">name</a>, <a href="#int_setting-make_variable">make_variable</a>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case for supporting --*_setting, if it can't be set by the user at the command line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can still be set via transitions, which combined with this PR makes it possible to control rule attributes with make variable support via transitions.

</pre>

An int-typed build setting that cannot be set on the command line
Expand All @@ -79,14 +80,15 @@ An int-typed build setting that cannot be set on the command line
| Name | Description | Type | Mandatory | Default |
| :------------- | :------------- | :------------- | :------------- | :------------- |
| <a id="int_setting-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | |
| <a id="int_setting-make_variable"></a>make_variable | If set, the build setting's value will be available as a Make variable with this name in the attributes of rules that list this build setting in their 'toolchains' attribute. | String | optional | <code>""</code> |


<a id="string_flag"></a>

## string_flag

<pre>
string_flag(<a href="#string_flag-name">name</a>, <a href="#string_flag-values">values</a>)
string_flag(<a href="#string_flag-name">name</a>, <a href="#string_flag-make_variable">make_variable</a>, <a href="#string_flag-values">values</a>)
</pre>

A string-typed build setting that can be set on the command line
Expand All @@ -97,6 +99,7 @@ A string-typed build setting that can be set on the command line
| Name | Description | Type | Mandatory | Default |
| :------------- | :------------- | :------------- | :------------- | :------------- |
| <a id="string_flag-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | |
| <a id="string_flag-make_variable"></a>make_variable | If set, the build setting's value will be available as a Make variable with this name in the attributes of rules that list this build setting in their 'toolchains' attribute. | String | optional | <code>""</code> |
| <a id="string_flag-values"></a>values | The list of allowed values for this setting. An error is raised if any other value is given. | List of strings | optional | <code>[]</code> |


Expand Down Expand Up @@ -141,7 +144,7 @@ A string list-typed build setting that cannot be set on the command line
## string_setting

<pre>
string_setting(<a href="#string_setting-name">name</a>, <a href="#string_setting-values">values</a>)
string_setting(<a href="#string_setting-name">name</a>, <a href="#string_setting-make_variable">make_variable</a>, <a href="#string_setting-values">values</a>)
</pre>

A string-typed build setting that cannot be set on the command line
Expand All @@ -152,6 +155,7 @@ A string-typed build setting that cannot be set on the command line
| Name | Description | Type | Mandatory | Default |
| :------------- | :------------- | :------------- | :------------- | :------------- |
| <a id="string_setting-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | |
| <a id="string_setting-make_variable"></a>make_variable | If set, the build setting's value will be available as a Make variable with this name in the attributes of rules that list this build setting in their 'toolchains' attribute. | String | optional | <code>""</code> |
| <a id="string_setting-values"></a>values | The list of allowed values for this setting. An error is raised if any other value is given. | List of strings | optional | <code>[]</code> |


Expand Down
40 changes: 38 additions & 2 deletions rules/common_settings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,52 @@ BuildSettingInfo = provider(
},
)

_MAKE_VARIABLE_ATTR = attr.string(
doc = "If set, the build setting's value will be available as a Make variable with this " +
"name in the attributes of rules that list this build setting in their 'toolchains' " +
"attribute.",
)

def _is_valid_make_variable_char(c):
# Restrict make variable names for consistency with predefined ones. There are no enforced
# restrictions on make variable names, but when they contain e.g. spaces or braces, they
# aren't expanded by e.g. cc_binary.
return c == "_" or c.isdigit() or (c.isalpha() and c.isupper())

def _get_template_variable_info(ctx):
make_variable = getattr(ctx.attr, "make_variable", None)
if not make_variable:
return []

if not all([_is_valid_make_variable_char(c) for c in make_variable.elems()]):
fail("Error setting " + _no_at_str(ctx.label) + ": invalid make variable name '" + make_variable + "'. Make variable names may only contain uppercase letters, digits, and underscores.")

return [
platform_common.TemplateVariableInfo({
make_variable: str(ctx.build_setting_value),
}),
]

def _impl(ctx):
return BuildSettingInfo(value = ctx.build_setting_value)
return [
BuildSettingInfo(value = ctx.build_setting_value),
] + _get_template_variable_info(ctx)

int_flag = rule(
implementation = _impl,
build_setting = config.int(flag = True),
attrs = {
"make_variable": _MAKE_VARIABLE_ATTR,
},
doc = "An int-typed build setting that can be set on the command line",
)

int_setting = rule(
implementation = _impl,
build_setting = config.int(),
attrs = {
"make_variable": _MAKE_VARIABLE_ATTR,
},
doc = "An int-typed build setting that cannot be set on the command line",
)

Expand Down Expand Up @@ -82,7 +116,7 @@ def _string_impl(ctx):
allowed_values = ctx.attr.values
value = ctx.build_setting_value
if len(allowed_values) == 0 or value in ctx.attr.values:
return BuildSettingInfo(value = value)
return [BuildSettingInfo(value = value)] + _get_template_variable_info(ctx)
else:
fail("Error setting " + _no_at_str(ctx.label) + ": invalid value '" + value + "'. Allowed values are " + str(allowed_values))

Expand All @@ -93,6 +127,7 @@ string_flag = rule(
"values": attr.string_list(
doc = "The list of allowed values for this setting. An error is raised if any other value is given.",
),
"make_variable": _MAKE_VARIABLE_ATTR,
},
doc = "A string-typed build setting that can be set on the command line",
)
Expand All @@ -104,6 +139,7 @@ string_setting = rule(
"values": attr.string_list(
doc = "The list of allowed values for this setting. An error is raised if any other value is given.",
),
"make_variable": _MAKE_VARIABLE_ATTR,
},
doc = "A string-typed build setting that cannot be set on the command line",
)
3 changes: 3 additions & 0 deletions tests/BUILD
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
load("//:bzl_library.bzl", "bzl_library")
load(":build_test_tests.bzl", "build_test_test_suite")
load(":collections_tests.bzl", "collections_test_suite")
load(":common_settings_tests.bzl", "common_settings_test_suite")
load(":dicts_tests.bzl", "dicts_test_suite")
load(":new_sets_tests.bzl", "new_sets_test_suite")
load(":partial_tests.bzl", "partial_test_suite")
Expand All @@ -24,6 +25,8 @@ build_test_test_suite()

collections_test_suite()

common_settings_test_suite()

dicts_test_suite()

new_sets_test_suite()
Expand Down
25 changes: 25 additions & 0 deletions tests/common_settings/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
load("//rules:common_settings.bzl", "int_flag", "string_flag")

int_flag(
name = "my_int_flag",
build_setting_default = 42,
make_variable = "MY_INT_FLAG",
)

string_flag(
name = "my_string_flag",
build_setting_default = "foo",
make_variable = "MY_STRING_FLAG",
)

sh_test(
name = "make_variable_test",
srcs = ["make_variable_test.sh"],
env = {
"MESSAGE": "Hello, $(MY_STRING_FLAG)! My name is $(MY_INT_FLAG).",
},
toolchains = [
":my_int_flag",
":my_string_flag",
],
)
17 changes: 17 additions & 0 deletions tests/common_settings/make_variable_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env bash

# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

[[ "$MESSAGE" == "Hello, foo! My name is 42." ]]
167 changes: 167 additions & 0 deletions tests/common_settings_tests.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Analysis tests for common_settings.bzl."""

load("//lib:unittest.bzl", "analysistest", "asserts")
load("//rules:common_settings.bzl", "int_flag", "int_setting", "string_flag", "string_setting")

def _template_variable_info_contents_test_impl(ctx):
env = analysistest.begin(ctx)

target_under_test = analysistest.target_under_test(env)
if ctx.attr.expected:
asserts.equals(
env,
expected = ctx.attr.expected,
actual = target_under_test[platform_common.TemplateVariableInfo].variables,
)
else:
asserts.false(env, platform_common.TemplateVariableInfo in target_under_test)

return analysistest.end(env)

_template_variable_info_contents_test = analysistest.make(
_template_variable_info_contents_test_impl,
attrs = {
"expected": attr.string_dict(),
},
)

def _test_template_variable_info_contents():
int_flag(
name = "my_int_flag",
build_setting_default = 42,
make_variable = "MY_INT_1",
)

_template_variable_info_contents_test(
name = "my_int_flag_test",
target_under_test = ":my_int_flag",
expected = {
"MY_INT_1": "42",
},
)

int_setting(
name = "my_int_setting",
build_setting_default = 21,
make_variable = "MY_INT_2",
)

_template_variable_info_contents_test(
name = "my_int_setting_test",
target_under_test = ":my_int_setting",
expected = {
"MY_INT_2": "21",
},
)

string_flag(
name = "my_string_flag",
build_setting_default = "foo",
make_variable = "MY_STRING_1",
)

_template_variable_info_contents_test(
name = "my_string_flag_test",
target_under_test = ":my_string_flag",
expected = {
"MY_STRING_1": "foo",
},
)

string_setting(
name = "my_string_setting",
build_setting_default = "bar",
make_variable = "MY_STRING_2",
)

_template_variable_info_contents_test(
name = "my_string_setting_test",
target_under_test = ":my_string_setting",
expected = {
"MY_STRING_2": "bar",
},
)

string_flag(
name = "my_string_flag_without_make_variable",
build_setting_default = "foo",
)

_template_variable_info_contents_test(
name = "my_string_flag_without_make_variable_test",
target_under_test = ":my_string_flag_without_make_variable",
expected = {},
)

def _failure_test_impl(ctx):
env = analysistest.begin(ctx)

asserts.expect_failure(env, ctx.attr.expected_failure)

return analysistest.end(env)

_failure_test = analysistest.make(
_failure_test_impl,
attrs = {
"expected_failure": attr.string(),
},
expect_failure = True,
)

def _test_make_variable_name_failures():
int_flag(
name = "my_failing_int_flag",
build_setting_default = 42,
make_variable = "my_int_1",
tags = ["manual"],
)

_failure_test(
name = "my_failing_int_flag_test",
target_under_test = ":my_failing_int_flag",
expected_failure = "Error setting //tests:my_failing_int_flag: invalid make variable name 'my_int_1'. Make variable names may only contain uppercase letters, digits, and underscores.",
)

string_flag(
name = "my_failing_string_flag",
build_setting_default = "foo",
make_variable = "MY STRING",
tags = ["manual"],
)

_failure_test(
name = "my_failing_string_flag_test",
target_under_test = ":my_failing_string_flag",
expected_failure = "Error setting //tests:my_failing_string_flag: invalid make variable name 'MY STRING'. Make variable names may only contain uppercase letters, digits, and underscores.",
)

def common_settings_test_suite(name = "common_settings_test_suite"):
_test_template_variable_info_contents()
_test_make_variable_name_failures()

native.test_suite(
name = "common_settings_test_suite",
tests = [
"my_int_flag_test",
"my_int_setting_test",
"my_string_flag_test",
"my_string_setting_test",
"my_string_flag_without_make_variable_test",
"my_failing_int_flag_test",
"my_failing_string_flag_test",
],
)
2 changes: 2 additions & 0 deletions tests/subpackages_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def _all_test(env):
"""Unit tests for subpackages.all."""

all_pkgs = [
"common_settings",
"copy_directory",
"copy_file",
"diff_test",
Expand All @@ -38,6 +39,7 @@ def _all_test(env):

# These exist in all cases
filtered_pkgs = [
"common_settings",
"copy_directory",
"copy_file",
"expand_template",
Expand Down