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`:
  77cb39e0c7/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:
77cb39e0c7/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 <jwata@google.com>
Commit-Queue: Adam Norberg <norberg@google.com>
This commit is contained in:
Adam Norberg
2025-11-20 21:03:12 -08:00
committed by LUCI CQ
parent 375fb27dca
commit 3ec220caca
3 changed files with 15 additions and 11 deletions

View File

@@ -2,6 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
import os
import subprocess import subprocess
import sys import sys
@@ -12,13 +13,16 @@ caffeinate:
{_NO_CAFFEINATE_FLAG} do not prepend `caffeinate` to ninja command {_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 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) print(_HELP_MESSAGE, file=sys.stderr)
if _NO_CAFFEINATE_FLAG in cmd: if _NO_CAFFEINATE_FLAG in args:
cmd.remove(_NO_CAFFEINATE_FLAG) args.remove(_NO_CAFFEINATE_FLAG)
else: else:
cmd = ['caffeinate'] + cmd args = ['caffeinate'] + args
return subprocess.call(cmd, env=env) return subprocess.call(args, **call_kwargs)

View File

@@ -77,7 +77,7 @@ def fallback(tool, out_dir, ninja_args):
ninja_path = find_ninja_in_path() ninja_path = find_ninja_in_path()
if ninja_path: if ninja_path:
check_out_dir(tool, out_dir) check_out_dir(tool, out_dir)
return caffeinate.run([ninja_path] + ninja_args) return caffeinate.call([ninja_path] + ninja_args)
print( print(
"depot_tools/ninja.py: Could not find Ninja in the third_party of " "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): if os.path.isfile(ninja_path):
check_out_dir(tool, out_dir) 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:]) return fallback(tool, out_dir, args[1:])

View File

@@ -311,14 +311,14 @@ def main(args, telemetry_cfg: Optional[build_telemetry.Config] = None):
for siso_path in siso_paths: for siso_path in siso_paths:
if siso_path and os.path.isfile(siso_path): if siso_path and os.path.isfile(siso_path):
check_outdir(subcmd, out_dir) check_outdir(subcmd, out_dir)
return caffeinate.run([siso_path] + processed_args, env=env) return caffeinate.call([siso_path] + processed_args, env=env)
print( print(
'depot_tools/siso.py: Could not find siso in third_party/siso ' 'depot_tools/siso.py: Could not find siso in third_party/siso '
'of the current project. Did you run gclient sync?', 'of the current project. Did you run gclient sync?',
file=sys.stderr) file=sys.stderr)
return 1 return 1
if siso_override_path: 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( print(
'depot_tools/siso.py: Could not find .sisoenv under build/config/siso ' 'depot_tools/siso.py: Could not find .sisoenv under build/config/siso '