From e952faee4ce22a7436b2c3bf9e9b9a45fb0014ed Mon Sep 17 00:00:00 2001 From: Bruce Dawson Date: Sat, 27 Feb 2021 23:33:37 +0000 Subject: [PATCH] Reland "Check whether goma is running when it is enabled" This reverts commit 428143ee24c5f084c8dfb38cd17f07b4f7ba9bf7. Reason for revert: Fixing the issues revealed by the original change by avoiding python3 and by checking for the existence of gomacc[.exe] before running it. This also relands 2241db8a1f6c05325c8a0c9eb5426d96cd6fa984 - "Avoid capture_output to support Python 3.6", to simplify relanding and any possible reverts. Original change's description: > Revert "Check whether goma is running when it is enabled" > > This reverts commit b7ddc5a0091bcd4d070fcd91027d7099338e84b9. > > Reason for revert: > This broke the builder where depot_tools is not in PATH. > https://logs.chromium.org/logs/infra-internal/buildbucket/cr-buildbucket.appspot.com/8858077852309878080/+/u/build/stdout > > Original change's description: > > 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 > > TBR=yyanagisawa@google.com,dpranke@google.com,brucedawson@chromium.org,sanfin@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com > > Change-Id: I57a6c73ea853259f3d1ec7ad0ce51e495acc96db > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 868590 > Bug: b/174673874 > Bug: 1167064 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2632018 > Reviewed-by: Yoshisato Yanagisawa > Commit-Queue: Yoshisato Yanagisawa TBR=yyanagisawa@google.com,dpranke@google.com,brucedawson@chromium.org,sanfin@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com # Not skipping CQ checks because this is a reland. Bug: 868590 Bug: b/174673874 Bug: 1167064 Change-Id: I8aa6830259bc18f8e7926cd0bf5c62e671c74a2d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2634201 Reviewed-by: Bruce Dawson Reviewed-by: Dirk Pranke Reviewed-by: Fumitoshi Ukai Reviewed-by: Yoshisato Yanagisawa Commit-Queue: Bruce Dawson --- autoninja | 3 ++- autoninja.bat | 20 ++++++++++++-------- autoninja.py | 32 ++++++++++++++++++++++---------- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/autoninja b/autoninja index 76fee587b..45c71fc64 100755 --- a/autoninja +++ b/autoninja @@ -14,7 +14,8 @@ 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" "$@") +# Don't use python3 because it doesn't work in git bash on Windows. +command=$(python "$(dirname -- "$0")/autoninja.py" "$@") if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then echo "$command" fi diff --git a/autoninja.bat b/autoninja.bat index b5bac5788..34b477e71 100755 --- a/autoninja.bat +++ b/autoninja.bat @@ -5,8 +5,10 @@ setlocal +set scriptdir=%~dp0 + REM Set unique build ID. -FOR /f "usebackq tokens=*" %%a in (`python3 -c "import uuid; print(uuid.uuid4())"`) do set AUTONINJA_BUILD_ID=%%a +FOR /f "usebackq tokens=*" %%a in (`python -c "import uuid; print(uuid.uuid4())"`) do set AUTONINJA_BUILD_ID=%%a REM If a build performance summary has been requested then also set NINJA_STATUS REM to trigger more verbose status updates. In particular this makes it possible @@ -14,8 +16,6 @@ REM to see how quickly process creation is happening - often a critical clue on REM Windows. The trailing space is intentional. if "%NINJA_SUMMARIZE_BUILD%" == "1" set NINJA_STATUS=[%%r processes, %%f/%%t @ %%o/s : %%es ] -set scriptdir=%~dp0 - :loop IF NOT "%1"=="" ( @rem Tell goma or reclient to not do network compiles. @@ -33,17 +33,21 @@ 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. +REM Don't use python3 because it doesn't work in git bash on Windows and we +REM should be consistent between autoninja.bat and the autoninja script used by +REM git bash. +FOR /f "usebackq tokens=*" %%a in (`python %scriptdir%autoninja.py "%*"`) do echo %%a & %%a @if errorlevel 1 goto buildfailure -REM Use call to invoke python script here, because we may use python3 via python3.bat. -@if "%NINJA_SUMMARIZE_BUILD%" == "1" call python3 %scriptdir%post_build_ninja_summary.py %* -@call python3 %scriptdir%ninjalog_uploader_wrapper.py --cmdline %* +REM Use call to invoke python script here, because we use python via python.bat. +@if "%NINJA_SUMMARIZE_BUILD%" == "1" call python %scriptdir%post_build_ninja_summary.py %* +@call python %scriptdir%ninjalog_uploader_wrapper.py --cmdline %* exit /b :buildfailure -@call python3 %scriptdir%ninjalog_uploader_wrapper.py --cmdline %* +@call python %scriptdir%ninjalog_uploader_wrapper.py --cmdline %* REM Return an error code of 1 so that if a developer types: REM "autoninja chrome && chrome" then chrome won't run if the build fails. diff --git a/autoninja.py b/autoninja.py index 53f972ccc..3afdff68c 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__)) @@ -125,6 +119,25 @@ goma_disabled_env = os.environ.get('GOMA_DISABLED', '0').lower() if offline or goma_disabled_env in ['true', 't', 'yes', 'y', '1']: use_goma = False +if use_goma: + gomacc_file = 'gomacc.exe' if sys.platform.startswith('win') else 'gomacc' + gomacc_path = os.path.join(SCRIPT_DIR, '.cipd_bin', gomacc_file) + # Don't invoke gomacc if it doesn't exist. + if os.path.exists(gomacc_path): + # Check to make sure that goma is running. If not, don't start the build. + status = subprocess.call([gomacc_path, 'port'], stdout=subprocess.PIPE, + stderr=subprocess.PIPE, shell=False) + if status == 1: + print('Goma is not running. Use "goma_ctl ensure_start" to start it.', + file=sys.stderr) + if sys.platform.startswith('win'): + # Set an exit code of 1 in the batch file. + print('cmd /c exit 1') + else: + # Set an exit code of 1 by executing 'false' in the bash script. + print('false') + sys.exit(1) + # 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' @@ -144,7 +157,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_goma or use_rbe: args.append('-j') @@ -201,4 +214,3 @@ if offline and not sys.platform.startswith('win'): print('RBE_remote_disabled=1 GOMA_DISABLED=1 ' + ' '.join(args)) else: print(' '.join(args)) -