From ac06ea07bdd681ad3eafa9477d3c2e662b5a4833 Mon Sep 17 00:00:00 2001 From: Jonas Termansen Date: Tue, 19 Mar 2024 09:22:08 +0000 Subject: [PATCH] [beta] Migrate from Goma to RBE. 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: https://github.com/dart-lang/sdk/issues/55184 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357342 Reviewed-by: William Hesse --- .gitignore | 3 ++ DEPS | 7 ++- build/config/BUILDCONFIG.gn | 5 +++ build/config/win/BUILD.gn | 41 ++++++++++++++++++ build/rbe/llvm.sh | 4 +- build/rbe/unix.cfg | 6 +++ build/rbe/win-intel.cfg | 6 +++ build/rbe/windows.cfg | 6 +++ build/toolchain/win/BUILD.gn | 5 ++- build/toolchain/win/setup_toolchain.py | 14 +++++- build/vs_toolchain.py | 5 +++ tools/build.py | 20 ++++----- tools/gn.py | 60 +++++++++++++++++++++----- 13 files changed, 156 insertions(+), 26 deletions(-) create mode 100644 build/rbe/unix.cfg create mode 100644 build/rbe/win-intel.cfg create mode 100644 build/rbe/windows.cfg diff --git a/.gitignore b/.gitignore index 781d9dfd4f0b..083421b4965d 100644 --- a/.gitignore +++ b/.gitignore @@ -39,6 +39,9 @@ CMakeLists.txt .clang_complete cmake-build-debug +# Windows toolchain. +win_toolchain + # VS project files .vs diff --git a/DEPS b/DEPS index 9f95f2770fea..611c76299a32 100644 --- a/DEPS +++ b/DEPS @@ -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", @@ -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', }, diff --git a/build/config/BUILDCONFIG.gn b/build/config/BUILDCONFIG.gn index f7d4b62e880b..51eb4c212f74 100644 --- a/build/config/BUILDCONFIG.gn +++ b/build/config/BUILDCONFIG.gn @@ -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 += [ diff --git a/build/config/win/BUILD.gn b/build/config/win/BUILD.gn index 232ab059c9eb..3d918548e01e 100644 --- a/build/config/win/BUILD.gn +++ b/build/config/win/BUILD.gn @@ -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") { @@ -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", + ] +} diff --git a/build/rbe/llvm.sh b/build/rbe/llvm.sh index a5a241434f82..55ddb1cf415a 100755 --- a/build/rbe/llvm.sh +++ b/build/rbe/llvm.sh @@ -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//\\/\/} "$@" diff --git a/build/rbe/unix.cfg b/build/rbe/unix.cfg new file mode 100644 index 000000000000..d5796d2939d6 --- /dev/null +++ b/build/rbe/unix.cfg @@ -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 diff --git a/build/rbe/win-intel.cfg b/build/rbe/win-intel.cfg new file mode 100644 index 000000000000..9892e10e7e44 --- /dev/null +++ b/build/rbe/win-intel.cfg @@ -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 diff --git a/build/rbe/windows.cfg b/build/rbe/windows.cfg new file mode 100644 index 000000000000..9892e10e7e44 --- /dev/null +++ b/build/rbe/windows.cfg @@ -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 diff --git a/build/toolchain/win/BUILD.gn b/build/toolchain/win/BUILD.gn index 248728b09b3a..e5ee9a4abf64 100644 --- a/build/toolchain/win/BUILD.gn +++ b/build/toolchain/win/BUILD.gn @@ -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", ] @@ -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 = [ @@ -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 = [ diff --git a/build/toolchain/win/setup_toolchain.py b/build/toolchain/win/setup_toolchain.py index 521c24398ef9..1e90ed8800b3 100644 --- a/build/toolchain/win/setup_toolchain.py +++ b/build/toolchain/win/setup_toolchain.py @@ -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', @@ -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'): diff --git a/build/vs_toolchain.py b/build/vs_toolchain.py index a9cd6f03d921..af493b8a6832 100644 --- a/build/vs_toolchain.py +++ b/build/vs_toolchain.py @@ -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') diff --git a/tools/build.py b/tools/build.py index 07c7a7709289..89a5522e5b26 100755 --- a/tools/build.py +++ b/tools/build.py @@ -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.', @@ -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: @@ -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" + @@ -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 @@ -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))] @@ -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 = [] @@ -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() diff --git a/tools/gn.py b/tools/gn.py index 9a77144b8556..08fb92c397b9 100755 --- a/tools/gn.py +++ b/tools/gn.py @@ -411,13 +411,26 @@ def ProcessOptions(args): if HOST_OS != 'win' and args.use_crashpad: print("Crashpad is only supported on Windows") return False - if os.environ.get('RBE_cfg') == None and \ + default_rbe = os.environ.get('RBE') == '1' or \ + os.environ.get('DART_RBE') == '1' or \ + os.environ.get('RBE_cfg') != None + if not default_rbe and args.rbe: + args.goma = False + elif default_rbe and args.goma: + args.rbe = False + if not args.rbe and \ (socket.getfqdn().endswith('.corp.google.com') or - socket.getfqdn().endswith('.c.googlers.com')) and \ - sys.platform in ['linux']: + socket.getfqdn().endswith('.c.googlers.com')): print('You can speed up your build by following: go/dart-rbe') if not args.rbe and not args.goma: print('Goma is no longer enabled by default since RBE is ready.') + old_rbe_cfg = 'win-intel.cfg' if HOST_OS == 'win32' else 'linux-intel.cfg' + new_rbe_cfg = 'windows.cfg' if HOST_OS == 'win32' else 'unix.cfg' + if os.environ.get('RBE_cfg') == os.path.join(os.getcwd(), 'build', 'rbe', + old_rbe_cfg): + print(f'warning: {old_rbe_cfg} is deprecated, please update your ' + f'RBE_cfg variable to {new_rbe_cfg} use RBE=1 instead per ' + 'go/dart-rbe') return True @@ -437,21 +450,24 @@ def ide_switch(host_os): def AddCommonGnOptionArgs(parser): """Adds arguments that will change the default GN arguments.""" - use_rbe = os.environ.get('RBE_cfg') != None + default_rbe = os.environ.get('RBE') == '1' or \ + os.environ.get('DART_RBE') == '1' or \ + os.environ.get('RBE_cfg') != None parser.add_argument('--goma', help='Use goma', action='store_true') parser.add_argument('--no-goma', help='Disable goma', dest='goma', action='store_false') - parser.set_defaults(goma=not use_rbe and sys.platform not in ['linux']) + parser.set_defaults( + goma=not default_rbe and sys.platform not in ['linux', 'win32']) parser.add_argument('--rbe', help='Use rbe', action='store_true') parser.add_argument('--no-rbe', help='Disable rbe', dest='rbe', action='store_false') - parser.set_defaults(rbe=use_rbe) + parser.set_defaults(rbe=default_rbe) # Disable git hashes when remote compiling to ensure cache hits of the final # output artifacts when nothing has changed. @@ -464,7 +480,7 @@ def AddCommonGnOptionArgs(parser): help='Disable SDK hash checks', dest='verify_sdk_hash', action='store_false') - parser.set_defaults(verify_sdk_hash=not use_rbe) + parser.set_defaults(verify_sdk_hash=not default_rbe) parser.add_argument('--git-version', help='Enable git commit in version', @@ -475,7 +491,7 @@ def AddCommonGnOptionArgs(parser): help='Disable git commit in version', dest='git_version', action='store_false') - parser.set_defaults(git_version=not use_rbe) + parser.set_defaults(git_version=not default_rbe) parser.add_argument('--clang', help='Use Clang', action='store_true') parser.add_argument('--no-clang', @@ -611,6 +627,26 @@ def parse_args(args): return options +def InitializeRBE(out_dir, env): + RBE_cfg = 'RBE_CFG' if HOST_OS == 'win32' else 'RBE_cfg' + RBE_server_address = ('RBE_SERVER_ADDRESS' + if HOST_OS == 'win32' else 'RBE_server_address') + # Default RBE_cfg to the appropriate configuration file. + if not RBE_cfg in env: + env[RBE_cfg] = os.path.join( + os.getcwd(), 'build', 'rbe', + 'windows.cfg' if HOST_OS == 'win32' else 'unix.cfg') + # Default RBE_server_address to inside the build directory. + if not RBE_server_address in env: + with open(env[RBE_cfg], 'r') as f: + if not any([l.startswith('server_address') for l in f.readlines()]): + schema = 'pipe' if HOST_OS == 'win32' else 'unix' + socket = os.path.join(os.getcwd(), out_dir, 'reproxy.sock') + if HOST_OS == 'win32': + socket = socket.replace('\\', '_').replace(':', '_') + env[RBE_server_address] = f'{schema}://{socket}' + + def ExecutableName(basename): if utils.IsWindows(): return f'{basename}.exe' @@ -642,13 +678,17 @@ def BuildGnCommand(args, mode, arch, target_os, sanitizer, out_dir): return command -def RunGnOnConfiguredConfigurations(args): +def RunGnOnConfiguredConfigurations(args, env={}): + initialized_rbe = False commands = [] for target_os in args.os: for mode in args.mode: for arch in args.arch: for sanitizer in args.sanitizer: out_dir = GetOutDir(mode, arch, target_os, sanitizer) + if args.rbe and not initialized_rbe: + InitializeRBE(out_dir, env) + initialized_rbe = True commands.append( BuildGnCommand(args, mode, arch, target_os, sanitizer, out_dir)) @@ -664,7 +704,7 @@ def cleanup(command): for command in commands: try: - process = subprocess.Popen(command, cwd=DART_ROOT) + process = subprocess.Popen(command, cwd=DART_ROOT, env=env) active_commands.append([command, process]) except Exception as e: print('Error: %s' % e)