Commit 428143ee authored by Yoshisato Yanagisawa's avatar Yoshisato Yanagisawa Committed by LUCI CQ

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/+/2632018Reviewed-by: 's avatarYoshisato Yanagisawa <yyanagisawa@google.com>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@google.com>
parent f79e4320
...@@ -14,13 +14,13 @@ fi ...@@ -14,13 +14,13 @@ fi
# Execute whatever is printed by autoninja.py. # Execute whatever is printed by autoninja.py.
# Also print it to reassure that the right settings are being used. # 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 if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then
echo "$command" echo "$command"
fi fi
if eval "$command"; then if eval "$command"; then
if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then
python "$(dirname -- "$0")/post_build_ninja_summary.py" "$@" vpython "$(dirname -- "$0")/post_build_ninja_summary.py" "$@"
fi fi
# Collect ninjalog from googler. # Collect ninjalog from googler.
......
...@@ -33,12 +33,11 @@ IF NOT "%1"=="" ( ...@@ -33,12 +33,11 @@ IF NOT "%1"=="" (
REM Execute whatever is printed by autoninja.py. REM Execute whatever is printed by autoninja.py.
REM Also print it to reassure that the right settings are being used. 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 (`vpython %scriptdir%autoninja.py "%*"`) do echo %%a & %%a
FOR /f "usebackq tokens=*" %%a in (`python3.bat %scriptdir%autoninja.py "%*"`) do echo %%a & %%a
@if errorlevel 1 goto buildfailure @if errorlevel 1 goto buildfailure
REM Use call to invoke python script here, because we use python via python.bat. REM Use call to invoke vpython script here, because we use vpython via vpython.bat.
@if "%NINJA_SUMMARIZE_BUILD%" == "1" call python.bat %scriptdir%post_build_ninja_summary.py %* @if "%NINJA_SUMMARIZE_BUILD%" == "1" call vpython.bat %scriptdir%post_build_ninja_summary.py %*
@call python.bat %scriptdir%ninjalog_uploader_wrapper.py --cmdline %* @call python.bat %scriptdir%ninjalog_uploader_wrapper.py --cmdline %*
exit /b exit /b
......
...@@ -11,12 +11,18 @@ makes using remote build acceleration simpler and safer, and avoids errors that ...@@ -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. 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 from __future__ import print_function
import multiprocessing
import os import os
import psutil
import re import re
import subprocess
import sys import sys
SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
...@@ -62,7 +68,6 @@ for index, arg in enumerate(input_args[1:]): ...@@ -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'] input_args = [ arg for arg in input_args if arg != '-o' and arg != '--offline']
use_remote_build = False use_remote_build = False
remote_build_is_goma = False
# Attempt to auto-detect remote build acceleration. We support gn-based # 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 # 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')): ...@@ -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)', if re.search(r'(^|\s)(use_goma|use_rbe)\s*=\s*true($|\s)',
line_without_comment): line_without_comment):
use_remote_build = True use_remote_build = True
# Distinguish between rbe and goma
if line_without_comment.count('use_goma') > 0:
remote_build_is_goma = True
continue continue
else: else:
for relative_path in [ for relative_path in [
...@@ -109,14 +111,6 @@ goma_disabled_env = os.environ.get('GOMA_DISABLED', '0').lower() ...@@ -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']: if offline or goma_disabled_env in ['true', 't', 'yes', 'y', '1']:
use_remote_build = False 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 # Specify ninja.exe on Windows so that ninja.bat can call autoninja and not
# be called back. # be called back.
ninja_exe = 'ninja.exe' if sys.platform.startswith('win') else 'ninja' ninja_exe = 'ninja.exe' if sys.platform.startswith('win') else 'ninja'
...@@ -136,7 +130,7 @@ if (sys.platform.startswith('linux') ...@@ -136,7 +130,7 @@ if (sys.platform.startswith('linux')
# or fail to execute ninja if depot_tools is not in PATH. # or fail to execute ninja if depot_tools is not in PATH.
args = prefix_args + [ninja_exe_path] + input_args[1:] 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 not j_specified and not t_specified:
if use_remote_build: if use_remote_build:
args.append('-j') args.append('-j')
......
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