refactor: use logger in repo_utils.execute_* functions (#2082)
This is to unify how it handles printing log messages.
This also updates repo rules using repo_utils to create loggers and set
the `_rule_name` attribute for the logger to use.
* Also removes defunct repo_utils.debug_print
* Also makes some functions private that aren't used elsewhere
diff --git a/python/private/local_runtime_repo.bzl b/python/private/local_runtime_repo.bzl
index a206e6d..4e7edde 100644
--- a/python/private/local_runtime_repo.bzl
+++ b/python/private/local_runtime_repo.bzl
@@ -69,6 +69,7 @@
rctx.path(rctx.attr._get_local_runtime_info),
],
quiet = True,
+ logger = logger,
)
if exec_result.return_code != 0:
if on_failure == "fail":
diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl
index 0419926..c4e1f76 100644
--- a/python/private/pypi/whl_library.bzl
+++ b/python/private/pypi/whl_library.bzl
@@ -60,7 +60,7 @@
"-isysroot {}/SDKs/MacOSX.sdk".format(xcode_root),
]
-def _get_toolchain_unix_cflags(rctx, python_interpreter):
+def _get_toolchain_unix_cflags(rctx, python_interpreter, logger = None):
"""Gather cflags from a standalone toolchain for unix systems.
Pip won't be able to compile c extensions from sdists with the pre built python distributions from indygreg
@@ -72,7 +72,7 @@
return []
# Only update the location when using a standalone toolchain.
- if not is_standalone_interpreter(rctx, python_interpreter):
+ if not is_standalone_interpreter(rctx, python_interpreter, logger = logger):
return []
stdout = repo_utils.execute_checked_stdout(
@@ -147,12 +147,13 @@
return args
-def _create_repository_execution_environment(rctx, python_interpreter):
+def _create_repository_execution_environment(rctx, python_interpreter, logger = None):
"""Create a environment dictionary for processes we spawn with rctx.execute.
Args:
rctx (repository_ctx): The repository context.
python_interpreter (path): The resolved python interpreter.
+ logger: Optional logger to use for operations.
Returns:
Dictionary of environment variable suitable to pass to rctx.execute.
"""
@@ -160,7 +161,7 @@
# Gather any available CPPFLAGS values
cppflags = []
cppflags.extend(_get_xcode_location_cflags(rctx))
- cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter))
+ cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter, logger = logger))
env = {
"PYTHONPATH": pypi_repo_utils.construct_pythonpath(
@@ -173,6 +174,7 @@
return env
def _whl_library_impl(rctx):
+ logger = repo_utils.logger(rctx)
python_interpreter = pypi_repo_utils.resolve_python_interpreter(
rctx,
python_interpreter = rctx.attr.python_interpreter,
@@ -189,7 +191,7 @@
extra_pip_args.extend(rctx.attr.extra_pip_args)
# Manually construct the PYTHONPATH since we cannot use the toolchain here
- environment = _create_repository_execution_environment(rctx, python_interpreter)
+ environment = _create_repository_execution_environment(rctx, python_interpreter, logger = logger)
whl_path = None
if rctx.attr.whl_file:
@@ -245,6 +247,7 @@
environment = environment,
quiet = rctx.attr.quiet,
timeout = rctx.attr.timeout,
+ logger = logger,
)
whl_path = rctx.path(json.decode(rctx.read("whl_file.json"))["whl_file"])
@@ -292,6 +295,7 @@
environment = environment,
quiet = rctx.attr.quiet,
timeout = rctx.attr.timeout,
+ logger = logger,
)
metadata = json.decode(rctx.read("metadata.json"))
@@ -440,6 +444,7 @@
for repo in all_repo_names
],
),
+ "_rule_name": attr.string(default = "whl_library"),
}, **ATTRS)
whl_library_attrs.update(AUTH_ATTRS)
diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl
index 3c07027..1c50ac6 100644
--- a/python/private/repo_utils.bzl
+++ b/python/private/repo_utils.bzl
@@ -31,17 +31,6 @@
"""
return _getenv(rctx, REPO_DEBUG_ENV_VAR) == "1"
-def _debug_print(rctx, message_cb):
- """Prints a message if repo debugging is enabled.
-
- Args:
- rctx: repository_ctx
- message_cb: Callable that returns the string to print. Takes
- no arguments.
- """
- if _is_repo_debug_enabled(rctx):
- print(message_cb()) # buildifier: disable=print
-
def _logger(ctx, name = None):
"""Creates a logger instance for printing messages.
@@ -73,7 +62,7 @@
elif not name:
fail("The name has to be specified when using the logger with `module_ctx`")
- def _log(enabled_on_verbosity, level, message_cb_or_str):
+ def _log(enabled_on_verbosity, level, message_cb_or_str, printer = print):
if verbosity < enabled_on_verbosity:
return
@@ -82,7 +71,8 @@
else:
message = message_cb_or_str()
- print("\nrules_python:{} {}:".format(
+ # NOTE: printer may be the `fail` function.
+ printer("\nrules_python:{} {}:".format(
name,
level.upper(),
), message) # buildifier: disable=print
@@ -92,6 +82,7 @@
debug = lambda message_cb: _log(2, "DEBUG", message_cb),
info = lambda message_cb: _log(1, "INFO", message_cb),
warn = lambda message_cb: _log(0, "WARNING", message_cb),
+ fail = lambda message_cb: _log(-1, "FAIL", message_cb, fail),
)
def _execute_internal(
@@ -101,6 +92,7 @@
fail_on_error = False,
arguments,
environment = {},
+ logger = None,
**kwargs):
"""Execute a subprocess with debugging instrumentation.
@@ -114,12 +106,15 @@
arguments: list of arguments; see rctx.execute#arguments.
environment: optional dict of the environment to run the command
in; see rctx.execute#environment.
+ logger: optional `Logger` to use for logging execution details. If
+ not specified, a default will be created.
**kwargs: additional kwargs to pass onto rctx.execute
Returns:
exec_result object, see repository_ctx.execute return type.
"""
- _debug_print(rctx, lambda: (
+ logger = logger or _logger(rctx)
+ logger.debug(lambda: (
"repo.execute: {op}: start\n" +
" command: {cmd}\n" +
" working dir: {cwd}\n" +
@@ -137,7 +132,7 @@
result = rctx.execute(arguments, environment = environment, **kwargs)
if fail_on_error and result.return_code != 0:
- fail((
+ logger.fail((
"repo.execute: {op}: end: failure:\n" +
" command: {cmd}\n" +
" return code: {return_code}\n" +
@@ -155,8 +150,7 @@
output = _outputs_to_str(result),
))
elif _is_repo_debug_enabled(rctx):
- # buildifier: disable=print
- print((
+ logger.debug((
"repo.execute: {op}: end: {status}\n" +
" return code: {return_code}\n" +
"{output}"
@@ -413,7 +407,6 @@
repo_utils = struct(
# keep sorted
- debug_print = _debug_print,
execute_checked = _execute_checked,
execute_checked_stdout = _execute_checked_stdout,
execute_unchecked = _execute_unchecked,
diff --git a/python/private/toolchains_repo.bzl b/python/private/toolchains_repo.bzl
index dd59e47..df16fb8 100644
--- a/python/private/toolchains_repo.bzl
+++ b/python/private/toolchains_repo.bzl
@@ -120,9 +120,10 @@
)
def _toolchain_aliases_impl(rctx):
- (os_name, arch) = get_host_os_arch(rctx)
+ logger = repo_utils.logger(rctx)
+ (os_name, arch) = _get_host_os_arch(rctx, logger)
- host_platform = get_host_platform(os_name, arch)
+ host_platform = _get_host_platform(os_name, arch)
is_windows = (os_name == WINDOWS_NAME)
python3_binary_path = "python.exe" if is_windows else "bin/python3"
@@ -236,14 +237,15 @@
)
def _host_toolchain_impl(rctx):
+ logger = repo_utils.logger(rctx)
rctx.file("BUILD.bazel", """\
# Generated by python/private/toolchains_repo.bzl
exports_files(["python"], visibility = ["//visibility:public"])
""")
- (os_name, arch) = get_host_os_arch(rctx)
- host_platform = get_host_platform(os_name, arch)
+ (os_name, arch) = _get_host_os_arch(rctx, logger)
+ host_platform = _get_host_platform(os_name, arch)
repo = "@@{py_repository}_{host_platform}".format(
py_repository = rctx.attr.name[:-len("_host")],
host_platform = host_platform,
@@ -320,6 +322,7 @@
mandatory = True,
doc = "The base name for all created repositories, like 'python38'.",
),
+ "_rule_name": attr.string(default = "host_toolchain"),
"_rules_python_workspace": attr.label(default = Label("//:WORKSPACE")),
},
)
@@ -384,7 +387,7 @@
def sanitize_platform_name(platform):
return platform.replace("-", "_")
-def get_host_platform(os_name, arch):
+def _get_host_platform(os_name, arch):
"""Gets the host platform.
Args:
@@ -401,11 +404,13 @@
fail("No platform declared for host OS {} on arch {}".format(os_name, arch))
return host_platform
-def get_host_os_arch(rctx):
+def _get_host_os_arch(rctx, logger):
"""Infer the host OS name and arch from a repository context.
Args:
rctx: Bazel's repository_ctx.
+ logger: Logger to use for operations.
+
Returns:
A tuple with the host OS name and arch.
"""
@@ -423,6 +428,7 @@
rctx,
op = "GetUname",
arguments = [repo_utils.which_checked(rctx, "uname"), "-m"],
+ logger = logger,
).stdout.strip()
# Normalize the os_name.
diff --git a/python/repositories.bzl b/python/repositories.bzl
index d58feef..baa5f5b 100644
--- a/python/repositories.bzl
+++ b/python/repositories.bzl
@@ -76,12 +76,13 @@
STANDALONE_INTERPRETER_FILENAME = "STANDALONE_INTERPRETER"
-def is_standalone_interpreter(rctx, python_interpreter_path):
+def is_standalone_interpreter(rctx, python_interpreter_path, *, logger = None):
"""Query a python interpreter target for whether or not it's a rules_rust provided toolchain
Args:
rctx (repository_ctx): The repository rule's context object.
python_interpreter_path (path): A path representing the interpreter.
+ logger: Optional logger to use for operations.
Returns:
bool: Whether or not the target is from a rules_python generated toolchain.
@@ -102,6 +103,7 @@
STANDALONE_INTERPRETER_FILENAME,
),
],
+ logger = logger,
).return_code == 0
def _python_repository_impl(rctx):
@@ -110,6 +112,8 @@
if bool(rctx.attr.url) == bool(rctx.attr.urls):
fail("Exactly one of (url, urls) must be set.")
+ logger = repo_utils.logger(rctx)
+
platform = rctx.attr.platform
python_version = rctx.attr.python_version
python_version_info = python_version.split(".")
@@ -145,6 +149,7 @@
timeout = 600,
quiet = True,
working_directory = working_directory,
+ logger = logger,
)
zstd = "{working_directory}/zstd".format(working_directory = working_directory)
unzstd = "./unzstd"
@@ -160,6 +165,7 @@
"--use-compress-program={unzstd}".format(unzstd = unzstd),
"--file={}".format(release_filename),
],
+ logger = logger,
)
else:
rctx.download_and_extract(
@@ -197,11 +203,13 @@
rctx,
op = "python_repository.MakeReadOnly",
arguments = [repo_utils.which_checked(rctx, "chmod"), "-R", "ugo-w", lib_dir],
+ logger = logger,
)
exec_result = repo_utils.execute_unchecked(
rctx,
op = "python_repository.TestReadOnly",
arguments = [repo_utils.which_checked(rctx, "touch"), "{}/.test".format(lib_dir)],
+ logger = logger,
)
# The issue with running as root is the installation is no longer
@@ -211,6 +219,7 @@
rctx,
op = "python_repository.GetUserId",
arguments = [repo_utils.which_checked(rctx, "id"), "-u"],
+ logger = logger,
)
uid = int(stdout.strip())
if uid == 0:
@@ -529,6 +538,7 @@
"zstd_version": attr.string(
default = "1.5.2",
),
+ "_rule_name": attr.string(default = "python_repository"),
},
environ = [REPO_DEBUG_ENV_VAR],
)
