Skip to content

Commit 90baa6b

Browse files
authored
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
1 parent 6c465e6 commit 90baa6b

File tree

5 files changed

+45
-30
lines changed

5 files changed

+45
-30
lines changed

python/private/local_runtime_repo.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ def _local_runtime_repo_impl(rctx):
6969
rctx.path(rctx.attr._get_local_runtime_info),
7070
],
7171
quiet = True,
72+
logger = logger,
7273
)
7374
if exec_result.return_code != 0:
7475
if on_failure == "fail":

python/private/pypi/whl_library.bzl

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def _get_xcode_location_cflags(rctx):
6060
"-isysroot {}/SDKs/MacOSX.sdk".format(xcode_root),
6161
]
6262

63-
def _get_toolchain_unix_cflags(rctx, python_interpreter):
63+
def _get_toolchain_unix_cflags(rctx, python_interpreter, logger = None):
6464
"""Gather cflags from a standalone toolchain for unix systems.
6565
6666
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):
7272
return []
7373

7474
# Only update the location when using a standalone toolchain.
75-
if not is_standalone_interpreter(rctx, python_interpreter):
75+
if not is_standalone_interpreter(rctx, python_interpreter, logger = logger):
7676
return []
7777

7878
stdout = repo_utils.execute_checked_stdout(
@@ -147,20 +147,21 @@ def _parse_optional_attrs(rctx, args, extra_pip_args = None):
147147

148148
return args
149149

150-
def _create_repository_execution_environment(rctx, python_interpreter):
150+
def _create_repository_execution_environment(rctx, python_interpreter, logger = None):
151151
"""Create a environment dictionary for processes we spawn with rctx.execute.
152152
153153
Args:
154154
rctx (repository_ctx): The repository context.
155155
python_interpreter (path): The resolved python interpreter.
156+
logger: Optional logger to use for operations.
156157
Returns:
157158
Dictionary of environment variable suitable to pass to rctx.execute.
158159
"""
159160

160161
# Gather any available CPPFLAGS values
161162
cppflags = []
162163
cppflags.extend(_get_xcode_location_cflags(rctx))
163-
cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter))
164+
cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter, logger = logger))
164165

165166
env = {
166167
"PYTHONPATH": pypi_repo_utils.construct_pythonpath(
@@ -173,6 +174,7 @@ def _create_repository_execution_environment(rctx, python_interpreter):
173174
return env
174175

175176
def _whl_library_impl(rctx):
177+
logger = repo_utils.logger(rctx)
176178
python_interpreter = pypi_repo_utils.resolve_python_interpreter(
177179
rctx,
178180
python_interpreter = rctx.attr.python_interpreter,
@@ -189,7 +191,7 @@ def _whl_library_impl(rctx):
189191
extra_pip_args.extend(rctx.attr.extra_pip_args)
190192

191193
# Manually construct the PYTHONPATH since we cannot use the toolchain here
192-
environment = _create_repository_execution_environment(rctx, python_interpreter)
194+
environment = _create_repository_execution_environment(rctx, python_interpreter, logger = logger)
193195

194196
whl_path = None
195197
if rctx.attr.whl_file:
@@ -245,6 +247,7 @@ def _whl_library_impl(rctx):
245247
environment = environment,
246248
quiet = rctx.attr.quiet,
247249
timeout = rctx.attr.timeout,
250+
logger = logger,
248251
)
249252

250253
whl_path = rctx.path(json.decode(rctx.read("whl_file.json"))["whl_file"])
@@ -292,6 +295,7 @@ def _whl_library_impl(rctx):
292295
environment = environment,
293296
quiet = rctx.attr.quiet,
294297
timeout = rctx.attr.timeout,
298+
logger = logger,
295299
)
296300

297301
metadata = json.decode(rctx.read("metadata.json"))
@@ -440,6 +444,7 @@ attr makes `extra_pip_args` and `download_only` ignored.""",
440444
for repo in all_repo_names
441445
],
442446
),
447+
"_rule_name": attr.string(default = "whl_library"),
443448
}, **ATTRS)
444449
whl_library_attrs.update(AUTH_ATTRS)
445450

python/private/repo_utils.bzl

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,6 @@ def _is_repo_debug_enabled(rctx):
3131
"""
3232
return _getenv(rctx, REPO_DEBUG_ENV_VAR) == "1"
3333

34-
def _debug_print(rctx, message_cb):
35-
"""Prints a message if repo debugging is enabled.
36-
37-
Args:
38-
rctx: repository_ctx
39-
message_cb: Callable that returns the string to print. Takes
40-
no arguments.
41-
"""
42-
if _is_repo_debug_enabled(rctx):
43-
print(message_cb()) # buildifier: disable=print
44-
4534
def _logger(ctx, name = None):
4635
"""Creates a logger instance for printing messages.
4736
@@ -73,7 +62,7 @@ def _logger(ctx, name = None):
7362
elif not name:
7463
fail("The name has to be specified when using the logger with `module_ctx`")
7564

