From a9b28d4c15cbd4496705d1f0e924a7e05b05662f Mon Sep 17 00:00:00 2001 From: Alex Hornby Date: Mon, 6 Jan 2025 11:16:30 -0800 Subject: [PATCH] skip unnecessary github actions steps Summary: X-link: https://github.com/facebookincubator/zstrong/pull/1100 Update generated github actions to only run the fetch and and build steps when there are sources expected for a manifest For local github actions testing using `act` this speeds up the test runs, and in real github CI it makes it clearer which steps are actually doing something on the given runner (we don't know exactly what it has installed beforehand) Also set the windows git config the same as on internal CI Reviewed By: bigfootjon Differential Revision: D67839708 fbshipit-source-id: 0a60c6fc89e8c6abb2464f879459aa23d5aec969 --- build/fbcode_builder/getdeps.py | 111 ++++++++++++++++-------- build/fbcode_builder/getdeps/fetcher.py | 4 +- 2 files changed, 80 insertions(+), 35 deletions(-) diff --git a/build/fbcode_builder/getdeps.py b/build/fbcode_builder/getdeps.py index 03a5f3c0..138e729e 100755 --- a/build/fbcode_builder/getdeps.py +++ b/build/fbcode_builder/getdeps.py @@ -552,6 +552,33 @@ def setup_project_cmd_parser(self, parser): ) +@cmd("query-paths", "print the paths for tooling to use") +class QueryPathsCmd(ProjectCmdBase): + def run_project_cmd(self, args, loader, manifest): + if args.recursive: + manifests = loader.manifests_in_dependency_order() + else: + manifests = [manifest] + + for m in manifests: + fetcher = loader.create_fetcher(m) + if isinstance(fetcher, SystemPackageFetcher): + # We are guaranteed that if the fetcher is set to + # SystemPackageFetcher then this item is completely + # satisfied by the appropriate system packages + continue + src_dir = fetcher.get_src_dir() + print(f"{m.name}_SOURCE={src_dir}") + + def setup_project_cmd_parser(self, parser): + parser.add_argument( + "--recursive", + help="print the transitive deps also", + action="store_true", + default=False, + ) + + @cmd("show-source-dir", "print the source dir for a given project") class ShowSourceDirCmd(ProjectCmdBase): def run_project_cmd(self, args, loader, manifest): @@ -1001,6 +1028,10 @@ def write_job_for_platform(self, platform, args): # noqa: C901 manifest_ctx.set("test", "on") run_on = self.get_run_on(args) + tests_arg = "--no-tests " + if run_tests: + tests_arg = "" + # Some projects don't do anything "useful" as a leaf project, only # as a dep for a leaf project. Check for those here; we don't want # to waste the effort scheduling them on CI. @@ -1086,12 +1117,14 @@ def write_job_for_platform(self, platform, args): # noqa: C901 ) out.write(" shell: cmd\n") - # The git installation may not like long filenames, so tell it - # that we want it to use them! out.write(" - name: Fix Git config\n") - out.write(" run: git config --system core.longpaths true\n") - out.write(" - name: Disable autocrlf\n") - out.write(" run: git config --system core.autocrlf false\n") + out.write(" run: >\n") + out.write(" git config --system core.longpaths true &&\n") + out.write(" git config --system core.autocrlf false &&\n") + # cxx crate needs symlinks enabled + out.write(" git config --system core.symlinks true\n") + # && is not supported on default windows powershell, so use cmd + out.write(" shell: cmd\n") out.write(" - uses: actions/checkout@v4\n") @@ -1127,17 +1160,12 @@ def write_job_for_platform(self, platform, args): # noqa: C901 if build_opts.is_darwin(): # brew is installed as regular user sudo_arg = "" - tests_arg = "--no-tests " - if run_tests: - tests_arg = "" - out.write( - f" run: {sudo_arg}python3 build/fbcode_builder/getdeps.py --allow-system-packages install-system-deps {tests_arg}--recursive {manifest.name}\n" - ) + + system_deps_cmd = f"{sudo_arg}{getdepscmd}{allow_sys_arg} install-system-deps {tests_arg}--recursive {manifest.name}" if build_opts.is_linux() or build_opts.is_freebsd(): - out.write(" - name: Install packaging system deps\n") - out.write( - f" run: {sudo_arg}python3 build/fbcode_builder/getdeps.py --allow-system-packages install-system-deps {tests_arg}--recursive patchelf\n" - ) + system_deps_cmd += f" && {sudo_arg}{getdepscmd}{allow_sys_arg} install-system-deps {tests_arg}--recursive patchelf" + out.write(f" run: {system_deps_cmd}\n") + required_locales = manifest.get( "github.actions", "required_locales", ctx=manifest_ctx ) @@ -1152,6 +1180,18 @@ def write_job_for_platform(self, platform, args): # noqa: C901 out.write(f" - name: Ensure {loc} locale present\n") out.write(f" run: {sudo_arg}locale-gen {loc}\n") + out.write(" - id: paths\n") + out.write(" name: Query paths\n") + if build_opts.is_windows(): + out.write( + f" run: {getdepscmd}{allow_sys_arg} query-paths {tests_arg}--recursive --src-dir=. {manifest.name} >> $env:GITHUB_OUTPUT\n" + ) + out.write(" shell: pwsh\n") + else: + out.write( + f' run: {getdepscmd}{allow_sys_arg} query-paths {tests_arg}--recursive --src-dir=. {manifest.name} >> "$GITHUB_OUTPUT"\n' + ) + projects = loader.manifests_in_dependency_order() main_repo_url = manifest.get_repo_url(manifest_ctx) @@ -1178,25 +1218,32 @@ def write_job_for_platform(self, platform, args): # noqa: C901 ctx = loader.ctx_gen.get_context(m.name) if m.get_repo_url(ctx) != main_repo_url: out.write(" - name: Fetch %s\n" % m.name) + out.write( + f" if: ${{{{ steps.paths.outputs.{m.name}_SOURCE }}}}\n" + ) out.write( f" run: {getdepscmd}{allow_sys_arg} fetch --no-tests {m.name}\n" ) for m in projects: - if m != manifest: - if m.name == "rust": - continue - else: - src_dir_arg = "" - ctx = loader.ctx_gen.get_context(m.name) - if main_repo_url and m.get_repo_url(ctx) == main_repo_url: - # Its in the same repo, so src-dir is also . - src_dir_arg = "--src-dir=. " - has_same_repo_dep = True - out.write(" - name: Build %s\n" % m.name) - out.write( - f" run: {getdepscmd}{allow_sys_arg} build {build_type_arg}{src_dir_arg}{free_up_disk}--no-tests {m.name}\n" - ) + if m == manifest or m.name == "rust": + continue + src_dir_arg = "" + ctx = loader.ctx_gen.get_context(m.name) + if main_repo_url and m.get_repo_url(ctx) == main_repo_url: + # Its in the same repo, so src-dir is also . + src_dir_arg = "--src-dir=. " + has_same_repo_dep = True + + out.write(" - name: Build %s\n" % m.name) + if not src_dir_arg: + # only run the step if needed + out.write( + f" if: ${{{{ steps.paths.outputs.{m.name}_SOURCE }}}}\n" + ) + out.write( + 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) @@ -1213,12 +1260,8 @@ def write_job_for_platform(self, platform, args): # noqa: C901 if has_same_repo_dep: no_deps_arg = "--no-deps " - no_tests_arg = "" - if not run_tests: - no_tests_arg = "--no-tests " - out.write( - f" run: {getdepscmd}{allow_sys_arg} build {build_type_arg}{no_tests_arg}{no_deps_arg}--src-dir=. {manifest.name} {project_prefix}\n" + f" run: {getdepscmd}{allow_sys_arg} build {build_type_arg}{tests_arg}{no_deps_arg}--src-dir=. {manifest.name} {project_prefix}\n" ) out.write(" - name: Copy artifacts\n") diff --git a/build/fbcode_builder/getdeps/fetcher.py b/build/fbcode_builder/getdeps/fetcher.py index c769c118..a1737089 100644 --- a/build/fbcode_builder/getdeps/fetcher.py +++ b/build/fbcode_builder/getdeps/fetcher.py @@ -244,7 +244,9 @@ def __init__(self, build_options, manifest, repo_url, rev, depth) -> None: if not m: raise Exception("Failed to parse rev from %s" % hash_file) rev = m.group(1) - print("Using pinned rev %s for %s" % (rev, repo_url)) + print( + "Using pinned rev %s for %s" % (rev, repo_url), file=sys.stderr + ) self.rev = rev or "main" self.origin_repo = repo_url