From 21cbf997f45986cd23f9185647063333e6ad0e92 Mon Sep 17 00:00:00 2001 From: Chad Austin Date: Fri, 20 May 2022 12:33:19 -0700 Subject: [PATCH] make errors better when ctest is not found Summary: Without ctest installed, `getdeps.py test $project` would produce a bad error message about failing to run a `''` command. Instead, explicitly error early when a command is required. Reviewed By: genevievehelsel Differential Revision: D36535929 fbshipit-source-id: d300c6b1b0c124b56dffffae0a71bee9b7f12fe7 --- build/fbcode_builder/getdeps/builder.py | 26 ++++++++++++++++++++---- build/fbcode_builder/getdeps/envfuncs.py | 5 +++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/build/fbcode_builder/getdeps/builder.py b/build/fbcode_builder/getdeps/builder.py index 9e75ff872..483a0513c 100644 --- a/build/fbcode_builder/getdeps/builder.py +++ b/build/fbcode_builder/getdeps/builder.py @@ -13,6 +13,7 @@ import stat import subprocess import sys import typing +from typing import Optional from .dyndeps import create_dyn_dep_munger from .envfuncs import add_path_entry, Env, path_search @@ -723,6 +724,11 @@ if __name__ == "__main__": ctest = path_search(env, "ctest") cmake = path_search(env, "cmake") + def require_command(path: Optional[str], name: str) -> str: + if path is None: + raise RuntimeError("unable to find command `{}`".format(name)) + return path + # On Windows, we also need to update $PATH to include the directories that # contain runtime library dependencies. This is not needed on other platforms # since CMake will emit RPATH properly in the binary so they can find these @@ -756,7 +762,9 @@ if __name__ == "__main__": def list_tests(): output = subprocess.check_output( - [ctest, "--show-only=json-v1"], env=env, cwd=self.build_dir + [require_command(ctest, "ctest"), "--show-only=json-v1"], + env=env, + cwd=self.build_dir, ) try: data = json.loads(output.decode("utf-8")) @@ -780,7 +788,12 @@ if __name__ == "__main__": labels.append("disabled") command = test["command"] if working_dir: - command = [cmake, "-E", "chdir", working_dir] + command + command = [ + require_command(cmake, "cmake"), + "-E", + "chdir", + working_dir, + ] + command import os @@ -906,7 +919,12 @@ if __name__ == "__main__": use_cmd_prefix=use_cmd_prefix, ) else: - args = [ctest, "--output-on-failure", "-j", str(self.num_jobs)] + args = [ + require_command(ctest, "ctest"), + "--output-on-failure", + "-j", + str(self.num_jobs), + ] if test_filter: args += ["-R", test_filter] @@ -964,7 +982,7 @@ class OpenSSLBuilder(BuilderBase): bindir = os.path.join(d, "bin") add_path_entry(env, "PATH", bindir, append=False) - perl = path_search(env, "perl", "perl") + perl = typing.cast(str, path_search(env, "perl", "perl")) make_j_args = [] if self.build_opts.is_windows(): diff --git a/build/fbcode_builder/getdeps/envfuncs.py b/build/fbcode_builder/getdeps/envfuncs.py index 8ada5d24e..6072a69ec 100644 --- a/build/fbcode_builder/getdeps/envfuncs.py +++ b/build/fbcode_builder/getdeps/envfuncs.py @@ -6,6 +6,7 @@ import os import shlex import sys +from typing import Optional class Env(object): @@ -158,7 +159,7 @@ def tpx_path() -> str: return "xplat/testinfra/tpx/ctp.tpx" -def path_search(env, exename: str, defval=None): +def path_search(env, exename: str, defval: Optional[str] = None) -> Optional[str]: """Search for exename in the PATH specified in env. exename is eg: `ninja` and this function knows to append a .exe to the end on windows. @@ -180,7 +181,7 @@ def path_search(env, exename: str, defval=None): return result -def _perform_path_search(path, exename: str): +def _perform_path_search(path, exename: str) -> Optional[str]: is_win = sys.platform.startswith("win") if is_win: exename = "%s.exe" % exename