Skip to content

Commit

Permalink
refactor: consolidate py_executable_bazel, common_bazel (#2523)
Browse files Browse the repository at this point in the history
This furthers the work of removing the artificial split of code that
stemmed from
when the implementation was part of Bazel itself. Summary of changes:

* Move most of `py_executable_bazel.bzl` into `py_executable.bzl`
* Move most of `common_bazel.bzl` into `common.bzl`
* Create `precompile.bzl` for the precompile helpers. This is to avoid a
  circular dependency between common.bzl and attributes.bzl.

Work towards #2522
  • Loading branch information
rickeylev authored Dec 23, 2024
1 parent be950f9 commit 026b300
Show file tree
Hide file tree
Showing 11 changed files with 843 additions and 891 deletions.
54 changes: 21 additions & 33 deletions python/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -104,30 +104,18 @@ bzl_library(
deps = [":py_internal_bzl"],
)

bzl_library(
name = "common_bazel_bzl",
srcs = ["common_bazel.bzl"],
deps = [
":attributes_bzl",
":common_bzl",
":py_cc_link_params_info_bzl",
":py_internal_bzl",
":py_interpreter_program_bzl",
":toolchain_types_bzl",
"@bazel_skylib//lib:paths",
],
)

bzl_library(
name = "common_bzl",
srcs = ["common.bzl"],
deps = [
":cc_helper_bzl",
":py_cc_link_params_info_bzl",
":py_info_bzl",
":py_internal_bzl",
":reexports_bzl",
":rules_cc_srcs_bzl",
":semantics_bzl",
"@bazel_skylib//lib:paths",
],
)

Expand Down Expand Up @@ -199,6 +187,18 @@ bzl_library(
srcs = ["normalize_name.bzl"],
)

bzl_library(
name = "precompile_bzl",
srcs = ["precompile.bzl"],
deps = [
":attributes_bzl",
":py_internal_bzl",
":py_interpreter_program_bzl",
":toolchain_types_bzl",
"@bazel_skylib//lib:paths",
],
)

bzl_library(
name = "python_bzl",
srcs = ["python.bzl"],
Expand Down Expand Up @@ -265,8 +265,8 @@ bzl_library(
name = "py_binary_macro_bzl",
srcs = ["py_binary_macro.bzl"],
deps = [
":common_bzl",
":py_binary_rule_bzl",
":py_executable_bzl",
],
)

Expand All @@ -275,7 +275,7 @@ bzl_library(
srcs = ["py_binary_rule.bzl"],
deps = [
":attributes_bzl",
":py_executable_bazel_bzl",
":py_executable_bzl",
":semantics_bzl",
"@bazel_skylib//lib:dicts",
],
Expand Down Expand Up @@ -343,20 +343,6 @@ bzl_library(
],
)

bzl_library(
name = "py_executable_bazel_bzl",
srcs = ["py_executable_bazel.bzl"],
deps = [
":attributes_bzl",
":common_bazel_bzl",
":common_bzl",
":py_executable_bzl",
":py_internal_bzl",
":py_runtime_info_bzl",
":semantics_bzl",
],
)

bzl_library(
name = "py_executable_bzl",
srcs = ["py_executable.bzl"],
Expand All @@ -365,6 +351,7 @@ bzl_library(
":cc_helper_bzl",
":common_bzl",
":flags_bzl",
":precompile_bzl",
":py_cc_link_params_info_bzl",
":py_executable_info_bzl",
":py_info_bzl",
Expand All @@ -373,6 +360,7 @@ bzl_library(
":rules_cc_srcs_bzl",
":toolchain_types_bzl",
"@bazel_skylib//lib:dicts",
"@bazel_skylib//lib:paths",
"@bazel_skylib//lib:structs",
"@bazel_skylib//rules:common_settings",
],
Expand Down Expand Up @@ -431,8 +419,8 @@ bzl_library(
name = "py_library_rule_bzl",
srcs = ["py_library_rule.bzl"],
deps = [
":common_bazel_bzl",
":common_bzl",
":precompile_bzl",
":py_library_bzl",
],
)
Expand Down Expand Up @@ -508,7 +496,7 @@ bzl_library(
name = "py_test_macro_bzl",
srcs = ["py_test_macro.bzl"],
deps = [
":common_bazel_bzl",
":py_executable_bzl",
":py_test_rule_bzl",
],
)
Expand All @@ -519,7 +507,7 @@ bzl_library(
deps = [
":attributes_bzl",
":common_bzl",
":py_executable_bazel_bzl",
":py_executable_bzl",
":semantics_bzl",
"@bazel_skylib//lib:dicts",
],
Expand Down
61 changes: 60 additions & 1 deletion python/private/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@
# 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.
"""Various things common to Bazel and Google rule implementations."""
"""Various things common to rule implementations."""

load("@bazel_skylib//lib:paths.bzl", "paths")
load("@rules_cc//cc/common:cc_common.bzl", "cc_common")
load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")
load(":cc_helper.bzl", "cc_helper")
load(":py_cc_link_params_info.bzl", "PyCcLinkParamsInfo")
load(":py_info.bzl", "PyInfo", "PyInfoBuilder")
load(":py_internal.bzl", "py_internal")
load(":reexports.bzl", "BuiltinPyInfo")
Expand Down Expand Up @@ -262,6 +266,30 @@ def filter_to_py_srcs(srcs):
# as a valid extension.
return [f for f in srcs if f.extension == "py"]

def collect_cc_info(ctx, extra_deps = []):
"""Collect C++ information from dependencies for Bazel.
Args:
ctx: Rule ctx; must have `deps` attribute.
extra_deps: list of Target to also collect C+ information from.
Returns:
CcInfo provider of merged information.
"""
deps = ctx.attr.deps
if extra_deps:
deps = list(deps)
deps.extend(extra_deps)
cc_infos = []
for dep in deps:
if CcInfo in dep:
cc_infos.append(dep[CcInfo])

if PyCcLinkParamsInfo in dep:
cc_infos.append(dep[PyCcLinkParamsInfo].cc_info)

return cc_common.merge_cc_infos(cc_infos = cc_infos)

def collect_imports(ctx, semantics):
"""Collect the direct and transitive `imports` strings.
Expand All @@ -280,6 +308,37 @@ def collect_imports(ctx, semantics):
transitive.append(dep[BuiltinPyInfo].imports)
return depset(direct = semantics.get_imports(ctx), transitive = transitive)

def get_imports(ctx):
"""Gets the imports from a rule's `imports` attribute.
See create_binary_semantics_struct for details about this function.
Args:
ctx: Rule ctx.
Returns:
List of strings.
"""
prefix = "{}/{}".format(
ctx.workspace_name,
py_internal.get_label_repo_runfiles_path(ctx.label),
)
result = []
for import_str in ctx.attr.imports:
import_str = ctx.expand_make_variables("imports", import_str, {})
if import_str.startswith("/"):
continue

# To prevent "escaping" out of the runfiles tree, we normalize
# the path and ensure it doesn't have up-level references.
import_path = paths.normalize("{}/{}".format(prefix, import_str))
if import_path.startswith("../") or import_path == "..":
fail("Path '{}' references a path above the execution root".format(
import_str,
))
result.append(import_path)
return result

def collect_runfiles(ctx, files = depset()):
"""Collects the necessary files from the rule's context.
Expand Down
73 changes: 0 additions & 73 deletions python/private/common_bazel.bzl → python/private/precompile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,44 +13,12 @@
# limitations under the License.
"""Common functions that are specific to Bazel rule implementation"""

load("@bazel_skylib//lib:paths.bzl", "paths")
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load("@rules_cc//cc/common:cc_common.bzl", "cc_common")
load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")
load(":attributes.bzl", "PrecompileAttr", "PrecompileInvalidationModeAttr", "PrecompileSourceRetentionAttr")
load(":common.bzl", "is_bool")
load(":flags.bzl", "PrecompileFlag")
load(":py_cc_link_params_info.bzl", "PyCcLinkParamsInfo")
load(":py_internal.bzl", "py_internal")
load(":py_interpreter_program.bzl", "PyInterpreterProgramInfo")
load(":toolchain_types.bzl", "EXEC_TOOLS_TOOLCHAIN_TYPE", "TARGET_TOOLCHAIN_TYPE")

_py_builtins = py_internal

def collect_cc_info(ctx, extra_deps = []):
"""Collect C++ information from dependencies for Bazel.
Args:
ctx: Rule ctx; must have `deps` attribute.
extra_deps: list of Target to also collect C+ information from.
Returns:
CcInfo provider of merged information.
"""
deps = ctx.attr.deps
if extra_deps:
deps = list(deps)
deps.extend(extra_deps)
cc_infos = []
for dep in deps:
if CcInfo in dep:
cc_infos.append(dep[CcInfo])

if PyCcLinkParamsInfo in dep:
cc_infos.append(dep[PyCcLinkParamsInfo].cc_info)

return cc_common.merge_cc_infos(cc_infos = cc_infos)

def maybe_precompile(ctx, srcs):
"""Computes all the outputs (maybe precompiled) from the input srcs.
Expand Down Expand Up @@ -237,44 +205,3 @@ def _precompile(ctx, src, *, use_pycache):
toolchain = EXEC_TOOLS_TOOLCHAIN_TYPE,
)
return pyc

def get_imports(ctx):
"""Gets the imports from a rule's `imports` attribute.
See create_binary_semantics_struct for details about this function.
Args:
ctx: Rule ctx.
Returns:
List of strings.
"""
prefix = "{}/{}".format(
ctx.workspace_name,
_py_builtins.get_label_repo_runfiles_path(ctx.label),
)
result = []
for import_str in ctx.attr.imports:
import_str = ctx.expand_make_variables("imports", import_str, {})
if import_str.startswith("/"):
continue

# To prevent "escaping" out of the runfiles tree, we normalize
# the path and ensure it doesn't have up-level references.
import_path = paths.normalize("{}/{}".format(prefix, import_str))
if import_path.startswith("../") or import_path == "..":
fail("Path '{}' references a path above the execution root".format(
import_str,
))
result.append(import_path)
return result

def convert_legacy_create_init_to_int(kwargs):
"""Convert "legacy_create_init" key to int, in-place.
Args:
kwargs: The kwargs to modify. The key "legacy_create_init", if present
and bool, will be converted to its integer value, in place.
"""
if is_bool(kwargs.get("legacy_create_init")):
kwargs["legacy_create_init"] = 1 if kwargs["legacy_create_init"] else 0
2 changes: 1 addition & 1 deletion python/private/py_binary_macro.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
# limitations under the License.
"""Implementation of macro-half of py_binary rule."""

load(":common_bazel.bzl", "convert_legacy_create_init_to_int")
load(":py_binary_rule.bzl", py_binary_rule = "py_binary")
load(":py_executable.bzl", "convert_legacy_create_init_to_int")

def py_binary(**kwargs):
convert_legacy_create_init_to_int(kwargs)
Expand Down
6 changes: 3 additions & 3 deletions python/private/py_binary_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
load("@bazel_skylib//lib:dicts.bzl", "dicts")
load(":attributes.bzl", "AGNOSTIC_BINARY_ATTRS")
load(
":py_executable_bazel.bzl",
":py_executable.bzl",
"create_executable_rule",
"py_executable_bazel_impl",
"py_executable_impl",
)

_PY_TEST_ATTRS = {
Expand All @@ -39,7 +39,7 @@ _PY_TEST_ATTRS = {
}

def _py_binary_impl(ctx):
return py_executable_bazel_impl(
return py_executable_impl(
ctx = ctx,
is_test = False,
inherited_environment = [],
Expand Down
Loading

0 comments on commit 026b300

Please sign in to comment.