feat: Improve exec error handling (bazel-contrib#1368)

chrislovecnm · rickeylev · web-flow · commit 99695ee7ba21 · 2023-08-10T18:12:17.000Z
At times binaries are not in the path. This commit tests that the binary exists before we try to execute the binary. This allows us to provide a more informative error message to the user. Closes: bazel-contrib#662 --------- Co-authored-by: Richard Levasseur <richardlev@gmail.com>
diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl
@@ -24,6 +24,7 @@ load("//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED")
 load("//python/private:normalize_name.bzl", "normalize_name")
 load("//python/private:render_pkg_aliases.bzl", "render_pkg_aliases")
 load("//python/private:toolchains_repo.bzl", "get_host_os_arch")
+load("//python/private:which.bzl", "which_with_fail")
 
 CPPFLAGS = "CPPFLAGS"
 
@@ -108,10 +109,7 @@ def _get_xcode_location_cflags(rctx):
     if not rctx.os.name.lower().startswith("mac os"):
         return []
 
-    # Locate xcode-select
-    xcode_select = rctx.which("xcode-select")
-
-    xcode_sdk_location = rctx.execute([xcode_select, "--print-path"])
+    xcode_sdk_location = rctx.execute([which_with_fail("xcode-select", rctx), "--print-path"])
     if xcode_sdk_location.return_code != 0:
         return []
 
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
@@ -57,6 +57,15 @@ bzl_library(
     deps = ["@bazel_skylib//lib:types"],
 )
 
+bzl_library(
+    name = "which_bzl",
+    srcs = ["which.bzl"],
+    visibility = [
+        "//docs:__subpackages__",
+        "//python:__subpackages__",
+    ],
+)
+
 bzl_library(
     name = "py_cc_toolchain_bzl",
     srcs = [
diff --git a/python/private/toolchains_repo.bzl b/python/private/toolchains_repo.bzl
@@ -30,6 +30,7 @@ load(
     "PLATFORMS",
     "WINDOWS_NAME",
 )
+load(":which.bzl", "which_with_fail")
 
 def get_repository_name(repository_workspace):
     dummy_label = "//:_"
@@ -325,7 +326,7 @@ def get_host_os_arch(rctx):
         os_name = WINDOWS_NAME
     else:
         # This is not ideal, but bazel doesn't directly expose arch.
-        arch = rctx.execute(["uname", "-m"]).stdout.strip()
+        arch = rctx.execute([which_with_fail("uname", rctx), "-m"]).stdout.strip()
 
         # Normalize the os_name.
         if "mac" in os_name.lower():
diff --git a/python/private/which.bzl b/python/private/which.bzl
@@ -0,0 +1,32 @@
+# Copyright 2023 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Wrapper for repository which call"""
+
+_binary_not_found_msg = "Unable to find the binary '{binary_name}'.  Please update your PATH to include '{binary_name}'."
+
+def which_with_fail(binary_name, rctx):
+    """Tests to see if a binary exists, and otherwise fails with a message.
+
+    Args:
+        binary_name: name of the binary to find.
+        rctx: repository context.
+
+    Returns:
+        rctx.Path for the binary.
+    """
+    binary = rctx.which(binary_name)
+    if binary == None:
+        fail(_binary_not_found_msg.format(binary_name = binary_name))
+    return binary
diff --git a/python/repositories.bzl b/python/repositories.bzl
@@ -27,6 +27,7 @@ load(
     "toolchain_aliases",
     "toolchains_repo",
 )
+load("//python/private:which.bzl", "which_with_fail")
 load(
     ":versions.bzl",
     "DEFAULT_RELEASE_BASE_URL",
@@ -123,8 +124,9 @@ def _python_repository_impl(rctx):
                 sha256 = rctx.attr.zstd_sha256,
             )
             working_directory = "zstd-{version}".format(version = rctx.attr.zstd_version)
+
             make_result = rctx.execute(
-                ["make", "--jobs=4"],
+                [which_with_fail("make", rctx), "--jobs=4"],
                 timeout = 600,
                 quiet = True,
                 working_directory = working_directory,
@@ -140,7 +142,7 @@ def _python_repository_impl(rctx):
             rctx.symlink(zstd, unzstd)
 
         exec_result = rctx.execute([
-            "tar",
+            which_with_fail("tar", rctx),
             "--extract",
             "--strip-components=2",
             "--use-compress-program={unzstd}".format(unzstd = unzstd),
@@ -179,15 +181,16 @@ def _python_repository_impl(rctx):
     if not rctx.attr.ignore_root_user_error:
         if "windows" not in rctx.os.name:
             lib_dir = "lib" if "windows" not in platform else "Lib"
-            exec_result = rctx.execute(["chmod", "-R", "ugo-w", lib_dir])
+
+            exec_result = rctx.execute([which_with_fail("chmod", rctx), "-R", "ugo-w", lib_dir])
             if exec_result.return_code != 0:
                 fail_msg = "Failed to make interpreter installation read-only. 'chmod' error msg: {}".format(
                     exec_result.stderr,
                 )
                 fail(fail_msg)
-            exec_result = rctx.execute(["touch", "{}/.test".format(lib_dir)])
+            exec_result = rctx.execute([which_with_fail("touch", rctx), "{}/.test".format(lib_dir)])
             if exec_result.return_code == 0:
-                exec_result = rctx.execute(["id", "-u"])
+                exec_result = rctx.execute([which_with_fail("id", rctx), "-u"])
                 if exec_result.return_code != 0:
                     fail("Could not determine current user ID. 'id -u' error msg: {}".format(
                         exec_result.stderr,
