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

Support setting python_runtime stub_shebang in python repository #804

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
13 changes: 12 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,18 @@ pip_parse(
```

After registration, your Python targets will use the toolchain's interpreter during execution, but a system-installed interpreter
is still used to 'bootstrap' Python targets (see https://github.com/bazelbuild/rules_python/issues/691).
is still used to 'bootstrap' Python targets (see https://github.com/bazelbuild/rules_python/issues/691). It is possible to avoid the
system-installed intrepreter by setting `hermetic_stub_shebang`
when not on windows. However if executing a Python executable
target outside bazel, the python interpreteter used to 'bootstrap' may not be found:
```
python_register_toolchains(
...
hermetic_stub_shebang = True
...
)
```

You may also find some quirks while using this toolchain. Please refer to [python-build-standalone documentation's _Quirks_ section](https://python-build-standalone.readthedocs.io/en/latest/quirks.html) for details.

### Toolchain usage in other rules
Expand Down
13 changes: 13 additions & 0 deletions python/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ def _python_repository_impl(rctx):
"share/**",
]

hermetic_stub_shebang = "#!external/{name}/{python_bin}".format(name = rctx.attr.name, python_bin = python_bin)
stub_shebang = hermetic_stub_shebang if rctx.attr.hermetic_stub_shebang else None

build_content = """\
# Generated by python/repositories.bzl

Expand Down Expand Up @@ -214,6 +217,7 @@ py_runtime(
files = [":files"],
interpreter = "{python_path}",
python_version = "PY3",
stub_shebang = {stub_shebang},
)

py_runtime_pair(
Expand All @@ -223,6 +227,7 @@ py_runtime_pair(
)
""".format(
glob_include = repr(glob_include),
stub_shebang = repr(stub_shebang),
python_path = python_bin,
python_version = python_short_version,
)
Expand All @@ -233,6 +238,7 @@ py_runtime_pair(
return {
"distutils": rctx.attr.distutils,
"distutils_content": rctx.attr.distutils_content,
"hermetic_stub_shebang": rctx.attr.hermetic_stub_shebang,
"name": rctx.attr.name,
"platform": platform,
"python_version": python_version,
Expand All @@ -257,6 +263,13 @@ python_repository = repository_rule(
"Either distutils or distutils_content can be specified, but not both.",
mandatory = False,
),
"hermetic_stub_shebang": attr.bool(
default = False,
doc = "Whether to use the Python toolchain's binary for the underlying python_runtime stub_shebang, " +
"rather than the Python binary in the path. Consider using this if the host may not have python installed. " +
"However if a py_binary executable is used outside bazel the executable may fail due to not find the Python binary.",
mandatory = False,
),
"ignore_root_user_error": attr.bool(
default = False,
doc = "Whether the check for root should be ignored or not. This causes cache misses with .pyc files.",
Expand Down
48 changes: 31 additions & 17 deletions python/tests/toolchains/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,26 @@ powershell.exe -c "& ./{interpreter_path} {run_acceptance_test_py}"
"""

def _acceptance_test_impl(ctx):
workspace = ctx.actions.declare_file("/".join([ctx.attr.python_version, "WORKSPACE"]))
stub_shebang = "hermetic-stub-shebang" if ctx.attr.hermetic_stub_shebang else "default-stub-shebang"
workspace_dir = "/".join([ctx.attr.python_version, stub_shebang])
workspace = ctx.actions.declare_file("/".join([workspace_dir, "WORKSPACE"]))
ctx.actions.expand_template(
template = ctx.file._workspace_tmpl,
output = workspace,
substitutions = {"%python_version%": ctx.attr.python_version},
substitutions = {
"%hermetic_stub_shebang%": repr(ctx.attr.hermetic_stub_shebang),
"%python_version%": ctx.attr.python_version,
},
)

build_bazel = ctx.actions.declare_file("/".join([ctx.attr.python_version, "BUILD.bazel"]))
build_bazel = ctx.actions.declare_file("/".join([workspace_dir, "BUILD.bazel"]))
ctx.actions.expand_template(
template = ctx.file._build_bazel_tmpl,
output = build_bazel,
substitutions = {"%python_version%": ctx.attr.python_version},
)

python_version_test = ctx.actions.declare_file("/".join([ctx.attr.python_version, "python_version_test.py"]))
python_version_test = ctx.actions.declare_file("/".join([workspace_dir, "python_version_test.py"]))

# With the current approach in the run_acceptance_test.sh, we use this
# symlink to find the absolute path to the rules_python to be passed to the
Expand All @@ -48,14 +53,14 @@ def _acceptance_test_impl(ctx):
output = python_version_test,
)

run_acceptance_test_py = ctx.actions.declare_file("/".join([ctx.attr.python_version, "run_acceptance_test.py"]))
run_acceptance_test_py = ctx.actions.declare_file("/".join([workspace_dir, "run_acceptance_test.py"]))
ctx.actions.expand_template(
template = ctx.file._run_acceptance_test_tmpl,
output = run_acceptance_test_py,
substitutions = {
"%is_windows%": str(ctx.attr.is_windows),
"%python_version%": ctx.attr.python_version,
"%test_location%": "/".join([ctx.attr.test_location, ctx.attr.python_version]),
"%test_location%": "/".join([ctx.attr.test_location, workspace_dir]),
},
)

Expand All @@ -66,7 +71,7 @@ def _acceptance_test_impl(ctx):
interpreter_path = py3_runtime.interpreter.short_path

if ctx.attr.is_windows:
executable = ctx.actions.declare_file("run_test_{}.bat".format(ctx.attr.python_version))
executable = ctx.actions.declare_file("run_test_{}_{}.bat".format(ctx.attr.python_version, stub_shebang))
ctx.actions.write(
output = executable,
content = _WINDOWS_RUNNER_TEMPLATE.format(
Expand All @@ -76,7 +81,7 @@ def _acceptance_test_impl(ctx):
is_executable = True,
)
else:
executable = ctx.actions.declare_file("run_test_{}.sh".format(ctx.attr.python_version))
executable = ctx.actions.declare_file("run_test_{}_{}.sh".format(ctx.attr.python_version, stub_shebang))
ctx.actions.write(
output = executable,
content = "exec '{interpreter_path}' '{run_acceptance_test_py}'".format(
Expand Down Expand Up @@ -109,6 +114,10 @@ _acceptance_test = rule(
implementation = _acceptance_test_impl,
doc = "A rule for the toolchain acceptance tests.",
attrs = {
"hermetic_stub_shebang": attr.bool(
doc = "Whether to enable Python toolchain stub shebang for executable wrapper.",
mandatory = True,
),
"is_windows": attr.bool(
doc = "(Provided by the macro) Whether this is running under Windows or not.",
mandatory = True,
Expand Down Expand Up @@ -146,8 +155,9 @@ _acceptance_test = rule(
toolchains = ["@bazel_tools//tools/python:toolchain_type"],
)

def acceptance_test(python_version, **kwargs):
def acceptance_test(python_version, hermetic_stub_shebang = False, **kwargs):
_acceptance_test(
hermetic_stub_shebang = hermetic_stub_shebang,
is_windows = select({
"@bazel_tools//src/conditions:host_windows": True,
"//conditions:default": False,
Expand All @@ -165,11 +175,15 @@ def acceptance_tests():
for platform, meta in PLATFORMS.items():
if platform not in TOOL_VERSIONS[python_version]["sha256"]:
continue
acceptance_test(
name = "python_{python_version}_{platform}_test".format(
python_version = python_version.replace(".", "_"),
platform = platform,
),
python_version = python_version,
target_compatible_with = meta.compatible_with,
)
for hermetic_stub_shebang in [False, True]:
stub_shebang = "hermetic-stub-shebang" if hermetic_stub_shebang else "default-stub-shebang"
acceptance_test(
name = "python_{python_version}_{platform}_{stub_shebang}_test".format(
python_version = python_version.replace(".", "_"),
platform = platform,
stub_shebang = stub_shebang,
),
python_version = python_version,
hermetic_stub_shebang = hermetic_stub_shebang,
target_compatible_with = meta.compatible_with,
)
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ load("@rules_python//python:repositories.bzl", "python_register_toolchains")
python_register_toolchains(
name = "python",
python_version = "%python_version%",
hermetic_stub_shebang = %hermetic_stub_shebang%,
)