Skip to content

Commit

Permalink
[beta] Migrate from Goma to RBE.
Browse files Browse the repository at this point in the history
This change is a necessary infrastructure migration that switches
building dart for testing to RBE instead of Goma. The release
artifacts are unchanged as RBE must not be used for official
releases and only affects the commit queue and post-submit testing.

The anthology of RBE commits to main since the 3.4.0-190.1 branch point
is cherry-picked in their entirety without merge conflicts to put the
beta branch in the same supported state as main.

Bug: b/296994239
Change-Id: I0b454d9d95e7bb6f978db344ba9eadfde6e979dc
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/355761
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/355780
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/355683
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/355400
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/343440
Cherry-pick-request: #55184
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357342
Reviewed-by: William Hesse <[email protected]>
  • Loading branch information
sortie committed Mar 19, 2024
1 parent c38c85a commit ac06ea0
Show file tree
Hide file tree
Showing 13 changed files with 156 additions and 26 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ CMakeLists.txt
.clang_complete
cmake-build-debug

# Windows toolchain.
win_toolchain

# VS project files
.vs

Expand Down
7 changes: 5 additions & 2 deletions DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ vars = {
# https://chrome-infra-packages.appspot.com/p/gn/gn
"gn_version": "git_revision:a2e2717ea670249a34b0de4b3e54f268d320bdfa",

"reclient_version": "git_revision:f3883c2237b0eb9cc9524cb571b5ab8378f257e4",
"reclient_version": "git_revision:c7349324c93c6e0d85bc1e00b5d7526771006ea0",
"download_reclient": True,

# Update from https://chrome-infra-packages.appspot.com/p/fuchsia/sdk/core
"fuchsia_sdk_version": "version:18.20240208.2.1",
Expand Down Expand Up @@ -617,7 +618,9 @@ Var("dart_root") + "/third_party/pkg/tar":
}
],
# Download reclient only on the platforms where it has packages available.
'condition': '((host_os == "linux" or host_os == "mac" ) and host_cpu == "x64") or (host_os == "mac" and host_cpu == "arm64")',
# Unfortunately windows-arm64 gclient uses x64 python which lies in
# host_cpu, so we have to use a variable to not download reclient there.
'condition': 'download_reclient and (((host_os == "linux" or host_os == "mac" or host_os == "win") and host_cpu == "x64") or (host_os == "mac" and host_cpu == "arm64"))',
'dep_type': 'cipd',
},

