From 283b885fde2c2c54ef014d64018b55bffdb3492e Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 22 Jul 2024 22:26:03 -0700 Subject: [PATCH 01/10] wip: add +x to runtime env interpreter.sh --- CHANGELOG.md | 3 ++ .../runtime_env_toolchain_interpreter.sh | 0 tests/runtime_env_toolchain/BUILD.bazel | 12 +++++++ .../toolchain_runs_test.py | 17 ++++++++++ tests/support/BUILD.bazel | 9 +++++ tests/support/sh_py_run_test.bzl | 34 ++++++++++++++++++- 6 files changed, 74 insertions(+), 1 deletion(-) mode change 100644 => 100755 python/private/runtime_env_toolchain_interpreter.sh create mode 100644 tests/runtime_env_toolchain/toolchain_runs_test.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c03a79061..7fa1cd2170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/python/private/runtime_env_toolchain_interpreter.sh b/python/private/runtime_env_toolchain_interpreter.sh old mode 100644 new mode 100755 diff --git a/tests/runtime_env_toolchain/BUILD.bazel b/tests/runtime_env_toolchain/BUILD.bazel index ebcdbaf017..c3a9e7fb95 100644 --- a/tests/runtime_env_toolchain/BUILD.bazel +++ b/tests/runtime_env_toolchain/BUILD.bazel @@ -12,6 +12,18 @@ # 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"], + main = "py_test.py", + deps = ["//python/runfiles"], +) diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py new file mode 100644 index 0000000000..513abcd6cb --- /dev/null +++ b/tests/runtime_env_toolchain/toolchain_runs_test.py @@ -0,0 +1,17 @@ +import json +import pathlib +import unittest + +from python.runfiles import runfiles + + +class RunTest(unittest.TestCase): + def test_ran(self): + rf = runfiles.Create() + settings_path = rf.Rlocation("_main/tests/support/current_build_settings.json") + settings = json.loads(pathlib.Path(settings_path).read_text()) + self.assertIn("runtime_env_toolchain_interpreter.sh", settings["interpreter"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/support/BUILD.bazel b/tests/support/BUILD.bazel index e5d5189a3b..58c74d6d48 100644 --- a/tests/support/BUILD.bazel +++ b/tests/support/BUILD.bazel @@ -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", @@ -104,3 +109,7 @@ toolchain( toolchain = ":platform_runtime_pair", toolchain_type = "//python:toolchain_type", ) + +current_build_settings( + name = "current_build_settings", +) diff --git a/tests/support/sh_py_run_test.bzl b/tests/support/sh_py_run_test.bzl index 35be484b0d..870931da87 100644 --- a/tests/support/sh_py_run_test.bzl +++ b/tests/support/sh_py_run_test.bzl @@ -19,12 +19,15 @@ 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 return settings _perform_transition = transition( @@ -34,6 +37,7 @@ _perform_transition = transition( ], outputs = [ "//command_line_option:build_python_zip", + "//command_line_option:extra_toolchains", "//python/config_settings:bootstrap_impl", ], ) @@ -85,6 +89,7 @@ def _make_reconfig_rule(**kwargs): "bootstrap_impl": attr.string(), "build_python_zip": attr.string(default = "auto"), "env": attr.string_dict(), + "extra_toolchains": attr.string_list(), "target": attr.label(executable = True, cfg = "target"), "_allowlist_function_transition": attr.label( default = "@bazel_tools//tools/allowlists/function_transition_allowlist", @@ -106,7 +111,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( @@ -147,3 +153,29 @@ 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] + files = [info] + ctx.actions.write( + output = info, + content = json.encode({ + "interpreter": toolchain.py3_runtime.interpreter.short_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, + ], +) From 9ff8fc9c87a50ac66448b63ea31399dd6a8e6803 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 08:19:08 -0700 Subject: [PATCH 02/10] fixup! wip: add +x to runtime env interpreter.sh --- tests/runtime_env_toolchain/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/runtime_env_toolchain/BUILD.bazel b/tests/runtime_env_toolchain/BUILD.bazel index c3a9e7fb95..57a3bf0b1e 100644 --- a/tests/runtime_env_toolchain/BUILD.bazel +++ b/tests/runtime_env_toolchain/BUILD.bazel @@ -24,6 +24,6 @@ py_reconfig_test( "//tests/support:current_build_settings", ], extra_toolchains = ["//python/runtime_env_toolchains:all"], - main = "py_test.py", + main = "toolchain_runs_test.py", deps = ["//python/runfiles"], ) From c13e7de08ea73255ff62a50482f89e6cfc7c72d4 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 18:08:45 -0700 Subject: [PATCH 03/10] fixup! fixup! wip: add +x to runtime env interpreter.sh --- tests/support/sh_py_run_test.bzl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/support/sh_py_run_test.bzl b/tests/support/sh_py_run_test.bzl index 870931da87..30e17e6e4c 100644 --- a/tests/support/sh_py_run_test.bzl +++ b/tests/support/sh_py_run_test.bzl @@ -28,12 +28,15 @@ def _perform_transition_impl(input_settings, attr): 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", From 491b2605a27b1096bf03cd709f05b62c2ffbcd20 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 18:17:25 -0700 Subject: [PATCH 04/10] fixup! fixup! fixup! wip: add +x to runtime env interpreter.sh --- tests/runtime_env_toolchain/BUILD.bazel | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/runtime_env_toolchain/BUILD.bazel b/tests/runtime_env_toolchain/BUILD.bazel index 57a3bf0b1e..99bdbab101 100644 --- a/tests/runtime_env_toolchain/BUILD.bazel +++ b/tests/runtime_env_toolchain/BUILD.bazel @@ -23,7 +23,11 @@ py_reconfig_test( data = [ "//tests/support:current_build_settings", ], - extra_toolchains = ["//python/runtime_env_toolchains:all"], + extra_toolchains = [ + "//python/runtime_env_toolchains:all", + # Necessary for RBE CI + "//tests/cc:all", + ], main = "toolchain_runs_test.py", deps = ["//python/runfiles"], ) From 3308a34beaf9660e509ac4765fc49160e468a4b3 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 18:24:41 -0700 Subject: [PATCH 05/10] fixup! fixup! fixup! fixup! wip: add +x to runtime env interpreter.sh --- .../toolchain_runs_test.py | 2 +- tests/support/sh_py_run_test.bzl | 35 +++++++++++++------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py index 513abcd6cb..ed68467116 100644 --- a/tests/runtime_env_toolchain/toolchain_runs_test.py +++ b/tests/runtime_env_toolchain/toolchain_runs_test.py @@ -10,7 +10,7 @@ def test_ran(self): rf = runfiles.Create() settings_path = rf.Rlocation("_main/tests/support/current_build_settings.json") settings = json.loads(pathlib.Path(settings_path).read_text()) - self.assertIn("runtime_env_toolchain_interpreter.sh", settings["interpreter"]) + self.assertIn("runtime_env_toolchain_interpreter.sh", settings["interpreter.short_path"]) if __name__ == "__main__": diff --git a/tests/support/sh_py_run_test.bzl b/tests/support/sh_py_run_test.bzl index 30e17e6e4c..183122a6ba 100644 --- a/tests/support/sh_py_run_test.bzl +++ b/tests/support/sh_py_run_test.bzl @@ -86,18 +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(), - "extra_toolchains": attr.string_list(), - "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 ) @@ -160,11 +169,15 @@ def sh_py_run_test(*, name, sh_src, py_src, **kwargs): 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": toolchain.py3_runtime.interpreter.short_path, + "interpreter": { + "short_path": runtime.interpreter.short_path if runtime.interpreter else None, + }, + "interpreter_path": runtime.interpreter_path, }), ) return [DefaultInfo( From cd8c2215bafbe3e242a1dd94cbcd46e714a78f46 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 18:27:11 -0700 Subject: [PATCH 06/10] fixup! fixup! fixup! fixup! fixup! wip: add +x to runtime env interpreter.sh --- tests/runtime_env_toolchain/toolchain_runs_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py index ed68467116..f1ce132dd5 100644 --- a/tests/runtime_env_toolchain/toolchain_runs_test.py +++ b/tests/runtime_env_toolchain/toolchain_runs_test.py @@ -10,7 +10,7 @@ def test_ran(self): rf = runfiles.Create() settings_path = rf.Rlocation("_main/tests/support/current_build_settings.json") settings = json.loads(pathlib.Path(settings_path).read_text()) - self.assertIn("runtime_env_toolchain_interpreter.sh", settings["interpreter.short_path"]) + self.assertIn("runtime_env_toolchain_interpreter.sh", settings["interpreter"]["short_path"]) if __name__ == "__main__": From c1a28e0ae83b8629a1b0d26d48652bbafbd40135 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 18:32:28 -0700 Subject: [PATCH 07/10] checking windows ci --- tests/runtime_env_toolchain/toolchain_runs_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py index f1ce132dd5..636f481758 100644 --- a/tests/runtime_env_toolchain/toolchain_runs_test.py +++ b/tests/runtime_env_toolchain/toolchain_runs_test.py @@ -10,6 +10,8 @@ def test_ran(self): rf = runfiles.Create() settings_path = rf.Rlocation("_main/tests/support/current_build_settings.json") settings = json.loads(pathlib.Path(settings_path).read_text()) + import sys + print("===Settings:", settings, file=sys.stderr) self.assertIn("runtime_env_toolchain_interpreter.sh", settings["interpreter"]["short_path"]) From bfe9c2fe399897e7278b73ca50d4a8b3784a7f4f Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 18:45:38 -0700 Subject: [PATCH 08/10] fixing windows ci --- tests/runtime_env_toolchain/toolchain_runs_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py index 636f481758..ea847b8d5c 100644 --- a/tests/runtime_env_toolchain/toolchain_runs_test.py +++ b/tests/runtime_env_toolchain/toolchain_runs_test.py @@ -1,6 +1,7 @@ import json import pathlib import unittest +import platform from python.runfiles import runfiles @@ -12,7 +13,10 @@ def test_ran(self): settings = json.loads(pathlib.Path(settings_path).read_text()) import sys print("===Settings:", settings, file=sys.stderr) - self.assertIn("runtime_env_toolchain_interpreter.sh", settings["interpreter"]["short_path"]) + 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__": From 0f7e1feee11b76ef249cdf8b62d6b090f9dd9357 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 18:54:33 -0700 Subject: [PATCH 09/10] fixing runfiles lookup for workspace --- .../toolchain_runs_test.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py index ea847b8d5c..b99d323c27 100644 --- a/tests/runtime_env_toolchain/toolchain_runs_test.py +++ b/tests/runtime_env_toolchain/toolchain_runs_test.py @@ -1,7 +1,7 @@ import json import pathlib -import unittest import platform +import unittest from python.runfiles import runfiles @@ -9,14 +9,19 @@ class RunTest(unittest.TestCase): def test_ran(self): rf = runfiles.Create() - settings_path = rf.Rlocation("_main/tests/support/current_build_settings.json") + settings_path = rf.Rlocation( + "rules_python/tests/support/current_build_settings.json" + ) settings = json.loads(pathlib.Path(settings_path).read_text()) - import sys - print("===Settings:", settings, file=sys.stderr) if platform.system == "Windows": - self.assertEqual("/_magic_pyruntime_sentinel_do_not_use", settings["interpreter_path"]) + self.assertEqual( + "/_magic_pyruntime_sentinel_do_not_use", settings["interpreter_path"] + ) else: - self.assertIn("runtime_env_toolchain_interpreter.sh", settings["interpreter"]["short_path"]) + self.assertIn( + "runtime_env_toolchain_interpreter.sh", + settings["interpreter"]["short_path"], + ) if __name__ == "__main__": From face1d0807c243700a57d879fef895aa991724e4 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Jul 2024 19:08:10 -0700 Subject: [PATCH 10/10] fixup! fixing runfiles lookup for workspace --- tests/runtime_env_toolchain/toolchain_runs_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py index b99d323c27..7be2472e8b 100644 --- a/tests/runtime_env_toolchain/toolchain_runs_test.py +++ b/tests/runtime_env_toolchain/toolchain_runs_test.py @@ -13,7 +13,7 @@ def test_ran(self): "rules_python/tests/support/current_build_settings.json" ) settings = json.loads(pathlib.Path(settings_path).read_text()) - if platform.system == "Windows": + if platform.system() == "Windows": self.assertEqual( "/_magic_pyruntime_sentinel_do_not_use", settings["interpreter_path"] )