refactor: use logger in repo_utils.execute_* functions (bazelbuild#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

python/private/local_runtime_repo.bzl

@@ -69,6 +69,7 @@ def _local_runtime_repo_impl(rctx):
             rctx.path(rctx.attr._get_local_runtime_info),
         ],
         quiet = True,
+        logger = logger,
     )
     if exec_result.return_code != 0:
         if on_failure == "fail":

python/private/pypi/whl_library.bzl

@@ -60,7 +60,7 @@ def _get_xcode_location_cflags(rctx):
         "-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 @@ def _get_toolchain_unix_cflags(rctx, python_interpreter):
         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,20 +147,21 @@ def _parse_optional_attrs(rctx, args, extra_pip_args = None):
 
     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.
     """
 
     # 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 @@ def _create_repository_execution_environment(rctx, python_interpreter):
     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 @@ def _whl_library_impl(rctx):
     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 @@ def _whl_library_impl(rctx):
             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 @@ def _whl_library_impl(rctx):
         environment = environment,
         quiet = rctx.attr.quiet,
         timeout = rctx.attr.timeout,
+        logger = logger,
     )
 
     metadata = json.decode(rctx.read("metadata.json"))
@@ -440,6 +444,7 @@ attr makes `extra_pip_args` and `download_only` ignored.""",
             for repo in all_repo_names
         ],
     ),
+    "_rule_name": attr.string(default = "whl_library"),
 }, **ATTRS)
 whl_library_attrs.update(AUTH_ATTRS)
 

python/private/repo_utils.bzl

@@ -31,17 +31,6 @@ def _is_repo_debug_enabled(rctx):
     """
     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 @@ def _logger(ctx, name = None):
     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 @@ def _logger(ctx, name = None):
         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 @@ def _logger(ctx, name = None):
         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 @@ def _execute_internal(
         fail_on_error = False,
         arguments,
         environment = {},
+        logger = None,
         **kwargs):
     """Execute a subprocess with debugging instrumentation.
 
@@ -114,12 +106,15 @@ def _execute_internal(
         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 @@ def _execute_internal(
     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 @@ def _execute_internal(
             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 @@ def _watch_tree(rctx, *args, **kwargs):
 
 repo_utils = struct(
     # keep sorted
-    debug_print = _debug_print,
     execute_checked = _execute_checked,
     execute_checked_stdout = _execute_checked_stdout,
     execute_unchecked = _execute_unchecked,

python/private/toolchains_repo.bzl

@@ -120,9 +120,10 @@ toolchains_repo = repository_rule(
 )
 
 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 @@ actions.""",
 )
 
 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 @@ this repo causes an eager fetch of the toolchain for the host platform.
             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 @@ multi_toolchain_aliases = repository_rule(
 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 @@ def get_host_platform(os_name, arch):
         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 @@ def get_host_os_arch(rctx):
             rctx,
             op = "GetUname",
             arguments = [repo_utils.which_checked(rctx, "uname"), "-m"],
+            logger = logger,
         ).stdout.strip()
 
         # Normalize the os_name.

python/repositories.bzl

@@ -76,12 +76,13 @@ def py_repositories():
 
 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 @@ def is_standalone_interpreter(rctx, python_interpreter_path):
                 STANDALONE_INTERPRETER_FILENAME,
             ),
         ],
+        logger = logger,
     ).return_code == 0
 
 def _python_repository_impl(rctx):
@@ -110,6 +112,8 @@ def _python_repository_impl(rctx):
     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 @@ def _python_repository_impl(rctx):
                 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 @@ def _python_repository_impl(rctx):
                 "--use-compress-program={unzstd}".format(unzstd = unzstd),
                 "--file={}".format(release_filename),
             ],
+            logger = logger,
         )
     else:
         rctx.download_and_extract(
@@ -197,11 +203,13 @@ def _python_repository_impl(rctx):
                 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 @@ def _python_repository_impl(rctx):
                     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 @@ For more information see the official bazel docs
         "zstd_version": attr.string(
             default = "1.5.2",
         ),
+        "_rule_name": attr.string(default = "python_repository"),
     },
     environ = [REPO_DEBUG_ENV_VAR],
 )