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

While build settings allow for much cleaner flag and setting definitions
than `--define`, they have the major drawback that rules need to provide
dedicated support for them, which isn't the case for native and most
community-maintained rules.

This change attempts to bridge this gap by optionally exposing the value
of the common build setting types as Make variables to rules that depend
on them via the `toolchains` attribute: If the new `make_variable`
attribute is set, the value of the flag or setting is available as a
Make variable with that.

Consistency with pre-defined Make variables is enforced by limiting the
character set for `make_variable` values to `[A-Z0-9_]`. The new
attribute is also only added to int- and string-valued build settings as
the other types lack a canonical stringification.

Co-authored-by: Xùdōng Yáng <wyverald@gmail.com>

7 files changed, 260 insertions, 6 deletions

diff --git a/docs/common_settings_doc.md b/docs/common_settings_doc.md
index 7de7965..b224f52 100755
--- a/docs/common_settings_doc.md
+++ b/docs/common_settings_doc.md
@@ -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
@@ -61,6 +61,7 @@ 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>
@@ -68,7 +69,7 @@ An int-typed build setting that can be set on the command line
 ## 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>)
 </pre>
 
 An int-typed build setting that cannot be set on the command line
@@ -79,6 +80,7 @@ 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>
@@ -86,7 +88,7 @@ An int-typed build setting that cannot be set on the command line
 ## 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
@@ -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> |
 
 
@@ -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
@@ -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> |
 
 
diff --git a/rules/common_settings.bzl b/rules/common_settings.bzl
index eb057cc..58a15ae 100644
--- a/rules/common_settings.bzl
+++ b/rules/common_settings.bzl
@@ -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",
 )
 
@@ -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))
 
@@ -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",
 )
@@ -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",
 )
diff --git a/tests/BUILD b/tests/BUILD
index bbab077..7f056d2 100644
--- a/tests/BUILD
+++ b/tests/BUILD
@@ -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")
@@ -24,6 +25,8 @@ build_test_test_suite()
 
 collections_test_suite()
 
+common_settings_test_suite()
+
 dicts_test_suite()
 
 new_sets_test_suite()
diff --git a/tests/common_settings/BUILD b/tests/common_settings/BUILD
new file mode 100644
index 0000000..bbf32d5
--- /dev/null
+++ b/tests/common_settings/BUILD
@@ -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",
+    ],
+)
diff --git a/tests/common_settings/make_variable_test.sh b/tests/common_settings/make_variable_test.sh
new file mode 100755
index 0000000..546c53d
--- /dev/null
+++ b/tests/common_settings/make_variable_test.sh
@@ -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." ]]
diff --git a/tests/common_settings_tests.bzl b/tests/common_settings_tests.bzl
new file mode 100644
index 0000000..efbe0e0
--- /dev/null
+++ b/tests/common_settings_tests.bzl
@@ -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",
+        ],
+    )
diff --git a/tests/subpackages_tests.bzl b/tests/subpackages_tests.bzl
index 07a52e9..3c494d6 100644
--- a/tests/subpackages_tests.bzl
+++ b/tests/subpackages_tests.bzl
@@ -21,6 +21,7 @@ def _all_test(env):
     """Unit tests for subpackages.all."""
 
     all_pkgs = [
+        "common_settings",
         "copy_directory",
         "copy_file",
         "diff_test",
@@ -38,6 +39,7 @@ def _all_test(env):
 
     # These exist in all cases
     filtered_pkgs = [
+        "common_settings",
         "copy_directory",
         "copy_file",
         "expand_template",