From e40d21286276308b428e7b3a8cd24983e3f3aec3 Mon Sep 17 00:00:00 2001 From: Adam Simpkins Date: Thu, 15 Aug 2019 17:53:32 -0700 Subject: [PATCH] getdeps: consolidate code for project subcommands Summary: Most of the getdeps subcommands operate on a single project, and some of the argument parsing and initial logic to load the project is largely the same. This consolidates that logic into a base class so that we can share this code across subcommands, instead of repeating it. This also unifies the behavior so that by default all commands enable tests on the specified project, and disable test on dependent projects. Making sure all commands use the same behavior here is important as whether are not tests are enabled can affect the project configuration, and therefore affect its getdeps hash. Reviewed By: wez Differential Revision: D16778010 fbshipit-source-id: 044f99ad6cdd4a56f843276cec8ead786249ee7a --- build/fbcode_builder/getdeps.py | 215 +++++++++++--------------------- 1 file changed, 70 insertions(+), 145 deletions(-) diff --git a/build/fbcode_builder/getdeps.py b/build/fbcode_builder/getdeps.py index 345e125f3..ff8c6fc7d 100755 --- a/build/fbcode_builder/getdeps.py +++ b/build/fbcode_builder/getdeps.py @@ -58,8 +58,22 @@ class ShowHostType(SubCmd): return 0 -@cmd("fetch", "fetch the code for a given project") -class FetchCmd(SubCmd): +class ProjectCmdBase(SubCmd): + def run(self, args): + opts = setup_build_options(args) + ctx_gen = opts.get_context_generator(facebook_internal=args.facebook_internal) + if args.test_dependencies: + ctx_gen.set_value_for_all_projects("test", "on") + if args.enable_tests: + ctx_gen.set_value_for_project(args.project, "test", "on") + else: + ctx_gen.set_value_for_project(args.project, "test", "off") + + loader = ManifestLoader(opts, ctx_gen) + manifest = loader.load_manifest(args.project) + + self.run_project_cmd(args, loader, manifest) + def setup_parser(self, parser): parser.add_argument( "project", @@ -68,6 +82,28 @@ class FetchCmd(SubCmd): "file describing the project" ), ) + parser.add_argument( + "--no-tests", + action="store_false", + dest="enable_tests", + default=True, + help="Disable building tests for this project.", + ) + parser.add_argument( + "--test-dependencies", + action="store_true", + help="Enable building tests for dependencies as well.", + ) + + self.setup_project_cmd_parser(parser) + + def setup_project_cmd_parser(self, parser): + pass + + +@cmd("fetch", "fetch the code for a given project") +class FetchCmd(ProjectCmdBase): + def setup_project_cmd_parser(self, parser): parser.add_argument( "--recursive", help="fetch the transitive deps also", @@ -82,10 +118,7 @@ class FetchCmd(SubCmd): ), ) - def run(self, args): - opts = setup_build_options(args) - loader = ManifestLoader(opts) - manifest = loader.load_manifest(args.project) + def run_project_cmd(self, args, loader, manifest): if args.recursive: projects = loader.manifests_in_dependency_order() else: @@ -96,17 +129,13 @@ class FetchCmd(SubCmd): @cmd("list-deps", "lists the transitive deps for a given project") -class ListDepsCmd(SubCmd): - def run(self, args): - opts = setup_build_options(args) - loader = ManifestLoader(opts) - loader.ctx_gen.set_value_for_project(args.project, "test", "on") - loader.load_manifest(args.project) +class ListDepsCmd(ProjectCmdBase): + def run_project_cmd(self, args, loader, manifest): for m in loader.manifests_in_dependency_order(): print(m.name) return 0 - def setup_parser(self, parser): + def setup_project_cmd_parser(self, parser): parser.add_argument( "--host-type", help=( @@ -114,13 +143,6 @@ class ListDepsCmd(SubCmd): "rather than that of the current system" ), ) - parser.add_argument( - "project", - help=( - "name of the project or path to a manifest " - "file describing the project" - ), - ) def clean_dirs(opts): @@ -139,16 +161,10 @@ class CleanCmd(SubCmd): @cmd("show-inst-dir", "print the installation dir for a given project") -class ShowInstDirCmd(SubCmd): - def run(self, args): - opts = setup_build_options(args) - loader = ManifestLoader(opts) - loader.ctx_gen.set_value_for_project(args.project, "test", "on") - manifest = loader.load_manifest(args.project) - projects = loader.manifests_in_dependency_order() - +class ShowInstDirCmd(ProjectCmdBase): + def run_project_cmd(self, args, loader, manifest): if args.recursive: - manifests = projects + manifests = loader.manifests_in_dependency_order() else: manifests = [manifest] @@ -156,14 +172,7 @@ class ShowInstDirCmd(SubCmd): inst_dir = loader.get_project_install_dir(m) print(inst_dir) - def setup_parser(self, parser): - parser.add_argument( - "project", - help=( - "name of the project or path to a manifest " - "file describing the project" - ), - ) + def setup_project_cmd_parser(self, parser): parser.add_argument( "--recursive", help="print the transitive deps also", @@ -173,13 +182,8 @@ class ShowInstDirCmd(SubCmd): @cmd("show-source-dir", "print the source dir for a given project") -class ShowSourceDirCmd(SubCmd): - def run(self, args): - opts = setup_build_options(args) - loader = ManifestLoader(opts) - loader.ctx_gen.set_value_for_project(args.project, "test", "on") - manifest = loader.load_manifest(args.project) - +class ShowSourceDirCmd(ProjectCmdBase): + def run_project_cmd(self, args, loader, manifest): if args.recursive: manifests = loader.manifests_in_dependency_order() else: @@ -189,14 +193,7 @@ class ShowSourceDirCmd(SubCmd): fetcher = loader.create_fetcher(m) print(fetcher.get_src_dir()) - def setup_parser(self, parser): - parser.add_argument( - "project", - help=( - "name of the project or path to a manifest " - "file describing the project" - ), - ) + def setup_project_cmd_parser(self, parser): parser.add_argument( "--recursive", help="print the transitive deps also", @@ -206,20 +203,12 @@ class ShowSourceDirCmd(SubCmd): @cmd("build", "build a given project") -class BuildCmd(SubCmd): - def run(self, args): - opts = setup_build_options(args) - ctx_gen = opts.get_context_generator(facebook_internal=args.facebook_internal) - if args.enable_tests: - ctx_gen.set_value_for_project(args.project, "test", "on") - loader = ManifestLoader(opts, ctx_gen) - +class BuildCmd(ProjectCmdBase): + def run_project_cmd(self, args, loader, manifest): if args.clean: - clean_dirs(opts) + clean_dirs(loader.build_opts) - manifest = loader.load_manifest(args.project) - - print("Building on %s" % ctx_gen.get_context(args.project)) + print("Building on %s" % loader.ctx_gen.get_context(args.project)) projects = loader.manifests_in_dependency_order() # Accumulate the install directories so that the build steps @@ -256,8 +245,10 @@ class BuildCmd(SubCmd): if os.path.exists(built_marker): os.unlink(built_marker) src_dir = fetcher.get_src_dir() - ctx = ctx_gen.get_context(m.name) - builder = m.create_builder(opts, src_dir, build_dir, inst_dir, ctx) + ctx = loader.ctx_gen.get_context(m.name) + builder = m.create_builder( + loader.build_opts, src_dir, build_dir, inst_dir, ctx + ) builder.build(install_dirs, reconfigure=reconfigure) with open(built_marker, "w") as f: @@ -265,14 +256,7 @@ class BuildCmd(SubCmd): install_dirs.append(inst_dir) - def setup_parser(self, parser): - parser.add_argument( - "project", - help=( - "name of the project or path to a manifest " - "file describing the project" - ), - ) + def setup_project_cmd_parser(self, parser): parser.add_argument( "--clean", action="store_true", @@ -293,31 +277,11 @@ class BuildCmd(SubCmd): "slow up-to-date-ness checks" ), ) - parser.add_argument( - "--enable-tests", - action="store_true", - default=False, - help=( - "For the named project, build tests so that the test command " - "is able to execute tests" - ), - ) - parser.add_argument( - "--schedule-type", help="Indicates how the build was activated" - ) @cmd("fixup-dyn-deps", "Adjusts dynamic dependencies for packaging purposes") -class FixupDeps(SubCmd): - def run(self, args): - opts = setup_build_options(args) - ctx_gen = opts.get_context_generator(facebook_internal=args.facebook_internal) - if args.enable_tests: - ctx_gen.set_value_for_project(args.project, "test", "on") - - loader = ManifestLoader(opts, ctx_gen) - manifest = loader.load_manifest(args.project) - +class FixupDeps(ProjectCmdBase): + def run_project_cmd(self, args, loader, manifest): projects = loader.manifests_in_dependency_order() # Accumulate the install directories so that the build steps @@ -329,47 +293,19 @@ class FixupDeps(SubCmd): install_dirs.append(inst_dir) if m == manifest: - dep_munger = create_dyn_dep_munger(opts, install_dirs) + dep_munger = create_dyn_dep_munger(loader.build_opts, install_dirs) dep_munger.process_deps(args.destdir, args.final_install_prefix) - def setup_parser(self, parser): - parser.add_argument( - "project", - help=( - "name of the project or path to a manifest " - "file describing the project" - ), - ) + def setup_project_cmd_parser(self, parser): parser.add_argument("destdir", help=("Where to copy the fixed up executables")) parser.add_argument( "--final-install-prefix", help=("specify the final installation prefix") ) - parser.add_argument( - "--enable-tests", - action="store_true", - default=False, - help=( - "For the named project, build tests so that the test command " - "is able to execute tests" - ), - ) - parser.add_argument( - "--schedule-type", help="Indicates how the build was activated" - ) @cmd("test", "test a given project") -class TestCmd(SubCmd): - def run(self, args): - opts = setup_build_options(args) - ctx_gen = opts.get_context_generator(facebook_internal=args.facebook_internal) - if args.test_all: - ctx_gen.set_value_for_all_projects("test", "on") - else: - ctx_gen.set_value_for_project(args.project, "test", "on") - - loader = ManifestLoader(opts, ctx_gen) - manifest = loader.load_manifest(args.project) +class TestCmd(ProjectCmdBase): + def run_project_cmd(self, args, loader, manifest): projects = loader.manifests_in_dependency_order() # Accumulate the install directories so that the test steps @@ -379,7 +315,7 @@ class TestCmd(SubCmd): for m in projects: inst_dir = loader.get_project_install_dir(m) - if m == manifest or args.test_all: + if m == manifest or args.test_dependencies: built_marker = os.path.join(inst_dir, ".built-by-getdeps") if not os.path.exists(built_marker): print("project %s has not been built" % m.name) @@ -389,27 +325,16 @@ class TestCmd(SubCmd): return 1 fetcher = loader.create_fetcher(m) src_dir = fetcher.get_src_dir() - ctx = ctx_gen.get_context(m.name) + ctx = loader.ctx_gen.get_context(m.name) build_dir = loader.get_project_build_dir(m) - builder = m.create_builder(opts, src_dir, build_dir, inst_dir, ctx) + builder = m.create_builder( + loader.build_opts, src_dir, build_dir, inst_dir, ctx + ) builder.run_tests(install_dirs, schedule_type=args.schedule_type) install_dirs.append(inst_dir) - def setup_parser(self, parser): - parser.add_argument( - "project", - help=( - "name of the project or path to a manifest " - "file describing the project" - ), - ) - parser.add_argument( - "--test-all", - action="store_true", - default=False, - help="Enable running tests for the named project and all of its deps", - ) + def setup_project_cmd_parser(self, parser): parser.add_argument( "--schedule-type", help="Indicates how the build was activated" )