Expand Down
5 changes: 5 additions & 0 deletions build/config/BUILDCONFIG.gn
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ if (is_win) {
"//build/config/win:unicode",
"//build/config/win:winver",
]
if (is_clang) {
_native_compiler_configs += [
"//build/config/win:relative_paths",
]
}
}
if (is_posix) {
_native_compiler_configs += [
Expand Down
41 changes: 41 additions & 0 deletions build/config/win/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# found in the LICENSE file.

import("//build/config/win/visual_studio_version.gni")
import("//build/toolchain/rbe.gni")

# Compiler setup for the Windows SDK. Applied to all targets.
config("sdk") {
Expand Down Expand Up @@ -180,3 +181,43 @@ config("lean_and_mean") {
config("nominmax") {
defines = [ "NOMINMAX" ]
}

# Relative paths --------------------------------------------------------------

config("relative_paths") {
# Make builds independent of absolute file path. The file names
# embedded in debugging information will be expressed as relative to
# the build directory, e.g. "../.." for an "out/subdir" under //.
# This is consistent with the file names in __FILE__ expansions
# (e.g. in assertion messages), which the compiler doesn't provide a
# way to remap. That way source file names in logging and
# symbolization can all be treated the same way. This won't go well
# if root_build_dir is not a subdirectory //, but there isn't a better
# option to keep all source file name references uniformly relative to
# a single root.
if (use_rbe) {
absolute_path = "/b/f/w/"
} else {
absolute_path = rebase_path("//")
}
relative_path = ""
cflags = [
# This makes sure that debug information uses relative paths.
"-fdebug-prefix-map=$absolute_path=$relative_path",

# Remove absolute paths from the debug information.
"-fdebug-compilation-dir=",
"-fcoverage-compilation-dir=",

# This makes sure that include directories in the toolchain are
# represented as relative to the build directory (because that's how
# we invoke the compiler), rather than absolute. This can affect
# __FILE__ expansions (e.g. assertions in system headers). We
# normally run a compiler that's someplace within the source tree
# (//buildtools/...), so its absolute installation path will have a
# prefix matching absolute_path and hence be mapped to relative_path
# in the debugging information, so this should actually be
# superfluous for purposes of the debugging information.
"-no-canonical-prefixes",
]
}
4 changes: 3 additions & 1 deletion build/rbe/llvm.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#!/bin/sh
#!/bin/bash
os=$(uname -s | tr '[A-Z]' '[a-z'])
arch=$(uname -m | tr '[A-Z]' '[a-z'] | sed -E 's/^x86_64$/x64/')
rm -f "$1"
cp "../../buildtools/$os-$arch/clang/bin/llvm" "$1"
chmod +x "$1"
INCLUDE=${INCLUDE//\\/\/}
"$@"
6 changes: 6 additions & 0 deletions build/rbe/unix.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
service=remotebuildexecution.googleapis.com:443
instance=projects/flutter-rbe-prod/instances/default
use_application_default_credentials=true
enable_deps_cache=true
xattr_digest=user.fuchsia.rbe.digest.sha256
log_format=reducedtext
6 changes: 6 additions & 0 deletions build/rbe/win-intel.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
service=remotebuildexecution.googleapis.com:443
instance=projects/flutter-rbe-prod/instances/default
use_application_default_credentials=true
enable_deps_cache=true
server_address=pipe://reproxy.ipc
log_format=reducedtext
6 changes: 6 additions & 0 deletions build/rbe/windows.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
service=remotebuildexecution.googleapis.com:443
instance=projects/flutter-rbe-prod/instances/default
use_application_default_credentials=true
enable_deps_cache=true
server_address=pipe://reproxy.ipc
log_format=reducedtext
5 changes: 3 additions & 2 deletions build/toolchain/win/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ if (use_goma) {
rewrapper_args + [ "--labels=type=compile,compiler=clang-cl,lang=cpp" ]
if (rbe_os != host_os || rbe_cpu != host_cpu) {
compiler_args += [
"--env_var_allowlist=INCLUDE",
"--inputs=build/rbe,buildtools/$rbe_os-$rbe_cpu/clang/bin/llvm",
"--remote_wrapper=../../build/rbe/llvm.sh",
]
Expand Down Expand Up @@ -85,7 +86,7 @@ template("msvc_toolchain") {
# TODO(brettw) enable this when GN support in the binary has been rolled.
#precompiled_header_type = "msvc"
pdbname = "{{target_out_dir}}/{{target_output_name}}_c.pdb"
command = "$ninja_path -t msvc -e $env -- $cl /nologo /showIncludes /FC @$rspfile /c {{source}} /Fo{{output}} /Fd$pdbname"
command = "$ninja_path -t msvc -e $env -- $cl /nologo /showIncludes /FC {{defines}} {{include_dirs}} {{cflags}} {{cflags_c}} /c {{source}} /Fo{{output}} /Fd$pdbname"
depsformat = "msvc"
description = "CC {{output}}"
outputs = [
Expand All @@ -106,7 +107,7 @@ template("msvc_toolchain") {
if (is_clang && invoker.current_cpu == "x86") {
flags = "-m32"
}
command = "$ninja_path -t msvc -e $env -- $cl $flags /nologo /showIncludes /FC @$rspfile /c {{source}} /Fo{{output}} /Fd$pdbname"
command = "$ninja_path -t msvc -e $env -- $cl $flags /nologo /showIncludes /FC {{defines}} {{include_dirs}} {{cflags}} {{cflags_cc}} /c {{source}} /Fo{{output}} /Fd$pdbname"
depsformat = "msvc"
description = "CXX {{output}}"
outputs = [
Expand Down
14 changes: 13 additions & 1 deletion build/toolchain/win/setup_toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ def _ExtractImportantEnvironment(output_of_set):
'luci_context', # needed by vpython
'path',
'pathext',
'rbe_cfg', # Dart specific patch: RBE_cfg is needed by reclient.
'rbe_server_address', # Dart specific patch: RBE_server_address ditto.
'systemroot',
'temp',
'tmp',
Expand All @@ -59,15 +61,25 @@ def _ExtractImportantEnvironment(output_of_set):
# path. Add the path to this python here so that if it's not in the
# path when ninja is run later, python will still be found.
setting = os.path.dirname(sys.executable) + os.pathsep + setting
if envvar in ['include', 'lib']:
# Dart specific patch: Ensure the environment variables use relative
# paths to the toolchain such that RBE commands can be cached
# remotely due to no absolute paths. Rewrite libpath as well although
# don't use it remotely at the moment.
if envvar in ['include', 'lib', 'libpath']:
exec_root_abs = os.path.dirname(os.path.dirname(os.getcwd()))
exec_root_rel = os.path.join('..', '..')
# Make sure that the include and lib paths point to directories that
# exist. This ensures a (relatively) clear error message if the
# required SDK is not installed.
parts = []
for part in setting.split(';'):
part = part.replace(exec_root_abs, exec_root_rel)
if not os.path.exists(part) and len(part) != 0:
raise Exception(
'Path "%s" from environment variable "%s" does not exist. '
'Make sure the necessary SDK is installed.' % (part, envvar))
parts.append(part)
setting = ';'.join(parts)
env[var.upper()] = setting
break
if sys.platform in ('win32', 'cygwin'):
Expand Down
5 changes: 5 additions & 0 deletions build/vs_toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,12 +517,17 @@ def Update(force=False, no_download=False):
subprocess.check_call([
ciopfs, '-o', 'use_ino', toolchain_dir + '.ciopfs', toolchain_dir])

# Dart specific patch: Store the visual studio toolchain inside the exec
# root directory such that it can be sent to RBE and invoked with a
# relative path identical on all checkouts.
toolchain_dir = os.path.join(os.getcwd(), 'sdk', 'win_toolchain')
get_toolchain_args = [
sys.executable,
os.path.join(depot_tools_path,
'win_toolchain',
'get_toolchain_if_necessary.py'),
'--output-json', json_data_file,
'--toolchain-dir', toolchain_dir,
] + _GetDesiredVsToolchainHashes()
if force:
get_toolchain_args.append('--force')
Expand Down
20 changes: 10 additions & 10 deletions tools/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def BuildOptions():
other_group.add_argument("-j",
type=int,
help='Ninja -j option for Goma/RBE builds.',
default=1000)
default=200 if sys.platform == 'win32' else 1000)
other_group.add_argument("-l",
type=int,
help='Ninja -l option for Goma/RBE builds.',
Expand Down Expand Up @@ -140,7 +140,7 @@ def UseRBE(out_dir):
bootstrap_path = None


def StartRBE(out_dir, use_goma):
def StartRBE(out_dir, use_goma, env):
global rbe_started, bootstrap_path
rbe = 'goma' if use_goma else 'rbe'
if rbe_started:
Expand All @@ -166,7 +166,7 @@ def StartRBE(out_dir, use_goma):
bootstrap_command = [bootstrap_path]
if use_goma:
bootstrap_command = ['python3', bootstrap_path, 'ensure_start']
process = subprocess.Popen(bootstrap_command)
process = subprocess.Popen(bootstrap_command, env=env)
process.wait()
if process.returncode != 0:
print(f"Starting {rbe} failed. Try running it manually: " + "\n\t" +
Expand All @@ -176,17 +176,17 @@ def StartRBE(out_dir, use_goma):
return True


def StopRBE():
def StopRBE(env):
global rbe_started, bootstrap_path
if rbe_started != 'rbe':
return
bootstrap_command = [bootstrap_path, '--shutdown']
process = subprocess.Popen(bootstrap_command)
process = subprocess.Popen(bootstrap_command, env=env)
process.wait()


# Returns a tuple (build_config, command to run, whether rbe is used)
def BuildOneConfig(options, targets, target_os, mode, arch, sanitizer):
def BuildOneConfig(options, targets, target_os, mode, arch, sanitizer, env):
build_config = utils.GetBuildConf(mode, arch, target_os, sanitizer)
out_dir = utils.GetBuildRoot(HOST_OS, mode, arch, target_os, sanitizer)
using_rbe = False
Expand All @@ -196,7 +196,7 @@ def BuildOneConfig(options, targets, target_os, mode, arch, sanitizer):
use_rbe = UseRBE(out_dir)
use_goma = UseGoma(out_dir)
if use_rbe or use_goma:
if options.no_start_rbe or StartRBE(out_dir, use_goma):
if options.no_start_rbe or StartRBE(out_dir, use_goma, env):
using_rbe = True
command += [('-j%s' % str(options.j))]
command += [('-l%s' % str(options.l))]
Expand Down Expand Up @@ -327,7 +327,7 @@ def Main():
env.pop('SDKROOT', None)

# Always run GN before building.
gn_py.RunGnOnConfiguredConfigurations(options)
gn_py.RunGnOnConfiguredConfigurations(options, env)

# Build all targets for each requested configuration.
configs = []
Expand All @@ -337,11 +337,11 @@ def Main():
for sanitizer in options.sanitizer:
configs.append(
BuildOneConfig(options, targets, target_os, mode, arch,
sanitizer))
sanitizer, env))

exit_code = Build(configs, env, options)

StopRBE()
StopRBE(env)

if exit_code == 0:
endtime = time.time()
Expand Down
Loading

0 comments on commit ac06ea0

Please sign in to comment.