diff --git a/docs/py_binary.md b/docs/py_binary.md index 3ac22c02..1262c8b8 100644 --- a/docs/py_binary.md +++ b/docs/py_binary.md @@ -59,7 +59,7 @@ Run a Python program under Bazel. Most users should use the [py_binary macro](#p | env | Environment variables to set when running the binary. | Dictionary: String -> String | optional | {} | | imports | List of import directories to be added to the PYTHONPATH. | List of strings | optional | [] | | interpreter_options | Additional options to pass to the Python interpreter in addition to -B and -I passed by rules_py | List of strings | optional | [] | -| main | Script to execute with the Python interpreter. | Label | required | | +| main | Script to execute with the Python interpreter. Like rules_python, this is treated as a suffix of a file that should appear among the srcs. If absent, then [name].py is tried. As a final fallback, if the srcs has a single file, that is used as the main. | String | optional | "" | | package_collisions | The action that should be taken when a symlink collision is encountered when creating the venv. A collision can occur when multiple packages providing the same file are installed into the venv. The possible values are:

* "error": When conflicting symlinks are found, an error is reported and venv creation halts. * "warning": When conflicting symlinks are found, an warning is reported, however venv creation continues. * "ignore": When conflicting symlinks are found, no message is reported and venv creation continues. | String | optional | "error" | | python_version | Whether to build this target and its transitive deps for a specific python version. | String | optional | "" | | resolutions | Satisfy a virtual_dep with a mapping from external package name to the label of an installed package that provides it. See [virtual dependencies](/docs/virtual_deps.md). | Dictionary: Label -> String | optional | {} | diff --git a/docs/py_test.md b/docs/py_test.md index ea1294df..d4a1bc05 100644 --- a/docs/py_test.md +++ b/docs/py_test.md @@ -60,7 +60,7 @@ Run a Python program under Bazel. Most users should use the [py_test macro](#py_ | env_inherit | Specifies additional environment variables to inherit from the external environment when the test is executed by bazel test. | List of strings | optional | [] | | imports | List of import directories to be added to the PYTHONPATH. | List of strings | optional | [] | | interpreter_options | Additional options to pass to the Python interpreter in addition to -B and -I passed by rules_py | List of strings | optional | [] | -| main | Script to execute with the Python interpreter. | Label | required | | +| main | Script to execute with the Python interpreter. Like rules_python, this is treated as a suffix of a file that should appear among the srcs. If absent, then [name].py is tried. As a final fallback, if the srcs has a single file, that is used as the main. | String | optional | "" | | package_collisions | The action that should be taken when a symlink collision is encountered when creating the venv. A collision can occur when multiple packages providing the same file are installed into the venv. The possible values are:

* "error": When conflicting symlinks are found, an error is reported and venv creation halts. * "warning": When conflicting symlinks are found, an warning is reported, however venv creation continues. * "ignore": When conflicting symlinks are found, no message is reported and venv creation continues. | String | optional | "error" | | python_version | Whether to build this target and its transitive deps for a specific python version. | String | optional | "" | | resolutions | Satisfy a virtual_dep with a mapping from external package name to the label of an installed package that provides it. See [virtual dependencies](/docs/virtual_deps.md). | Dictionary: Label -> String | optional | {} | diff --git a/py/BUILD.bazel b/py/BUILD.bazel index 03a90557..db372fc2 100644 --- a/py/BUILD.bazel +++ b/py/BUILD.bazel @@ -30,7 +30,6 @@ bzl_library( visibility = ["//visibility:public"], deps = [ "//py/private:py_binary", - "//py/private:py_executable", "//py/private:py_image_layer", "//py/private:py_library", "//py/private:py_pex_binary", diff --git a/py/defs.bzl b/py/defs.bzl index b23ded39..2417a0a0 100644 --- a/py/defs.bzl +++ b/py/defs.bzl @@ -35,9 +35,7 @@ python.toolchain(python_version = "3.9", is_default = True) ``` """ -load("@aspect_bazel_lib//lib:utils.bzl", "propagate_common_rule_attributes") load("//py/private:py_binary.bzl", _py_binary = "py_binary", _py_test = "py_test") -load("//py/private:py_executable.bzl", "determine_main") load("//py/private:py_image_layer.bzl", _py_image_layer = "py_image_layer") load("//py/private:py_library.bzl", _py_library = "py_library") load("//py/private:py_pex_binary.bzl", _py_pex_binary = "py_pex_binary") @@ -63,31 +61,16 @@ py_image_layer = _py_image_layer resolutions = _resolutions def _py_binary_or_test(name, rule, srcs, main, data = [], deps = [], resolutions = {}, **kwargs): - exec_properties = kwargs.pop("exec_properties", {}) - non_test_exec_properties = {k: v for k, v in exec_properties.items() if not k.startswith("test.")} - - # Compatibility with rules_python, see docs in py_executable.bzl - main_target = "{}.find_main".format(name) - determine_main( - name = main_target, - target_name = name, - main = main, - srcs = srcs, - exec_properties = non_test_exec_properties, - **propagate_common_rule_attributes(kwargs) - ) - package_collisions = kwargs.pop("package_collisions", None) rule( name = name, srcs = srcs, - main = main_target, + main = main, data = data, deps = deps, resolutions = resolutions, package_collisions = package_collisions, - exec_properties = exec_properties, **kwargs ) diff --git a/py/private/BUILD.bazel b/py/private/BUILD.bazel index 3286ac5b..c8c53567 100644 --- a/py/private/BUILD.bazel +++ b/py/private/BUILD.bazel @@ -93,12 +93,6 @@ bzl_library( visibility = ["//py:__subpackages__"], ) -bzl_library( - name = "py_executable", - srcs = ["py_executable.bzl"], - visibility = ["//py:__subpackages__"], -) - bzl_library( name = "py_pex_binary", srcs = ["py_pex_binary.bzl"], diff --git a/py/private/py_binary.bzl b/py/private/py_binary.bzl index a4727f47..c1879924 100644 --- a/py/private/py_binary.bzl +++ b/py/private/py_binary.bzl @@ -14,10 +14,71 @@ def _dict_to_exports(env): for (k, v) in env.items() ] +def _csv(values): + """Convert a list of strings to comma separated value string.""" + return ", ".join(sorted(values)) + +def _path_endswith(path, endswith): + # Use slash to anchor each path to prevent e.g. + # "ab/c.py".endswith("b/c.py") from incorrectly matching. + return ("/" + path).endswith("/" + endswith) + +def _determine_main(ctx): + """Determine the main entry point .py source file. + + Args: + ctx: The rule ctx. + + Returns: + Artifact; the main file. If one can't be found, an error is raised. + """ + if ctx.attr.main: + # Deviation from rules_python: allow a leading colon, e.g. `main = ":my_target"` + proposed_main = ctx.attr.main.removeprefix(":") + if not proposed_main.endswith(".py"): + fail("main {} must end in '.py'".format(proposed_main)) + else: + if ctx.label.name.endswith(".py"): + fail("name {} must not end in '.py'".format(ctx.label.name)) + proposed_main = ctx.label.name + ".py" + + main_files = [src for src in ctx.files.srcs if _path_endswith(src.short_path, proposed_main)] + + # Deviation from logic in rules_python: rules_py is a bit more permissive. + # Allow a srcs of length one to determine the main, if the target name didn't match anything. + if not main_files and len(ctx.files.srcs) == 1: + main_files = ctx.files.srcs + + if not main_files: + if ctx.attr.main: + fail("could not find '{}' as specified by 'main' attribute".format(proposed_main)) + else: + fail(("corresponding default '{}' does not appear in srcs. Add " + + "it or override default file name with a 'main' attribute").format( + proposed_main, + )) + + elif len(main_files) > 1: + if ctx.attr.main: + fail(("file name '{}' specified by 'main' attributes matches multiple files. " + + "Matches: {}").format( + proposed_main, + _csv([f.short_path for f in main_files]), + )) + else: + fail(("default main file '{}' matches multiple files in srcs. Perhaps specify " + + "an explicit file with 'main' attribute? Matches were: {}").format( + proposed_main, + _csv([f.short_path for f in main_files]), + )) + return main_files[0] + def _py_binary_rule_impl(ctx): venv_toolchain = ctx.toolchains[VENV_TOOLCHAIN] py_toolchain = _py_semantics.resolve_toolchain(ctx) + main_file = _determine_main(ctx) + # Check for duplicate virtual dependency names. Those that map to the same resolution target would have been merged by the depset for us. virtual_resolution = _py_library.resolve_virtuals(ctx) imports_depset = _py_library.make_imports_depset(ctx, extra_imports_depsets = virtual_resolution.imports) @@ -78,7 +139,7 @@ def _py_binary_rule_impl(ctx): "{{ARG_PYTHON}}": to_rlocation_path(ctx, py_toolchain.python) if py_toolchain.runfiles_interpreter else py_toolchain.python.path, "{{ARG_VENV_NAME}}": ".{}.venv".format(ctx.attr.name), "{{ARG_PTH_FILE}}": to_rlocation_path(ctx, site_packages_pth_file), - "{{ENTRYPOINT}}": to_rlocation_path(ctx, ctx.file.main), + "{{ENTRYPOINT}}": to_rlocation_path(ctx, main_file), "{{PYTHON_ENV}}": "\n".join(_dict_to_exports(default_env)).strip(), "{{EXEC_PYTHON_BIN}}": "python{}".format( py_toolchain.interpreter_version_info.major, @@ -107,14 +168,13 @@ def _py_binary_rule_impl(ctx): instrumented_files_info = _py_library.make_instrumented_files_info( ctx, - extra_source_attributes = ["main"], ) return [ DefaultInfo( files = depset([ executable_launcher, - ctx.file.main, + main_file, site_packages_pth_file, ]), executable = executable_launcher, @@ -139,10 +199,13 @@ _attrs = dict({ doc = "Environment variables to set when running the binary.", default = {}, ), - "main": attr.label( - doc = "Script to execute with the Python interpreter.", - allow_single_file = True, - mandatory = True, + "main": attr.string( + doc = """Script to execute with the Python interpreter. +Like rules_python, this is treated as a suffix of a file that should appear among the srcs. +If absent, then `[name].py` is tried. As a final fallback, if the srcs has a single file, +that is used as the main. +""", + default = "", ), "python_version": attr.string( doc = """Whether to build this target and its transitive deps for a specific python version.""", diff --git a/py/private/py_executable.bzl b/py/private/py_executable.bzl deleted file mode 100644 index f9e5370a..00000000 --- a/py/private/py_executable.bzl +++ /dev/null @@ -1,93 +0,0 @@ -"""Some vendored helper code, copied from Bazel: -https://github.com/bazelbuild/bazel/blob/37983b2f89cca7cc8b0bd596f4558fa36c8fbff2/src/main/starlark/builtins_bzl/common/python/py_executable.bzl - -There's not a way for us to reference that at runtime (via @bazel_tools, say) and even if there was -we don't want to have to detect the version of Bazel that the user is running to know whether code -we want to re-use will be present. -""" - -def csv(values): - """Convert a list of strings to comma separated value string.""" - return ", ".join(sorted(values)) - -def _path_endswith(path, endswith): - # Use slash to anchor each path to prevent e.g. - # "ab/c.py".endswith("b/c.py") from incorrectly matching. - return ("/" + path).endswith("/" + endswith) - -def _determine_main(ctx): - """Determine the main entry point .py source file. - - Args: - ctx: The rule ctx. - - Returns: - Artifact; the main file. If one can't be found, an error is raised. - """ - if ctx.attr.main: - # Deviation from rules_python: allow a leading colon, e.g. `main = ":my_target"` - proposed_main = ctx.attr.main.removeprefix(":") - if not proposed_main.endswith(".py"): - fail("main {} must end in '.py'".format(proposed_main)) - else: - if ctx.attr.target_name.endswith(".py"): - fail("name {} must not end in '.py'".format(ctx.attr.target_name)) - proposed_main = ctx.attr.target_name + ".py" - - main_files = [src for src in ctx.files.srcs if _path_endswith(src.short_path, proposed_main)] - - # Deviation from logic in rules_python: rules_py is a bit more permissive. - # Allow a srcs of length one to determine the main, if the target name didn't match anything. - if not main_files and len(ctx.files.srcs) == 1: - main_files = ctx.files.srcs - - if not main_files: - if ctx.attr.main: - fail("could not find '{}' as specified by 'main' attribute".format(proposed_main)) - else: - fail(("corresponding default '{}' does not appear in srcs. Add " + - "it or override default file name with a 'main' attribute").format( - proposed_main, - )) - - elif len(main_files) > 1: - if ctx.attr.main: - fail(("file name '{}' specified by 'main' attributes matches multiple files. " + - "Matches: {}").format( - proposed_main, - csv([f.short_path for f in main_files]), - )) - else: - fail(("default main file '{}' matches multiple files in srcs. Perhaps specify " + - "an explicit file with 'main' attribute? Matches were: {}").format( - proposed_main, - csv([f.short_path for f in main_files]), - )) - return main_files[0] - -# Adapts the function above, which we copied from rules_python, to be a standalone rule so we can -# use it from a macro. -# This is because we want our underlying py_binary rule to be simple: 'main' is a mandatory label. -# The default output of this rule will be used as that label. -def _determine_main_impl(ctx): - return DefaultInfo(files = depset([_determine_main(ctx)])) - -determine_main = rule( - doc = """rules_python compatibility shim: find a main file with the given name among the srcs. - - From rules_python: - https://github.com/bazelbuild/rules_python/blob/4fe0db3cdcc063d5bdeab756e948640f3f16ae33/python/private/common/py_executable.bzl#L73 - # TODO(b/203567235): In the Java impl, any file is allowed. While marked - # label, it is more treated as a string, and doesn't have to refer to - # anything that exists because it gets treated as suffix-search string - # over `srcs`. - - We think these are lame semantics, however we want rules_py to be a drop-in replacement for rules_python. - """, - implementation = _determine_main_impl, - attrs = { - "target_name": attr.string(mandatory = True, doc = "The name of the py_binary or py_test we are finding a main for"), - "main": attr.string(doc = "Hint the user supplied as the main"), - "srcs": attr.label_list(allow_files = True), - }, -) diff --git a/py/tests/external-deps/custom-macro/macro.bzl b/py/tests/external-deps/custom-macro/macro.bzl index d5377ca8..ea705920 100644 --- a/py/tests/external-deps/custom-macro/macro.bzl +++ b/py/tests/external-deps/custom-macro/macro.bzl @@ -1,19 +1,11 @@ # buildifier: disable=module-docstring -load("//py:defs.bzl", "py_binary_rule", "py_library") +load("//py:defs.bzl", "py_binary") def click_cli_binary(name, deps = [], **kwargs): - py_library( - name = name + "_lib", - srcs = ["//py/tests/external-deps/custom-macro:__main__.py"], - ) - - # NB: we don't use the py_binary macro here, because we want our `main` attribute to be used - # exactly as specified here, rather than follow rules_python semantics. - py_binary_rule( + py_binary( name = name, - main = "//py/tests/external-deps/custom-macro:__main__.py", + srcs = ["//py/tests/external-deps/custom-macro:__main__.py"], deps = deps + [ - name + "_lib", "@pypi_click//:pkg", ], **kwargs