From 3ec220caca2f209f2802bf5fc1fcef7e5b069475 Mon Sep 17 00:00:00 2001 From: Adam Norberg Date: Thu, 20 Nov 2025 21:03:12 -0800 Subject: [PATCH] Rename caffeinate.run to .call and match the subprocess.call API. I'm looking at caffeinating more long-running depot_tools commands on Mac, which use a variety of `subprocess` APIs; one implementation strategy I'm considering is to wrap more of them, including `subprocess.run`, but the most obvious name to wrap `subprocess.run` into is already taken! I think it makes the most sense to name the `caffeinate` function(s) after the `subprocess` call it wraps. The callers to other parts of `subprocess` use more of the features exposed by those APIs. For consistency, I am plumbing through more of `subprocess.call`'s identifiable behaviors here, too: * command-to-invoke parameter is named "args" * as explicitly checked in cpython's subprocess.py's `_posix_spawn`: https://github.com/python/cpython/blob/77cb39e0c7ef606ef68a0e09fd7c8c9c360f97f1/Lib/subprocess.py#L1816 also allow `str`, `bytes`, and `os.PathLike` for the type of `args`, converting the non-list-like types into a single-element list * use kwargs packing and unpacking to forward other parameters into `subprocess.call` kwargs packing does not provide perfect emulation as implemented: https://github.com/python/cpython/blob/77cb39e0c7ef606ef68a0e09fd7c8c9c360f97f1/Lib/subprocess.py#L386 `timeout` could be provided as a positional parameter. However, Python documents all subprocess.call parameters as keyword only: https://docs.python.org/3.12/library/subprocess.html#older-high-level-api ...specifically because they don't want to bother distinguishing between parameters "local" to a higher-level `subprocess` helper and those that get forwarded (via `**kwargs`) to the underlying `Popen`, so I think it is reasonable to follow their lead on that. Bug: 462507017 Change-Id: Ia520ced7f8188c23c38826d22ccf20a3c52ddfc5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/7181621 Reviewed-by: Junji Watanabe Commit-Queue: Adam Norberg --- caffeinate.py | 18 +++++++++++------- ninja.py | 4 ++-- siso.py | 4 ++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/caffeinate.py b/caffeinate.py index 732bafff55..2e309dfa6d 100644 --- a/caffeinate.py +++ b/caffeinate.py @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import os import subprocess import sys @@ -12,13 +13,16 @@ caffeinate: {_NO_CAFFEINATE_FLAG} do not prepend `caffeinate` to ninja command """ -def run(cmd, env=None): - """Runs a command with `caffeinate` if it's on macOS.""" + +def call(args, **call_kwargs): + """Runs a command (via subprocess.call) with `caffeinate` if it's on macOS.""" if sys.platform == 'darwin': - if '-h' in cmd or '--help' in cmd: + if isinstance(args, (str, bytes, os.PathLike)): + args = [args] + if '-h' in args or '--help' in args: print(_HELP_MESSAGE, file=sys.stderr) - if _NO_CAFFEINATE_FLAG in cmd: - cmd.remove(_NO_CAFFEINATE_FLAG) + if _NO_CAFFEINATE_FLAG in args: + args.remove(_NO_CAFFEINATE_FLAG) else: - cmd = ['caffeinate'] + cmd - return subprocess.call(cmd, env=env) + args = ['caffeinate'] + args + return subprocess.call(args, **call_kwargs) diff --git a/ninja.py b/ninja.py index 2f673571ec..ede7cca937 100755 --- a/ninja.py +++ b/ninja.py @@ -77,7 +77,7 @@ def fallback(tool, out_dir, ninja_args): ninja_path = find_ninja_in_path() if ninja_path: check_out_dir(tool, out_dir) - return caffeinate.run([ninja_path] + ninja_args) + return caffeinate.call([ninja_path] + ninja_args) print( "depot_tools/ninja.py: Could not find Ninja in the third_party of " @@ -130,7 +130,7 @@ def main(args): ) if os.path.isfile(ninja_path): check_out_dir(tool, out_dir) - return caffeinate.run([ninja_path] + args[1:]) + return caffeinate.call([ninja_path] + args[1:]) return fallback(tool, out_dir, args[1:]) diff --git a/siso.py b/siso.py index 5a0ed01bc3..b61d8e3202 100644 --- a/siso.py +++ b/siso.py @@ -311,14 +311,14 @@ def main(args, telemetry_cfg: Optional[build_telemetry.Config] = None): for siso_path in siso_paths: if siso_path and os.path.isfile(siso_path): check_outdir(subcmd, out_dir) - return caffeinate.run([siso_path] + processed_args, env=env) + return caffeinate.call([siso_path] + processed_args, env=env) print( 'depot_tools/siso.py: Could not find siso in third_party/siso ' 'of the current project. Did you run gclient sync?', file=sys.stderr) return 1 if siso_override_path: - return caffeinate.run([siso_override_path] + args[1:], env=env) + return caffeinate.call([siso_override_path] + args[1:], env=env) print( 'depot_tools/siso.py: Could not find .sisoenv under build/config/siso '