From b7ddc5a0091bcd4d070fcd91027d7099338e84b9 Mon Sep 17 00:00:00 2001 From: Bruce Dawson Date: Thu, 14 Jan 2021 19:40:04 +0000 Subject: [PATCH] Check whether goma is running when it is enabled One of the mistakes one can make when running ninja is having goma enabled (use_goma=true in args.gn) but not having goma running. This can lead to ~1,000 failed compile steps, which is messy. This change teaches autoninja.py to check whether goma is running. If not then it tells autoninja to just print a warning message. The check costs roughly 30 ms which seems reasonable. In fact, because this change also switches away from vpython (necessary to use python3 to use subprocess.run) it actually runs about 600 ms _faster_ than before this change. If build acceleration is requested through use_rbe then no checking for whether the service is running is done. That could be added in the future. autoninja.py could auto-start goma but that is error prone and has limited additional value. This was tested on Linux, OSX, and Windows. Bug: 868590, b/174673874 Change-Id: Ie773e574878471e5136b9b82d52f86af3d848318 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2627014 Commit-Queue: Bruce Dawson Reviewed-by: Yoshisato Yanagisawa --- autoninja | 4 ++-- autoninja.bat | 7 ++++--- autoninja.py | 24 +++++++++++++++--------- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/autoninja b/autoninja index a1c7f669b2..815ae8a81b 100755 --- a/autoninja +++ b/autoninja @@ -14,13 +14,13 @@ fi # Execute whatever is printed by autoninja.py. # Also print it to reassure that the right settings are being used. -command=$(vpython "$(dirname -- "$0")/autoninja.py" "$@") +command=$(python3 "$(dirname -- "$0")/autoninja.py" "$@") if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then echo "$command" fi if eval "$command"; then if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then - vpython "$(dirname -- "$0")/post_build_ninja_summary.py" "$@" + python "$(dirname -- "$0")/post_build_ninja_summary.py" "$@" fi # Collect ninjalog from googler. diff --git a/autoninja.bat b/autoninja.bat index 3b9c62aa9d..e4d296564b 100755 --- a/autoninja.bat +++ b/autoninja.bat @@ -33,11 +33,12 @@ IF NOT "%1"=="" ( REM Execute whatever is printed by autoninja.py. REM Also print it to reassure that the right settings are being used. -FOR /f "usebackq tokens=*" %%a in (`vpython %scriptdir%autoninja.py "%*"`) do echo %%a & %%a +REM Don't use vpython - it is too slow to start. +FOR /f "usebackq tokens=*" %%a in (`python3.bat %scriptdir%autoninja.py "%*"`) do echo %%a & %%a @if errorlevel 1 goto buildfailure -REM Use call to invoke vpython script here, because we use vpython via vpython.bat. -@if "%NINJA_SUMMARIZE_BUILD%" == "1" call vpython.bat %scriptdir%post_build_ninja_summary.py %* +REM Use call to invoke python script here, because we use python via python.bat. +@if "%NINJA_SUMMARIZE_BUILD%" == "1" call python.bat %scriptdir%post_build_ninja_summary.py %* @call python.bat %scriptdir%ninjalog_uploader_wrapper.py --cmdline %* exit /b diff --git a/autoninja.py b/autoninja.py index aef7f0f616..f10b152874 100755 --- a/autoninja.py +++ b/autoninja.py @@ -11,18 +11,12 @@ makes using remote build acceleration simpler and safer, and avoids errors that can cause slow goma builds or swap-storms on unaccelerated builds. """ -# [VPYTHON:BEGIN] -# wheel: < -# name: "infra/python/wheels/psutil/${vpython_platform}" -# version: "version:5.6.2" -# > -# [VPYTHON:END] - from __future__ import print_function +import multiprocessing import os -import psutil import re +import subprocess import sys SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) @@ -68,6 +62,7 @@ for index, arg in enumerate(input_args[1:]): input_args = [ arg for arg in input_args if arg != '-o' and arg != '--offline'] use_remote_build = False +remote_build_is_goma = False # Attempt to auto-detect remote build acceleration. We support gn-based # builds, where we look for args.gn in the build tree, and cmake-based builds @@ -86,6 +81,9 @@ if os.path.exists(os.path.join(output_dir, 'args.gn')): if re.search(r'(^|\s)(use_goma|use_rbe)\s*=\s*true($|\s)', line_without_comment): use_remote_build = True + # Distinguish between rbe and goma + if line_without_comment.count('use_goma') > 0: + remote_build_is_goma = True continue else: for relative_path in [ @@ -111,6 +109,14 @@ goma_disabled_env = os.environ.get('GOMA_DISABLED', '0').lower() if offline or goma_disabled_env in ['true', 't', 'yes', 'y', '1']: use_remote_build = False +if use_remote_build and remote_build_is_goma: + # Check to make sure that goma is running. If not, don't start the build. + gomacc_path = os.path.join(sys.path[0], '.cipd_bin', 'gomacc') + status = subprocess.run([gomacc_path, 'port'], capture_output=True).returncode + if status == 1: + print('echo Goma is not running. Use "goma_ctl start" to start it.') + sys.exit(0) + # Specify ninja.exe on Windows so that ninja.bat can call autoninja and not # be called back. ninja_exe = 'ninja.exe' if sys.platform.startswith('win') else 'ninja' @@ -130,7 +136,7 @@ if (sys.platform.startswith('linux') # or fail to execute ninja if depot_tools is not in PATH. args = prefix_args + [ninja_exe_path] + input_args[1:] -num_cores = psutil.cpu_count() +num_cores = multiprocessing.cpu_count() if not j_specified and not t_specified: if use_remote_build: args.append('-j')