From 4b4cd840bb23a9fcb5423f70b068b31bf3d5e776 Mon Sep 17 00:00:00 2001 From: Alex Hornby Date: Tue, 19 Dec 2023 12:24:39 -0800 Subject: [PATCH] getdeps: add --build-type option to build in Debug or RelWithDebInfo mode Summary: X-link: https://github.com/facebookincubator/zstrong/pull/653 getdeps: add --build-type option to build in Debug or RelWithDebInfo mode Adds a --build-type option so one can force RelWithDebInfo or Debug explicity Default remains RelWithDebInfo for cmake. cargo default is updated to --release to match cmake more closely, if you don't want release use --build-type Debug. If you want to run github CI in Debug mode (faster build, but tests will run slower), then can pass --build-type Debug to getdeps.py generate-github-actions X-link: https://github.com/facebook/sapling/pull/786 Reviewed By: mitrandir77 Differential Revision: D51564770 Pulled By: bigfootjon fbshipit-source-id: ef30332ca193d9805bce005d12b5dbc9f58fcafc --- build/fbcode_builder/getdeps.py | 22 ++++++++++++++++++++-- build/fbcode_builder/getdeps/builder.py | 10 ++++++---- build/fbcode_builder/getdeps/buildopts.py | 8 ++++++++ build/fbcode_builder/getdeps/cargo.py | 18 +++++++++++++----- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/build/fbcode_builder/getdeps.py b/build/fbcode_builder/getdeps.py index 7f824c0ebd4f..aeae6bf63467 100755 --- a/build/fbcode_builder/getdeps.py +++ b/build/fbcode_builder/getdeps.py @@ -814,6 +814,13 @@ def setup_project_cmd_parser(self, parser): action="store_true", default=False, ) + parser.add_argument( + "--build-type", + help="Set the build type explicitly. Cmake and cargo builders act on them. Only Debug and RelWithDebInfo widely supported.", + choices=["Debug", "Release", "RelWithDebInfo", "MinSizeRel"], + action="store", + default=None, + ) @cmd("fixup-dyn-deps", "Adjusts dynamic dependencies for packaging purposes") @@ -1038,6 +1045,10 @@ def write_job_for_platform(self, platform, args): # noqa: C901 out.write(" - uses: actions/checkout@v4\n") + build_type_arg = "" + if args.build_type: + build_type_arg = f"--build-type {args.build_type} " + if build_opts.free_up_disk: free_up_disk = "--free-up-disk " if not build_opts.is_windows(): @@ -1118,7 +1129,7 @@ def write_job_for_platform(self, platform, args): # noqa: C901 has_same_repo_dep = True out.write(" - name: Build %s\n" % m.name) out.write( - f" run: {getdepscmd}{allow_sys_arg} build {src_dir_arg}{free_up_disk}--no-tests {m.name}\n" + f" run: {getdepscmd}{allow_sys_arg} build {build_type_arg}{src_dir_arg}{free_up_disk}--no-tests {m.name}\n" ) out.write(" - name: Build %s\n" % manifest.name) @@ -1139,7 +1150,7 @@ def write_job_for_platform(self, platform, args): # noqa: C901 no_tests_arg = "--no-tests " out.write( - f" run: {getdepscmd}{allow_sys_arg} build {no_tests_arg}{no_deps_arg}--src-dir=. {manifest.name} {project_prefix}\n" + f" run: {getdepscmd}{allow_sys_arg} build {build_type_arg}{no_tests_arg}{no_deps_arg}--src-dir=. {manifest.name} {project_prefix}\n" ) out.write(" - name: Copy artifacts\n") @@ -1225,6 +1236,13 @@ def setup_project_cmd_parser(self, parser): action="store_true", default=False, ) + parser.add_argument( + "--build-type", + help="Set the build type explicitly. Cmake and cargo builders act on them. Only Debug and RelWithDebInfo widely supported.", + choices=["Debug", "Release", "RelWithDebInfo", "MinSizeRel"], + action="store", + default=None, + ) def get_arg_var_name(args): diff --git a/build/fbcode_builder/getdeps/builder.py b/build/fbcode_builder/getdeps/builder.py index 0aa937af6c24..25e2076e99d6 100644 --- a/build/fbcode_builder/getdeps/builder.py +++ b/build/fbcode_builder/getdeps/builder.py @@ -473,7 +473,7 @@ def main(): "--target", target, "--config", - "Release", + "{build_type}", get_jobs_argument(args.num_jobs), ] + args.cmake_args elif args.mode == "test": @@ -595,8 +595,9 @@ def _compute_cmake_define_args(self, env): # unspecified. Some of the deps fail to compile in release mode # due to warning->error promotion. RelWithDebInfo is the happy # medium. - "CMAKE_BUILD_TYPE": "RelWithDebInfo", + "CMAKE_BUILD_TYPE": self.build_opts.build_type, } + if "SANDCASTLE" not in os.environ: # We sometimes see intermittent ccache related breakages on some # of the FB internal CI hosts, so we prefer to disable ccache @@ -693,6 +694,7 @@ def _build(self, install_dirs, reconfigure: bool) -> None: build_dir=self.build_dir, install_dir=self.inst_dir, sys=sys, + build_type=self.build_opts.build_type, ) self._invalidate_cache() @@ -706,7 +708,7 @@ def _build(self, install_dirs, reconfigure: bool) -> None: "--target", self.cmake_target, "--config", - "Release", + self.build_opts.build_type, "-j", str(self.num_jobs), ], @@ -1179,7 +1181,7 @@ def _build(self, install_dirs, reconfigure) -> None: "--target", "install", "--config", - "Release", + self.build_opts.build_type, "-j", str(self.num_jobs), ], diff --git a/build/fbcode_builder/getdeps/buildopts.py b/build/fbcode_builder/getdeps/buildopts.py index 48b000f90ebc..63bf7c696e1a 100644 --- a/build/fbcode_builder/getdeps/buildopts.py +++ b/build/fbcode_builder/getdeps/buildopts.py @@ -52,6 +52,7 @@ def __init__( shared_libs: bool = False, facebook_internal=None, free_up_disk: bool = False, + build_type: Optional[str] = None, ) -> None: """fbcode_builder_dir - the path to either the in-fbsource fbcode_builder dir, or for shipit-transformed repos, the build dir that @@ -67,6 +68,7 @@ def __init__( vcvars_path - Path to external VS toolchain's vsvarsall.bat shared_libs - whether to build shared libraries free_up_disk - take extra actions to save runner disk space + build_type - CMAKE_BUILD_TYPE, used by cmake and cargo builders """ if not install_dir: @@ -107,6 +109,11 @@ def __init__( self.shared_libs = shared_libs self.free_up_disk = free_up_disk + if build_type is None: + build_type = "RelWithDebInfo" + + self.build_type = build_type + lib_path = None if self.is_darwin(): lib_path = "DYLD_LIBRARY_PATH" @@ -606,6 +613,7 @@ def setup_build_options(args, host_type=None) -> BuildOptions: "lfs_path", "shared_libs", "free_up_disk", + "build_type", } } diff --git a/build/fbcode_builder/getdeps/cargo.py b/build/fbcode_builder/getdeps/cargo.py index bb41cc3e294b..315f80b17097 100644 --- a/build/fbcode_builder/getdeps/cargo.py +++ b/build/fbcode_builder/getdeps/cargo.py @@ -148,21 +148,29 @@ def _prepare(self, install_dirs, reconfigure) -> None: def _build(self, install_dirs, reconfigure) -> None: # _prepare has been run already. Actually do the build build_source_dir = self.build_source_dir() + + build_args = [ + "--out-dir", + os.path.join(self.inst_dir, "bin"), + "-Zunstable-options", + ] + + if self.build_opts.build_type != "Debug": + build_args.append("--release") + if self.manifests_to_build is None: self.run_cargo( install_dirs, "build", - ["--out-dir", os.path.join(self.inst_dir, "bin"), "-Zunstable-options"], + build_args, ) else: for manifest in self.manifests_to_build: self.run_cargo( install_dirs, "build", - [ - "--out-dir", - os.path.join(self.inst_dir, "bin"), - "-Zunstable-options", + build_args + + [ "--manifest-path", self.manifest_dir(manifest), ],