fix: add missing +x to runtime env toolchain interpreter script (#2086)

rickeylev · web-flow · commit 59bb4a88783c · 2024-07-24T02:21:27.000Z
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 #2085
diff --git 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
diff --git a/tests/runtime_env_toolchain/BUILD.bazel b/tests/runtime_env_toolchain/BUILD.bazel
@@ -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"],
+)
diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py
@@ -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()
diff --git 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
@@ -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",
     ],
 )
@@ -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
     )
@@ -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(
@@ -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,
+    ],
+)
