wip: rule builder

Author: rickeylev

python/private/builders.bzl

@@ -184,7 +184,131 @@ def _is_file(value):
 def _is_runfiles(value):
     return type(value) == "runfiles"
 
+def _Optional(*initial):
+    if len(initial) > 1:
+        fail("only one positional arg allowed")
+
+    # buildifier: disable=uninitialized
+    self = struct(
+        _value = list(initial),
+        present = lambda *a, **k: _Optional_present(self, *a, **k),
+        set = lambda *a, **k: _Optional_set(self, *a, **k),
+        get = lambda *a, **k: _Optional_get(self, *a, **k),
+    )
+    return self
+
+def _Optional_set(self, v):
+    if len(self._value) == 0:
+        self._value.append(v)
+    else:
+        self._value[0] = v
+
+def _Optional_get(self):
+    if not len(self._value):
+        fail("Value not present")
+    return self._value[0]
+
+def _Optional_present(self):
+    return len(self._value) > 0
+
+def _TransitionBuilder(implementation = None, inputs = None, outputs = None, **kwargs):
+    # buildifier: disable=uninitialized
+    self = struct(
+        implementation = _Optional(implementation),
+        inputs = _SetBuilder(inputs),
+        outputs = _SetBuilder(outputs),
+        kwargs = kwargs,
+        build = lambda *a, **k: _TransitionBuilder_build(self, *a, **k),
+    )
+    return self
+
+def _TransitionBuilder_build(self):
+    return transition(
+        implementation = self.implementation.get(),
+        inputs = self.inputs.build(),
+        outputs = self.outputs.build(),
+        **self.kwargs
+    )
+
+def _SetBuilder(initial = None):
+    initial = {} if not initial else {v: None for v in initial}
+
+    # buildifier: disable=uninitialized
+    self = struct(
+        _values = initial,
+        extend = lambda *a, **k: _SetBuilder_extend(self, *a, **k),
+        build = lambda *a, **k: _SetBuilder_build(self, *a, **k),
+    )
+    return self
+
+def _SetBuilder_build(self):
+    return self._values.keys()
+
+def _SetBuilder_extend(self, values):
+    for v in values:
+        if v not in self._values:
+            self._values[v] = None
+
+def _RuleBuilder(implementation = None, **kwargs):
+    # buildifier: disable=uninitialized
+    self = struct(
+        attrs = dict(kwargs.pop("attrs", None) or {}),
+        cfg = kwargs.pop("cfg", None) or _TransitionBuilder(),
+        exec_groups = dict(kwargs.pop("exec_groups", None) or {}),
+        executable = _Optional(),
+        fragments = list(kwargs.pop("fragments", None) or []),
+        implementation = _Optional(implementation),
+        extra_kwargs = kwargs,
+        provides = list(kwargs.pop("provides", None) or []),
+        test = _Optional(),
+        toolchains = list(kwargs.pop("toolchains", None) or []),
+        build = lambda *a, **k: _RuleBuilder_build(self, *a, **k),
+        to_kwargs = lambda *a, **k: _RuleBuilder_to_kwargs(self, *a, **k),
+    )
+    if "test" in kwargs:
+        self.test.set(kwargs.pop("test"))
+    if "executable" in kwargs:
+        self.executable.set(kwargs.pop("executable"))
+    return self
+
+def _RuleBuilder_build(self, debug = ""):
+    kwargs = self.to_kwargs()
+    if debug:
+        lines = ["=" * 80, "rule kwargs: {}:".format(debug)]
+        for k, v in sorted(kwargs.items()):
+            lines.append("  {}={}".format(k, v))
+
+        # buildifier: disable=print
+        print("\n".join(lines))
+    return rule(**kwargs)
+
+def _RuleBuilder_to_kwargs(self):
+    kwargs = {}
+    if self.executable.present():
+        kwargs["executable"] = self.executable.get()
+    if self.test.present():
+        kwargs["test"] = self.test.get()
+
+    kwargs.update(
+        implementation = self.implementation.get(),
+        cfg = self.cfg.build(),
+        attrs = {
+            k: (v.build() if hasattr(v, "build") else v)
+            for k, v in self.attrs.items()
+        },
+        exec_groups = self.exec_groups,
+        fragments = self.fragments,
+        provides = self.provides,
+        toolchains = self.toolchains,
+    )
+    kwargs.update(self.extra_kwargs)
+    return kwargs
+
 builders = struct(
     DepsetBuilder = _DepsetBuilder,
     RunfilesBuilder = _RunfilesBuilder,
+    RuleBuilder = _RuleBuilder,
+    TransitionBuilder = _TransitionBuilder,
+    SetBuilder = _SetBuilder,
+    Optional = _Optional,
 )

python/private/py_binary_rule.bzl

@@ -13,11 +13,10 @@
 # limitations under the License.
 """Rule implementation of py_binary for Bazel."""
 