76-
def _log(enabled_on_verbosity, level, message_cb_or_str):
65+
def _log(enabled_on_verbosity, level, message_cb_or_str, printer = print):
7766
if verbosity < enabled_on_verbosity:
7867
return
7968

@@ -82,7 +71,8 @@ def _logger(ctx, name = None):
8271
else:
8372
message = message_cb_or_str()
8473

85-
print("\nrules_python:{} {}:".format(
74+
# NOTE: printer may be the `fail` function.
75+
printer("\nrules_python:{} {}:".format(
8676
name,
8777
level.upper(),
8878
), message) # buildifier: disable=print
@@ -92,6 +82,7 @@ def _logger(ctx, name = None):
9282
debug = lambda message_cb: _log(2, "DEBUG", message_cb),
9383
info = lambda message_cb: _log(1, "INFO", message_cb),
9484
warn = lambda message_cb: _log(0, "WARNING", message_cb),
85+
fail = lambda message_cb: _log(-1, "FAIL", message_cb, fail),
9586
)
9687

9788
def _execute_internal(
@@ -101,6 +92,7 @@ def _execute_internal(
10192
fail_on_error = False,
10293
arguments,
10394
environment = {},
95+
logger = None,
10496
**kwargs):
10597
"""Execute a subprocess with debugging instrumentation.
10698
@@ -114,12 +106,15 @@ def _execute_internal(
114106
arguments: list of arguments; see rctx.execute#arguments.
115107
environment: optional dict of the environment to run the command
116108
in; see rctx.execute#environment.
109+
logger: optional `Logger` to use for logging execution details. If
110+
not specified, a default will be created.
117111
**kwargs: additional kwargs to pass onto rctx.execute
118112
119113
Returns:
120114
exec_result object, see repository_ctx.execute return type.
121115
"""
122-
_debug_print(rctx, lambda: (
116+
logger = logger or _logger(rctx)
117+
logger.debug(lambda: (
123118
"repo.execute: {op}: start\n" +
124119
" command: {cmd}\n" +
125120
" working dir: {cwd}\n" +
@@ -137,7 +132,7 @@ def _execute_internal(
137132
result = rctx.execute(arguments, environment = environment, **kwargs)
138133

139134
if fail_on_error and result.return_code != 0:
140-
fail((
135+
logger.fail((
141136
"repo.execute: {op}: end: failure:\n" +
142137
" command: {cmd}\n" +
143138
" return code: {return_code}\n" +
@@ -155,8 +150,7 @@ def _execute_internal(
155150
output = _outputs_to_str(result),
156151
))
157152
elif _is_repo_debug_enabled(rctx):
158-
# buildifier: disable=print
159-
print((
153+
logger.debug((
160154
"repo.execute: {op}: end: {status}\n" +
161155
" return code: {return_code}\n" +
162156
"{output}"
@@ -413,7 +407,6 @@ def _watch_tree(rctx, *args, **kwargs):
413407

414408
repo_utils = struct(
415409
# keep sorted
416-
debug_print = _debug_print,
417410
execute_checked = _execute_checked,
418411
execute_checked_stdout = _execute_checked_stdout,
419412
execute_unchecked = _execute_unchecked,

python/private/toolchains_repo.bzl

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,10 @@ toolchains_repo = repository_rule(
120120
)
121121

122122
def _toolchain_aliases_impl(rctx):
123-
(os_name, arch) = get_host_os_arch(rctx)
123+
logger = repo_utils.logger(rctx)
124+
(os_name, arch) = _get_host_os_arch(rctx, logger)
124125

125-
host_platform = get_host_platform(os_name, arch)
126+
host_platform = _get_host_platform(os_name, arch)
126127

127128
is_windows = (os_name == WINDOWS_NAME)
128129
python3_binary_path = "python.exe" if is_windows else "bin/python3"
@@ -236,14 +237,15 @@ actions.""",
236237
)
237238

238239
def _host_toolchain_impl(rctx):
240+
logger = repo_utils.logger(rctx)
239241
rctx.file("BUILD.bazel", """\
240242
# Generated by python/private/toolchains_repo.bzl
241243
242244
exports_files(["python"], visibility = ["//visibility:public"])
243245
""")
244246

245-
(os_name, arch) = get_host_os_arch(rctx)
246-
host_platform = get_host_platform(os_name, arch)
247+
(os_name, arch) = _get_host_os_arch(rctx, logger)
248+
host_platform = _get_host_platform(os_name, arch)
247249
repo = "@@{py_repository}_{host_platform}".format(
248250
py_repository = rctx.attr.name[:-len("_host")],
249251
host_platform = host_platform,
@@ -320,6 +322,7 @@ this repo causes an eager fetch of the toolchain for the host platform.
320322
mandatory = True,
321323
doc = "The base name for all created repositories, like 'python38'.",
322324
),
325+
"_rule_name": attr.string(default = "host_toolchain"),
323326
"_rules_python_workspace": attr.label(default = Label("//:WORKSPACE")),
324327
},
325328
)
@@ -384,7 +387,7 @@ multi_toolchain_aliases = repository_rule(
384387
def sanitize_platform_name(platform):
385388
return platform.replace("-", "_")
386389

387-
def get_host_platform(os_name, arch):
390+
def _get_host_platform(os_name, arch):
388391
"""Gets the host platform.
389392
390393
Args:
@@ -401,11 +404,13 @@ def get_host_platform(os_name, arch):
401404
fail("No platform declared for host OS {} on arch {}".format(os_name, arch))
402405
return host_platform
403406

404-
def get_host_os_arch(rctx):
407+
def _get_host_os_arch(rctx, logger):
405408
"""Infer the host OS name and arch from a repository context.
406409
407410
Args:
408411
rctx: Bazel's repository_ctx.
412+
logger: Logger to use for operations.
413+
409414
Returns:
410415
A tuple with the host OS name and arch.
411416
"""
@@ -423,6 +428,7 @@ def get_host_os_arch(rctx):
423428
rctx,
424429
op = "GetUname",
425430
arguments = [repo_utils.which_checked(rctx, "uname"), "-m"],
431+
logger = logger,
426432
).stdout.strip()
427433

428434
# Normalize the os_name.

python/repositories.bzl

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,13 @@ def py_repositories():
7676

7777
STANDALONE_INTERPRETER_FILENAME = "STANDALONE_INTERPRETER"
7878

79-
def is_standalone_interpreter(rctx, python_interpreter_path):
79+
def is_standalone_interpreter(rctx, python_interpreter_path, *, logger = None):
8080
"""Query a python interpreter target for whether or not it's a rules_rust provided toolchain
8181
8282
Args:
8383
rctx (repository_ctx): The repository rule's context object.
8484
python_interpreter_path (path): A path representing the interpreter.
85+
logger: Optional logger to use for operations.
8586
8687
Returns:
8788
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):
102103
STANDALONE_INTERPRETER_FILENAME,
103104
),
104105
],
106+
logger = logger,
105107
).return_code == 0
106108

107109
def _python_repository_impl(rctx):
@@ -110,6 +112,8 @@ def _python_repository_impl(rctx):
110112
if bool(rctx.attr.url) == bool(rctx.attr.urls):
111113
fail("Exactly one of (url, urls) must be set.")
112114

115+
logger = repo_utils.logger(rctx)
116+
113117
platform = rctx.attr.platform
114118
python_version = rctx.attr.python_version
115119
python_version_info = python_version.split(".")
@@ -145,6 +149,7 @@ def _python_repository_impl(rctx):
145149
timeout = 600,
146150
quiet = True,
147151
working_directory = working_directory,
152+
logger = logger,
148153
)
149154
zstd = "{working_directory}/zstd".format(working_directory = working_directory)
150155
unzstd = "./unzstd"
@@ -160,6 +165,7 @@ def _python_repository_impl(rctx):
160165
"--use-compress-program={unzstd}".format(unzstd = unzstd),
161166
"--file={}".format(release_filename),
162167
],
168+
logger = logger,
163169
)
164170
else:
165171
rctx.download_and_extract(
@@ -197,11 +203,13 @@ def _python_repository_impl(rctx):
197203
rctx,
198204
op = "python_repository.MakeReadOnly",
199205
arguments = [repo_utils.which_checked(rctx, "chmod"), "-R", "ugo-w", lib_dir],
206+
logger = logger,
200207
)
201208
exec_result = repo_utils.execute_unchecked(
202209
rctx,
203210
op = "python_repository.TestReadOnly",
204211
arguments = [repo_utils.which_checked(rctx, "touch"), "{}/.test".format(lib_dir)],
212+
logger = logger,
205213
)
206214

207215
# The issue with running as root is the installation is no longer
@@ -211,6 +219,7 @@ def _python_repository_impl(rctx):
211219
rctx,
212220
op = "python_repository.GetUserId",
213221
arguments = [repo_utils.which_checked(rctx, "id"), "-u"],
222+
logger = logger,
214223
)
215224
uid = int(stdout.strip())
216225
if uid == 0:
@@ -529,6 +538,7 @@ For more information see the official bazel docs
529538
"zstd_version": attr.string(
530539
default = "1.5.2",
531540
),
541+
"_rule_name": attr.string(default = "python_repository"),
532542
},
533543
environ = [REPO_DEBUG_ENV_VAR],
534544
)

0 commit comments

Comments
 (0)