Skip to content

Commit

Permalink
skip unnecessary github actions steps
Browse files Browse the repository at this point in the history
Summary:
X-link: facebookincubator/zstrong#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
  • Loading branch information
ahornby authored and facebook-github-bot committed Jan 6, 2025
1 parent 948885a commit a9b28d4
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 35 deletions.
111 changes: 77 additions & 34 deletions build/fbcode_builder/getdeps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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
)
Expand All @@ -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)
Expand All @@ -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)

Expand All @@ -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")
Expand Down
4 changes: 3 additions & 1 deletion build/fbcode_builder/getdeps/fetcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a9b28d4

Please sign in to comment.