Skip to content

Commit 87d3dc3

Browse files
chore(semantics): Remove determine_main (#702)
Vendor `rules_python`'s `determine_main` and remove the need for a separate `determine_main` target which complicates exec properties. Replaces #641 with thanks to Keith. Fixes #605 Fixes #621 Note that `py_venv_*` already mandates that a `main=` label be provided, so this is for legacy `py_binary` only. Should be removed in 2.0. ### Changes are visible to end-users: no - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes The internal `determine_main` targets have been eliminated, simplifying some advanced usages. ### Test plan - Covered by existing test cases --------- Co-authored-by: aspect-marvin[bot] <[email protected]>
1 parent b6de8cb commit 87d3dc3

File tree

6 files changed

+75
-127
lines changed

6 files changed

+75
-127
lines changed

py/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ bzl_library(
2727
visibility = ["//visibility:public"],
2828
deps = [
2929
"//py/private:py_binary",
30-
"//py/private:py_executable",
3130
"//py/private:py_image_layer",
3231
"//py/private:py_library",
3332
"//py/private:py_pex_binary",

py/defs.bzl

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ python.toolchain(python_version = "3.9", is_default = True)
3535
```
3636
"""
3737

38-
load("@aspect_bazel_lib//lib:utils.bzl", "propagate_common_rule_attributes")
3938
load("//py/private:py_binary.bzl", _py_binary = "py_binary", _py_test = "py_test")
40-
load("//py/private:py_executable.bzl", "determine_main")
4139
load("//py/private:py_image_layer.bzl", _py_image_layer = "py_image_layer")
4240
load("//py/private:py_library.bzl", _py_library = "py_library")
4341
load("//py/private:py_pex_binary.bzl", _py_pex_binary = "py_pex_binary")
@@ -62,32 +60,13 @@ py_image_layer = _py_image_layer
6260

6361
resolutions = _resolutions
6462

65-
def _py_binary_or_test(name, rule, srcs, main, data = [], deps = [], resolutions = {}, **kwargs):
66-
exec_properties = kwargs.pop("exec_properties", {})
67-
non_test_exec_properties = {k: v for k, v in exec_properties.items() if not k.startswith("test.")}
68-
69-
# Compatibility with rules_python, see docs in py_executable.bzl
70-
main_target = "{}.find_main".format(name)
71-
determine_main(
72-
name = main_target,
73-
target_name = name,
74-
main = main,
75-
srcs = srcs,
76-
exec_properties = non_test_exec_properties,
77-
**propagate_common_rule_attributes(kwargs)
78-
)
79-
80-
package_collisions = kwargs.pop("package_collisions", None)
81-
63+
def _py_binary_or_test(name, rule, srcs, main, data = [], deps = [], **kwargs):
8264
rule(
8365
name = name,
8466
srcs = srcs,
85-
main = main_target,
67+
main = main,
8668
data = data,
8769
deps = deps,
88-
resolutions = resolutions,
89-
package_collisions = package_collisions,
90-
exec_properties = exec_properties,
9170
**kwargs
9271
)
9372

@@ -97,8 +76,6 @@ def _py_binary_or_test(name, rule, srcs, main, data = [], deps = [], resolutions
9776
data = data,
9877
deps = deps,
9978
imports = kwargs.get("imports"),
100-
resolutions = resolutions,
101-
package_collisions = package_collisions,
10279
tags = ["manual"],
10380
testonly = kwargs.get("testonly", False),
10481
target_compatible_with = kwargs.get("target_compatible_with", []),

py/private/BUILD.bazel

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@ bzl_library(
9292
srcs = ["providers.bzl"],
9393
)
9494

95-
bzl_library(
96-
name = "py_executable",
97-
srcs = ["py_executable.bzl"],
98-
)
99-
10095
bzl_library(
10196
name = "transitions",
10297
srcs = ["transitions.bzl"],

py/private/py_binary.bzl

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ def _py_binary_rule_impl(ctx):
1818
venv_toolchain = ctx.toolchains[VENV_TOOLCHAIN]
1919
py_toolchain = _py_semantics.resolve_toolchain(ctx)
2020

21+
# Resolve our `main=` to a label, which it isn't
22+
main = _py_semantics.determine_main(ctx)
23+
2124
# Check for duplicate virtual dependency names. Those that map to the same resolution target would have been merged by the depset for us.
2225
virtual_resolution = _py_library.resolve_virtuals(ctx)
2326
imports_depset = _py_library.make_imports_depset(ctx, extra_imports_depsets = virtual_resolution.imports)
@@ -78,7 +81,7 @@ def _py_binary_rule_impl(ctx):
7881
"{{ARG_PYTHON}}": to_rlocation_path(ctx, py_toolchain.python) if py_toolchain.runfiles_interpreter else py_toolchain.python.path,
7982
"{{ARG_VENV_NAME}}": ".{}.venv".format(ctx.attr.name),
8083
"{{ARG_PTH_FILE}}": to_rlocation_path(ctx, site_packages_pth_file),
81-
"{{ENTRYPOINT}}": to_rlocation_path(ctx, ctx.file.main),
84+
"{{ENTRYPOINT}}": to_rlocation_path(ctx, main),
8285
"{{PYTHON_ENV}}": "\n".join(_dict_to_exports(default_env)).strip(),
8386
"{{EXEC_PYTHON_BIN}}": "python{}".format(
8487
py_toolchain.interpreter_version_info.major,
@@ -114,7 +117,7 @@ def _py_binary_rule_impl(ctx):
114117
DefaultInfo(
115118
files = depset([
116119
executable_launcher,
117-
ctx.file.main,
120+
main,
118121
site_packages_pth_file,
119122
]),
120123
executable = executable_launcher,
@@ -140,9 +143,21 @@ _attrs = dict({
140143
default = {},
141144
),
142145
"main": attr.label(
143-
doc = "Script to execute with the Python interpreter.",
144146
allow_single_file = True,
145-
mandatory = True,
147+
doc = """
148+
Script to execute with the Python interpreter.
149+
150+
Must be a label pointing to a `.py` source file.
151+
If such a label is provided, it will be honored.
152+
153+
If no label is provided AND there is only one `srcs` file, that `srcs` file will be used.
154+
155+
If there are more than one `srcs`, a file matching `{name}.py` is searched for.
156+
This is for historical compatibility with the Bazel native `py_binary` and `rules_python`.
157+
Relying on this behavior is STRONGLY discouraged, may produce warnings and may
158+
be deprecated in the future.
159+
160+
""",
146161
),
147162
"venv": attr.string(
148163
doc = """The name of the Python virtual environment within which deps should be resolved.

py/private/py_executable.bzl

Lines changed: 0 additions & 92 deletions
This file was deleted.

py/private/py_semantics.bzl

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,61 @@ def _resolve_toolchain(ctx):
8585
flags = _INTERPRETER_FLAGS,
8686
)
8787

88+
def _csv(values):
89+
"""Convert a list of strings to comma separated value string."""
90+
return ", ".join(sorted(values))
91+
92+
def _path_endswith(path, endswith):
93+
# Use slash to anchor each path to prevent e.g.
94+
# "ab/c.py".endswith("b/c.py") from incorrectly matching.
95+
return ("/" + path).endswith("/" + endswith)
96+
97+
def _determine_main(ctx):
98+
"""Determine the main entry point .py source file.
99+
100+
Args:
101+
ctx: The rule ctx.
102+
103+
Returns:
104+
Artifact; the main file. If one can't be found, an error is raised.
105+
"""
106+
if ctx.attr.main:
107+
if not ctx.attr.main.label.name.endswith(".py"):
108+
fail("main must end in '.py'")
109+
110+
# Short circuit; if the user gave us a label, believe them.
111+
return ctx.file.main
112+
113+
elif len(ctx.files.srcs) == 1:
114+
# If the user only provided one src, take that
115+
return ctx.files.srcs[0]
116+
117+
else:
118+
# Legacy rule name based searching :/
119+
if ctx.label.name.endswith(".py"):
120+
fail("name must not end in '.py'")
121+
proposed_main = ctx.label.name + ".py"
122+
123+
print(ctx.files.srcs)
124+
main_files = [src for src in ctx.files.srcs if _path_endswith(src.short_path, proposed_main)]
125+
126+
if len(main_files) > 1:
127+
fail(("file name '{}' specified by 'main' attributes matches multiple files. " +
128+
"Matches: {}").format(
129+
proposed_main,
130+
_csv([f.short_path for f in main_files]),
131+
))
132+
133+
elif len(main_files) == 1:
134+
return main_files[0]
135+
136+
else:
137+
fail("{} does not specify main=, and has multiple sources. Disambiguate the entrypoint".format(
138+
ctx.label,
139+
))
140+
88141
semantics = struct(
89142
interpreter_flags = _INTERPRETER_FLAGS,
90143
resolve_toolchain = _resolve_toolchain,
144+
determine_main = _determine_main,
91145
)

0 commit comments

Comments
 (0)