Commit e952faee authored by Bruce Dawson's avatar Bruce Dawson Committed by LUCI CQ

Reland "Check whether goma is running when it is enabled"

This reverts commit 428143ee.

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 2241db8a - "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 b7ddc5a0.
>
> 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 <brucedawson@chromium.org>
> > Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
>
> 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 <yyanagisawa@google.com>
> Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@google.com>

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/+/2634201Reviewed-by: 's avatarBruce Dawson <brucedawson@chromium.org>
Reviewed-by: 's avatarDirk Pranke <dpranke@google.com>
Reviewed-by: 's avatarFumitoshi Ukai <ukai@google.com>
Reviewed-by: 's avatarYoshisato Yanagisawa <yyanagisawa@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
parent 8a2e6a7a
......@@ -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
......
......@@ -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.
......
......@@ -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))
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment