From c3042681df6d7f84072cb3a670c86009acb2bf40 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 2 Feb 2025 10:10:59 -0800 Subject: [PATCH] wip: rule builder --- python/private/builders.bzl | 124 ++++++++++++++++++++++++++++++ python/private/py_binary_rule.bzl | 22 +++--- python/private/py_executable.bzl | 62 +++++---------- python/private/py_test_rule.bzl | 22 +++--- tests/support/sh_py_run_test.bzl | 55 ++++++------- 5 files changed, 186 insertions(+), 99 deletions(-) diff --git a/python/private/builders.bzl b/python/private/builders.bzl index 50aa3ed91a..fdd724a90e 100644 --- a/python/private/builders.bzl +++ b/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, ) diff --git a/python/private/py_binary_rule.bzl b/python/private/py_binary_rule.bzl index e5cc7f6cb4..5b40f52198 100644 --- a/python/private/py_binary_rule.bzl +++ b/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() diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index be1d39d38b..088c9008fa 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -1747,32 +1747,6 @@ 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, @@ -1780,33 +1754,37 @@ def create_executable_rule(*, attrs, **kwargs): **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, diff --git a/python/private/py_test_rule.bzl b/python/private/py_test_rule.bzl index 0ff3d04928..6ad4fbddb8 100644 --- a/python/private/py_test_rule.bzl +++ b/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() diff --git a/tests/support/sh_py_run_test.bzl b/tests/support/sh_py_run_test.bzl index 7bb09b2c55..cdd570b7a5 100644 --- a/tests/support/sh_py_run_test.bzl +++ b/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)