Skip to content

Commit

Permalink
feat: Improve exec binary error handling
Browse files Browse the repository at this point in the history
At times binaries are not in the path. This commit
uses a new method to test that a binary exists before we
try to execute the binary. The method fails with a message
if the binary does not exist.
  • Loading branch information
chrislovecnm committed Aug 8, 2023
1 parent fabb65f commit 3a1ee5d
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 10 deletions.
6 changes: 2 additions & 4 deletions python/pip_install/pip_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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:util.bzl", "which_with_fail")

CPPFLAGS = "CPPFLAGS"

Expand Down Expand Up @@ -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 []

Expand Down
3 changes: 2 additions & 1 deletion python/private/toolchains_repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ load(
"PLATFORMS",
"WINDOWS_NAME",
)
load(":util.bzl", "which_with_fail")

def get_repository_name(repository_workspace):
dummy_label = "//:_"
Expand Down Expand Up @@ -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():
Expand Down
17 changes: 17 additions & 0 deletions python/private/util.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,20 @@ def add_tag(attrs, tag):
attrs["tags"] = tags + [tag]
else:
attrs["tags"] = [tag]

_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)
return binary if binary != None else fail(
_binary_not_found_msg.format(binary_name = binary_name),
)
13 changes: 8 additions & 5 deletions python/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ load(
"toolchain_aliases",
"toolchains_repo",
)
load("//python/private:util.bzl", "which_with_fail")
load(
":versions.bzl",
"DEFAULT_RELEASE_BASE_URL",
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 3a1ee5d

Please sign in to comment.