-load("@bazel_skylib//lib:dicts.bzl", "dicts")
 load(":attributes.bzl", "AGNOSTIC_BINARY_ATTRS")
 load(
     ":py_executable.bzl",
-    "create_executable_rule",
+    "create_executable_rule_builder",
     "py_executable_impl",
 )
 
@@ -45,16 +44,13 @@ def _py_binary_impl(ctx):
         inherited_environment = [],
     )
 
-def create_binary_rule(*, attrs = None, **kwargs):
-    kwargs.setdefault("implementation", _py_binary_impl)
-    kwargs.setdefault("executable", True)
-    return create_executable_rule(
-        attrs = dicts.add(
-            AGNOSTIC_BINARY_ATTRS,
-            _COVERAGE_ATTRS,
-            attrs or {},
-        ),
-        **kwargs
+def create_binary_rule_builder():
+    builder = create_executable_rule_builder(
+        implementation = _py_binary_impl,
+        executable = True,
     )
+    builder.attrs.update(AGNOSTIC_BINARY_ATTRS)
+    builder.attrs.update(_COVERAGE_ATTRS)
+    return builder
 
-py_binary = create_binary_rule()
+py_binary = create_binary_rule_builder().build()

python/private/py_executable.bzl

@@ -1747,66 +1747,44 @@ def _transition_executable_impl(input_settings, attr):
         settings[_PYTHON_VERSION_FLAG] = attr.python_version
     return settings
 
-def create_transition(extend_implementation = None, inputs = None, outputs = None, **kwargs):
-    if extend_implementation:
-        implementation = lambda *args: extend_implementation(base_impl = _transition_executable_impl, *args)
-    else:
-        implementation = _transition_executable_impl
-
-    # todo: dedupe inputs/outputs
-    return transition(
-        implementation = implementation,
-        inputs = [_PYTHON_VERSION_FLAG] + (inputs or []),
-        outputs = [_PYTHON_VERSION_FLAG] + (outputs or []),
-        **kwargs
-    )
-
-_transition_executable = transition(
-    implementation = _transition_executable_impl,
-    inputs = [
-        _PYTHON_VERSION_FLAG,
-    ],
-    outputs = [
-        _PYTHON_VERSION_FLAG,
-    ],
-)
-
-transition_executable_impl = _transition_executable_impl
-
 def create_executable_rule(*, attrs, **kwargs):
     return create_base_executable_rule(
         attrs = attrs,
         fragments = ["py", "bazel_py"],
         **kwargs
     )
 
-def create_base_executable_rule(*, attrs, fragments = [], **kwargs):
+def create_base_executable_rule():
     """Create a function for defining for Python binary/test targets.
 
-    Args:
-        attrs: Rule attributes
-        fragments: List of str; extra config fragments that are required.
-        **kwargs: Additional args to pass onto `rule()`
-
     Returns:
         A rule function
     """
-    if "py" not in fragments:
-        # The list might be frozen, so use concatentation
-        fragments = fragments + ["py"]
-    kwargs.setdefault("provides", []).append(PyExecutableInfo)
-    kwargs["exec_groups"] = REQUIRED_EXEC_GROUPS | (kwargs.get("exec_groups") or {})
-    kwargs.setdefault("cfg", _transition_executable)
-    return rule(
-        # TODO: add ability to remove attrs, i.e. for imports attr
-        attrs = dicts.add(EXECUTABLE_ATTRS, attrs),
+    return create_executable_rule_builder().build()
+
+def create_executable_rule_builder(implementation, **kwargs):
+    builder = builders.RuleBuilder(
+        implementation = implementation,
+        attrs = EXECUTABLE_ATTRS,
+        exec_groups = REQUIRED_EXEC_GROUPS,
+        fragments = ["py", "bazel_py"],
+        provides = [PyExecutableInfo],
         toolchains = [
             TOOLCHAIN_TYPE,
             config_common.toolchain_type(EXEC_TOOLS_TOOLCHAIN_TYPE, mandatory = False),
         ] + _CC_TOOLCHAINS,
-        fragments = fragments,
+        cfg = builders.TransitionBuilder(
+            implementation = _transition_executable_impl,
+            inputs = [
+                _PYTHON_VERSION_FLAG,
+            ],
+            outputs = [
+                _PYTHON_VERSION_FLAG,
+            ],
+        ),
         **kwargs
     )
+    return builder
 
 def cc_configure_features(
         ctx,

python/private/py_test_rule.bzl

@@ -13,12 +13,11 @@
 # limitations under the License.
 """Implementation of py_test rule."""
 
-load("@bazel_skylib//lib:dicts.bzl", "dicts")
 load(":attributes.bzl", "AGNOSTIC_TEST_ATTRS")
 load(":common.bzl", "maybe_add_test_execution_info")
 load(
     ":py_executable.bzl",
-    "create_executable_rule",
+    "create_executable_rule_builder",
     "py_executable_impl",
 )
 
@@ -48,16 +47,13 @@ def _py_test_impl(ctx):
     maybe_add_test_execution_info(providers, ctx)
     return providers
 
-def create_test_rule(*, attrs = None, **kwargs):
-    kwargs.setdefault("implementation", _py_test_impl)
-    kwargs.setdefault("test", True)
-    return create_executable_rule(
-        attrs = dicts.add(
-            AGNOSTIC_TEST_ATTRS,
-            _BAZEL_PY_TEST_ATTRS,
-            attrs or {},
-        ),
-        **kwargs
+def create_test_rule_builder():
+    builder = create_executable_rule_builder(
+        implementation = _py_test_impl,
+        test = True,
     )
+    builder.attrs.update(AGNOSTIC_TEST_ATTRS)
+    builder.attrs.update(_BAZEL_PY_TEST_ATTRS)
+    return builder
 
-py_test = create_test_rule()
+py_test = create_test_rule_builder().build()

tests/support/sh_py_run_test.bzl

@@ -18,13 +18,10 @@ without the overhead of a bazel-in-bazel integration test.
 """
 
 load("@rules_shell//shell:sh_test.bzl", "sh_test")
-load("//python:py_binary.bzl", "py_binary")
-load("//python:py_test.bzl", "py_test")
-load("//python/private:py_binary_macro.bzl", "py_binary_macro")
-load("//python/private:py_binary_rule.bzl", "create_binary_rule")
-load("//python/private:py_executable.bzl", create_executable_transition = "create_transition")
-load("//python/private:py_test_macro.bzl", "py_test_macro")
-load("//python/private:py_test_rule.bzl", "create_test_rule")
+load("//python/private:py_binary_macro.bzl", "py_binary_macro")  # buildifier: disable=bzl-visibility
+load("//python/private:py_binary_rule.bzl", "create_binary_rule_builder")  # buildifier: disable=bzl-visibility
+load("//python/private:py_test_macro.bzl", "py_test_macro")  # buildifier: disable=bzl-visibility
+load("//python/private:py_test_rule.bzl", "create_test_rule_builder")  # buildifier: disable=bzl-visibility
 load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE")  # buildifier: disable=bzl-visibility
 load("//tests/support:support.bzl", "VISIBLE_FOR_TESTING")
 
@@ -40,21 +37,15 @@ def _perform_transition_impl(input_settings, attr, base_impl):
         settings["//python/config_settings:venvs_use_declare_symlink"] = attr.venvs_use_declare_symlink
     return settings
 
-_perform_transition = create_executable_transition(
-    extend_implementation = _perform_transition_impl,
-    inputs = [
-        "//python/config_settings:bootstrap_impl",
-        "//command_line_option:extra_toolchains",
-        "//python/config_settings:venvs_use_declare_symlink",
-    ],
-    outputs = [
-        "//command_line_option:build_python_zip",
-        "//command_line_option:extra_toolchains",
-        "//python/config_settings:bootstrap_impl",
-        "//python/config_settings:venvs_use_declare_symlink",
-        VISIBLE_FOR_TESTING,
-    ],
-)
+_RECONFIG_INPUTS = [
+    "//python/config_settings:bootstrap_impl",
+    "//command_line_option:extra_toolchains",
+    "//python/config_settings:venvs_use_declare_symlink",
+]
+_RECONFIG_OUTPUTS = _RECONFIG_INPUTS + [
+    "//command_line_option:build_python_zip",
+    VISIBLE_FOR_TESTING,
+]
 
 _RECONFIG_ATTRS = {
     "bootstrap_impl": attr.string(),
@@ -71,21 +62,23 @@ toolchain.
     "venvs_use_declare_symlink": attr.string(),
 }
 
-_py_reconfig_binary = create_binary_rule(
-    attrs = _RECONFIG_ATTRS,
-    cfg = _perform_transition,
-)
+def _create_reconfig_rule(builder):
+    builder.attrs.update(_RECONFIG_ATTRS)
 
-_py_reconfig_test = create_test_rule(
-    attrs = _RECONFIG_ATTRS,
-    cfg = _perform_transition,
-)
+    base_cfg_impl = builder.cfg.implementation.get()
+    builder.cfg.implementation.set(lambda *args: _perform_transition_impl(base_impl = base_cfg_impl, *args))
+    builder.cfg.inputs.extend(_RECONFIG_INPUTS)
+    builder.cfg.outputs.extend(_RECONFIG_OUTPUTS)
+    return builder.build()
+
+_py_reconfig_binary = _create_reconfig_rule(create_binary_rule_builder())
+
+_py_reconfig_test = _create_reconfig_rule(create_test_rule_builder())
 
 def py_reconfig_test(**kwargs):
     """Create a py_test with customized build settings for testing.
 
     Args:
-        name: str, name of test target.
         **kwargs: kwargs to pass along to _py_reconfig_test.
     """
     py_test_macro(_py_reconfig_test, **kwargs)