Skip to content

Commit

Permalink
fix: add missing +x to runtime env toolchain interpreter script (baze…
Browse files Browse the repository at this point in the history
…lbuild#2086)

The `runtime_env_toolchain_interpreter.sh` file was missing the
executable bit, which prevented the file from actually be runnable
later.

To fix, just `chmod +x` it. I also added tests to actually run using it
and verify that
it is the toolchain used by the test.

Fixes bazelbuild#2085
  • Loading branch information
rickeylev authored Jul 24, 2024
1 parent eef1d81 commit 59bb4a8
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 10 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ A brief description of the categories of changes:
([#2064](https://github.com/bazelbuild/rules_python/issues/2064)).
* (pip) Fixed pypi parse_simpleapi_html function for feeds with package metadata
containing ">" sign
* (toolchains) Added missing executable permission to
`//python/runtime_env_toolchains` interpreter script so that it is runnable.
([#2085](https://github.com/bazelbuild/rules_python/issues/2085)).

### Added
* (rules) `PYTHONSAFEPATH` is inherited from the calling environment to allow
Expand Down
Empty file modified python/private/runtime_env_toolchain_interpreter.sh
100644 → 100755
Empty file.
16 changes: 16 additions & 0 deletions tests/runtime_env_toolchain/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,22 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("//tests/support:sh_py_run_test.bzl", "py_reconfig_test")
load(":runtime_env_toolchain_tests.bzl", "runtime_env_toolchain_test_suite")

runtime_env_toolchain_test_suite(name = "runtime_env_toolchain_tests")

py_reconfig_test(
name = "toolchain_runs_test",
srcs = ["toolchain_runs_test.py"],
data = [
"//tests/support:current_build_settings",
],
extra_toolchains = [
"//python/runtime_env_toolchains:all",
# Necessary for RBE CI
"//tests/cc:all",
],
main = "toolchain_runs_test.py",
deps = ["//python/runfiles"],
)
28 changes: 28 additions & 0 deletions tests/runtime_env_toolchain/toolchain_runs_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import json
import pathlib
import platform
import unittest

from python.runfiles import runfiles


class RunTest(unittest.TestCase):
def test_ran(self):
rf = runfiles.Create()
settings_path = rf.Rlocation(
"rules_python/tests/support/current_build_settings.json"
)
settings = json.loads(pathlib.Path(settings_path).read_text())
if platform.system() == "Windows":
self.assertEqual(
"/_magic_pyruntime_sentinel_do_not_use", settings["interpreter_path"]
)
else:
self.assertIn(
"runtime_env_toolchain_interpreter.sh",
settings["interpreter"]["short_path"],
)


if __name__ == "__main__":
unittest.main()
9 changes: 9 additions & 0 deletions tests/support/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@

load("//python:py_runtime.bzl", "py_runtime")
load("//python:py_runtime_pair.bzl", "py_runtime_pair")
load(":sh_py_run_test.bzl", "current_build_settings")

package(
default_visibility = ["//:__subpackages__"],
)

platform(
name = "mac",
Expand Down Expand Up @@ -104,3 +109,7 @@ toolchain(
toolchain = ":platform_runtime_pair",
toolchain_type = "//python:toolchain_type",
)

current_build_settings(
name = "current_build_settings",
)
68 changes: 58 additions & 10 deletions tests/support/sh_py_run_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,28 @@ without the overhead of a bazel-in-bazel integration test.

load("//python:py_binary.bzl", "py_binary")
load("//python:py_test.bzl", "py_test")
load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") # buildifier: disable=bzl-visibility

def _perform_transition_impl(input_settings, attr):
settings = dict(input_settings)
settings["//command_line_option:build_python_zip"] = attr.build_python_zip
if attr.bootstrap_impl:
settings["//python/config_settings:bootstrap_impl"] = attr.bootstrap_impl
if attr.extra_toolchains:
settings["//command_line_option:extra_toolchains"] = attr.extra_toolchains
else:
settings["//command_line_option:extra_toolchains"] = input_settings["//command_line_option:extra_toolchains"]
return settings

_perform_transition = transition(
implementation = _perform_transition_impl,
inputs = [
"//python/config_settings:bootstrap_impl",
"//command_line_option:extra_toolchains",
],
outputs = [
"//command_line_option:build_python_zip",
"//command_line_option:extra_toolchains",
"//python/config_settings:bootstrap_impl",
],
)
Expand Down Expand Up @@ -79,17 +86,27 @@ def _py_reconfig_impl(ctx):
]

def _make_reconfig_rule(**kwargs):
attrs = {
"bootstrap_impl": attr.string(),
"build_python_zip": attr.string(default = "auto"),
"env": attr.string_dict(),
"extra_toolchains": attr.string_list(
doc = """
Value for the --extra_toolchains flag.
NOTE: You'll likely have to also specify //tests/cc:all (or some CC toolchain)
to make the RBE presubmits happy, which disable auto-detection of a CC
toolchain.
""",
),
"target": attr.label(executable = True, cfg = "target"),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
}
return rule(
implementation = _py_reconfig_impl,
attrs = {
"bootstrap_impl": attr.string(),
"build_python_zip": attr.string(default = "auto"),
"env": attr.string_dict(),
"target": attr.label(executable = True, cfg = "target"),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
attrs = attrs,
cfg = _perform_transition,
**kwargs
)
Expand All @@ -106,7 +123,8 @@ def py_reconfig_test(*, name, **kwargs):
**kwargs: kwargs to pass along to _py_reconfig_test and py_test.
"""
reconfig_kwargs = {}
reconfig_kwargs["bootstrap_impl"] = kwargs.pop("bootstrap_impl")
reconfig_kwargs["bootstrap_impl"] = kwargs.pop("bootstrap_impl", None)
reconfig_kwargs["extra_toolchains"] = kwargs.pop("extra_toolchains", None)
reconfig_kwargs["env"] = kwargs.get("env")
inner_name = "_{}_inner" + name
_py_reconfig_test(
Expand Down Expand Up @@ -147,3 +165,33 @@ def sh_py_run_test(*, name, sh_src, py_src, **kwargs):
main = py_src,
tags = ["manual"],
)

def _current_build_settings_impl(ctx):
info = ctx.actions.declare_file(ctx.label.name + ".json")
toolchain = ctx.toolchains[TARGET_TOOLCHAIN_TYPE]
runtime = toolchain.py3_runtime
files = [info]
ctx.actions.write(
output = info,
content = json.encode({
"interpreter": {
"short_path": runtime.interpreter.short_path if runtime.interpreter else None,
},
"interpreter_path": runtime.interpreter_path,
}),
)
return [DefaultInfo(
files = depset(files),
)]

current_build_settings = rule(
doc = """
Writes information about the current build config to JSON for testing.
This is so tests can verify information about the build config used for them.
""",
implementation = _current_build_settings_impl,
toolchains = [
TARGET_TOOLCHAIN_TYPE,
],
)

0 comments on commit 59bb4a8

Please sign in to comment.