From 428143ee24c5f084c8dfb38cd17f07b4f7ba9bf7 Mon Sep 17 00:00:00 2001 From: Yoshisato Yanagisawa Date: Fri, 15 Jan 2021 10:24:48 +0000 Subject: [PATCH] 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 --- autoninja | 4 ++-- autoninja.bat | 7 +++---- autoninja.py | 24 +++++++++--------------- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/autoninja b/autoninja index 815ae8a81..a1c7f669b 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=$(python3 "$(dirname -- "$0")/autoninja.py" "$@") +command=$(vpython "$(dirname -- "$0")/autoninja.py" "$@") if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then echo "$command" fi if eval "$command"; then if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then - python "$(dirname -- "$0")/post_build_ninja_summary.py" "$@" + vpython "$(dirname -- "$0")/post_build_ninja_summary.py" "$@" fi # Collect ninjalog from googler. diff --git a/autoninja.bat b/autoninja.bat index e4d296564..3b9c62aa9 100755 --- a/autoninja.bat +++ b/autoninja.bat @@ -33,12 +33,11 @@ IF NOT "%1"=="" ( REM Execute whatever is printed by autoninja.py. REM Also print it to reassure that the right settings are being used. -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 +FOR /f "usebackq tokens=*" %%a in (`vpython %scriptdir%autoninja.py "%*"`) do echo %%a & %%a @if errorlevel 1 goto buildfailure -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 %* +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 %* @call python.bat %scriptdir%ninjalog_uploader_wrapper.py --cmdline %* exit /b diff --git a/autoninja.py b/autoninja.py index f10b15287..aef7f0f61 100755 --- a/autoninja.py +++ b/autoninja.py @@ -11,12 +11,18 @@ 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__)) @@ -62,7 +68,6 @@ 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 @@ -81,9 +86,6 @@ 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 [ @@ -109,14 +111,6 @@ 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' @@ -136,7 +130,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 = multiprocessing.cpu_count() +num_cores = psutil.cpu_count() if not j_specified and not t_specified: if use_remote_build: args.append('-j')