Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(py_runtime): Allow py_runtime to take an executable target as the interpreter #1621

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ A brief description of the categories of changes:
* (pip_install) the deprecated `pip_install` macro and related items have been
removed.

* (toolchains) `py_runtime` can now take an executable target. Note: runfiles from
the target are not supported yet.

### Fixed

* (gazelle) The gazelle plugin helper was not working with Python toolchains 3.11
Expand Down
30 changes: 26 additions & 4 deletions python/private/common/py_runtime_rule.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ _py_builtins = py_internal

def _py_runtime_impl(ctx):
interpreter_path = ctx.attr.interpreter_path or None # Convert empty string to None
interpreter = ctx.file.interpreter
interpreter = ctx.attr.interpreter
if (interpreter_path and interpreter) or (not interpreter_path and not interpreter):
fail("exactly one of the 'interpreter' or 'interpreter_path' attributes must be specified")

Expand All @@ -34,12 +34,30 @@ def _py_runtime_impl(ctx):
for t in ctx.attr.files
])

runfiles = ctx.runfiles()

hermetic = bool(interpreter)
if not hermetic:
if runtime_files:
fail("if 'interpreter_path' is given then 'files' must be empty")
if not paths.is_absolute(interpreter_path):
fail("interpreter_path must be an absolute path")
else:
interpreter_di = interpreter[DefaultInfo]

if _is_singleton_depset(interpreter_di.files):
interpreter = interpreter_di.files.to_list()[0]
rickeylev marked this conversation as resolved.
Show resolved Hide resolved
elif interpreter_di.files_to_run and interpreter_di.files_to_run.executable:
interpreter = interpreter_di.files_to_run.executable
runfiles = runfiles.merge(interpreter_di.default_runfiles)

runtime_files = depset(transitive = [
interpreter_di.files,
interpreter_di.default_runfiles.files,
runtime_files,
])
else:
fail("interpreter must be an executable target or must product exactly one file.")

if ctx.attr.coverage_tool:
coverage_di = ctx.attr.coverage_tool[DefaultInfo]
Expand Down Expand Up @@ -88,7 +106,7 @@ def _py_runtime_impl(ctx):
BuiltinPyRuntimeInfo(**builtin_py_runtime_info_kwargs),
DefaultInfo(
files = runtime_files,
runfiles = ctx.runfiles(),
runfiles = runfiles,
),
]

Expand Down Expand Up @@ -186,10 +204,14 @@ runtime. For a platform runtime this attribute must not be set.
""",
),
"interpreter": attr.label(
allow_single_file = True,
# We set `allow_files = True` because users should have
# the ability to set this attr to an executable target, such as
# a target created by `sh_binary`
allow_files = True,
mishazharov marked this conversation as resolved.
Show resolved Hide resolved
doc = """
For an in-build runtime, this is the target to invoke as the interpreter. For a
platform runtime this attribute must not be set.
platform runtime this attribute must not be set. WIP: If the target is executable
the runfiles may not be properly setup, see bazelbuild/rules_python/issues/1612
""",
),
"interpreter_path": attr.string(doc = """
Expand Down
3 changes: 3 additions & 0 deletions tests/py_runtime/pretend_binary
rickeylev marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/usr/bin/env bash

exit 0
26 changes: 26 additions & 0 deletions tests/py_runtime/py_runtime_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,32 @@ def _test_system_interpreter_must_be_absolute_impl(env, target):

_tests.append(_test_system_interpreter_must_be_absolute)

def _test_interpreter_sh_binary_target(name):
native.sh_binary(
rickeylev marked this conversation as resolved.
Show resolved Hide resolved
rickeylev marked this conversation as resolved.
Show resolved Hide resolved
name = "built_interpreter",
srcs = [":pretend_binary"],
)

rt_util.helper_target(
py_runtime,
name = name + "_subject",
interpreter = ":built_interpreter",
python_version = "PY3",
)
analysis_test(
name = name,
target = name + "_subject",
impl = _test_interpreter_sh_binary_target_impl,
)

def _test_interpreter_sh_binary_target_impl(env, target):
env.expect.that_target(target).provider(
PyRuntimeInfo,
factory = py_runtime_info_subject,
).interpreter_path().equals(None)

_tests.append(_test_interpreter_sh_binary_target)

def py_runtime_test_suite(name):
test_suite(
name = name,
Expand Down
Loading