fix github disk full by cleaning intermediate build steps and sylinking

Now that linking is fixed in previous change, disk space usage is too large for github actions runner so I see Mononoke build fail on sapling PRs. Fix it by cleaning up intermediate state and symlinking instead of copying. The later exposed a couple of bugs in cargo.py when updating pre-existing workspace file, also fixed. Test Plan: Test locally on ubuntu 22.04 Before: ``` rm -rf ./build/fbcode_builder/getdeps.py --allow-system-packages build --src-dir=. mononoke ... $ du -sh $TMP/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/* Fri Jul 7 18:50:25 BST 2023 17G /home/alex/local/tmp/toolbox/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/build 800K /home/alex/local/tmp/toolbox/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/downloads 3.6M /home/alex/local/tmp/toolbox/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/extracted 5.9G /home/alex/local/tmp/toolbox/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/installed 433M /home/alex/local/tmp/toolbox/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/repos `` After ``` ./build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --src-dir=. mononoke ```
ahornby · Jul 10, 2023 · 21d35c3 · 21d35c3
1 parent 60ab39c
commit 21d35c3
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 54 deletions.
diff --git a/.github/workflows/mononoke_linux.yml b/.github/workflows/mononoke_linux.yml
@@ -84,67 +84,67 @@ jobs:
     - name: Fetch rust-shed
       run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests rust-shed
     - name: Build lld
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests lld
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests lld
     - name: Build ninja
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests ninja
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests ninja
     - name: Build cmake
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests cmake
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests cmake
     - name: Build fmt
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests fmt
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests fmt
     - name: Build googletest
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests googletest
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests googletest
     - name: Build python-six
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests python-six
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests python-six
     - name: Build zstd
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests zstd
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests zstd
     - name: Build boost
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests boost
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests boost
     - name: Build double-conversion
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests double-conversion
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests double-conversion
     - name: Build gflags
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests gflags
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests gflags
     - name: Build glog
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests glog
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests glog
     - name: Build libevent
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests libevent
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests libevent
     - name: Build lz4
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests lz4
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests lz4
     - name: Build snappy
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests snappy
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests snappy
     - name: Build zlib
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests zlib
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests zlib
     - name: Build bz2
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests bz2
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests bz2
     - name: Build autoconf
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests autoconf
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests autoconf
     - name: Build automake
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests automake
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests automake
     - name: Build libtool
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests libtool
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests libtool
     - name: Build libsodium
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests libsodium
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests libsodium
     - name: Build xz
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests xz
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests xz
     - name: Build folly
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests folly
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests folly
     - name: Build fizz
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests fizz
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests fizz
     - name: Build mvfst
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests mvfst
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests mvfst
     - name: Build libffi
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests libffi
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests libffi
     - name: Build ncurses
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests ncurses
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests ncurses
     - name: Build python
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests python
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests python
     - name: Build wangle
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests wangle
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests wangle
     - name: Build fbthrift
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests fbthrift
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests fbthrift
     - name: Build fb303
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests fb303
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests fb303
     - name: Build rust-shed
-      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests rust-shed
+      run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --clean-intermediate --no-tests rust-shed
     - name: Build mononoke
       run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --src-dir=. mononoke  --project-install-prefix mononoke:/usr/local
     - name: Copy artifacts

diff --git a/build/fbcode_builder/getdeps.py b/build/fbcode_builder/getdeps.py
@@ -795,6 +795,12 @@ def setup_project_cmd_parser(self, parser):
             action="store_true",
             default=False,
         )
+        parser.add_argument(
+            "--clean-intermediate",
+            help="Clean up intermediate files if possible to reduce disk usage",
+            action="store_true",
+            default=False,
+        )
 
 
 @cmd("fixup-dyn-deps", "Adjusts dynamic dependencies for packaging purposes")
@@ -1056,6 +1062,15 @@ def write_job_for_platform(self, platform, args):  # noqa: C901
                                 f"      run: {getdepscmd}{allow_sys_arg} fetch --no-tests {m.name}\n"
                             )
 
+            if build_opts.clean_intermediate:
+                # Strip debug info from the binaries, but only on linux.
+                # While the `strip` utility is also available on macOS,
+                # attempting to strip there results in an error.
+                # The `strip` utility is not available on Windows.
+                clean_intermediate = "--clean-intermediate "
+            else:
+                clean_intermediate = ""
+
             for m in projects:
                 if m != manifest:
                     if m.name == "rust":
@@ -1069,7 +1084,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}--no-tests {m.name}\n"
+                            f"      run: {getdepscmd}{allow_sys_arg} build {src_dir_arg}{clean_intermediate}--no-tests {m.name}\n"
                         )
 
             out.write("    - name: Build %s\n" % manifest.name)
@@ -1159,6 +1174,12 @@ def setup_project_cmd_parser(self, parser):
             help="add a prefix to all job names",
             default=None,
         )
+        parser.add_argument(
+            "--clean-intermediate",
+            help="Clean up intermediate files if possible to reduce disk usage",
+            action="store_true",
+            default=False,
+        )
 
 
 def get_arg_var_name(args):

diff --git a/build/fbcode_builder/getdeps/builder.py b/build/fbcode_builder/getdeps/builder.py
@@ -136,6 +136,11 @@ def build(self, install_dirs, reconfigure: bool) -> None:
         self._prepare(install_dirs=install_dirs, reconfigure=reconfigure)
         self._build(install_dirs=install_dirs, reconfigure=reconfigure)
 
+        if self.build_opts.clean_intermediate:
+            # don't clean --src-dir=. case as user may want to build again or run tests on the build
+            if self.src_dir.startswith(self.build_opts.scratch_dir) and os.path.isdir(self.build_dir):
+                shutil.rmtree(self.build_dir)
+
         # On Windows, emit a wrapper script that can be used to run build artifacts
         # directly from the build directory, without installing them.  On Windows $PATH
         # needs to be updated to include all of the directories containing the runtime

diff --git a/build/fbcode_builder/getdeps/buildopts.py b/build/fbcode_builder/getdeps/buildopts.py
@@ -51,6 +51,7 @@ def __init__(
         lfs_path=None,
         shared_libs: bool = False,
         facebook_internal=None,
+        clean_intermediate: bool = False,
     ) -> None:
         """fbcode_builder_dir - the path to either the in-fbsource fbcode_builder dir,
                              or for shipit-transformed repos, the build dir that
@@ -65,6 +66,7 @@ def __init__(
         use_shipit - use real shipit instead of the simple shipit transformer
         vcvars_path - Path to external VS toolchain's vsvarsall.bat
         shared_libs - whether to build shared libraries
+        clean_intermediate - whether to clean up intermediate build files to save runner disk space
         """
 
         if not install_dir:
@@ -103,6 +105,7 @@ def __init__(
         self.allow_system_packages = allow_system_packages
         self.lfs_path = lfs_path
         self.shared_libs = shared_libs
+        self.clean_intermediate = clean_intermediate
 
         lib_path = None
         if self.is_darwin():
@@ -602,6 +605,7 @@ def setup_build_options(args, host_type=None) -> BuildOptions:
             "allow_system_packages",
             "lfs_path",
             "shared_libs",
+            "clean_intermediate"
         }
     }
 

diff --git a/build/fbcode_builder/getdeps/cargo.py b/build/fbcode_builder/getdeps/cargo.py
@@ -63,10 +63,19 @@ def manifest_dir(self, manifest):
         return os.path.join(self.build_source_dir(), manifest)
 
     def recreate_dir(self, src, dst) -> None:
-        if os.path.isdir(dst):
+        if os.path.islink(dst):
+            os.remove(dst)
+        elif os.path.isdir(dst):
             shutil.rmtree(dst)
         shutil.copytree(src, dst)
 
+    def recreate_linked_dir(self, src, dst) -> None:
+        if os.path.islink(dst):
+            os.remove(dst)
+        elif os.path.isdir(dst):
+            shutil.rmtree(dst)
+        os.symlink(src, dst)
+
     def cargo_config_file(self):
         build_source_dir = self.build_dir
         if self.cargo_config_file_subdir:
@@ -135,7 +144,11 @@ def _create_cargo_config(self):
 
     def _prepare(self, install_dirs, reconfigure) -> None:
         build_source_dir = self.build_source_dir()
-        self.recreate_dir(self.src_dir, build_source_dir)
+
+        if self.build_opts.is_windows():
+            self.recreate_dir(self.src_dir, build_source_dir)
+        else:
+            self.recreate_linked_dir(self.src_dir, build_source_dir)
 
         dep_to_git = self._create_cargo_config()
 
@@ -165,8 +178,12 @@ def _build(self, install_dirs, reconfigure) -> None:
                     ],
                 )
 
-        self.recreate_dir(build_source_dir, os.path.join(self.inst_dir, "source"))
-
+        installed_source = os.path.join(self.inst_dir, "source")
+        if self.build_opts.is_windows():
+            self.recreate_dir(build_source_dir, installed_source)
+        else:
+            self.recreate_linked_dir(build_source_dir, installed_source)
+
     def run_tests(
         self, install_dirs, schedule_type, owner, test_filter, retry, no_testpilot
     ) -> None:
@@ -207,9 +224,9 @@ def _patchup_workspace(self, dep_to_git) -> None:
         producing bad results.
         """
         workspace_dir = self.workspace_dir()
-        config = self._resolve_config(dep_to_git)
-        if config:
-            patch_cargo = os.path.join(workspace_dir, "Cargo.toml")
+        git_url_to_crates_and_paths = self._resolve_config(dep_to_git)
+        if git_url_to_crates_and_paths:
+            patch_cargo = os.path.join(workspace_dir, "Cargo.toml")            
             print(f"writing patch to {patch_cargo}")
             with open(patch_cargo, "r+") as f:
                 manifest_content = f.read()
@@ -234,7 +251,19 @@ def _patchup_workspace(self, dep_to_git) -> None:
                     )
                 else:
                     f.write("\n")
-                f.write(config)
+                config = []
+                for git_url, crates_to_patch_path in git_url_to_crates_and_paths.items():
+                    crates_patches = [
+                        '{} = {{ path = "{}" }}'.format(
+                            crate,
+                            crates_to_patch_path[crate].replace("\\", "\\\\"),
+                        )
+                        for crate in sorted(crates_to_patch_path.keys())
+                    ]
+                    patch_key = f'[patch."{git_url}"]'
+                    if patch_key not in manifest_content:
+                        config.append(f'\n{patch_key}\n' + "\n".join(crates_patches))
+                f.write("\n".join(config) + "\n")
 
     def _resolve_config(self, dep_to_git) -> str:
         """
@@ -244,8 +273,6 @@ def _resolve_config(self, dep_to_git) -> str:
         """
         dep_to_crates = self._resolve_dep_to_crates(self.build_source_dir(), dep_to_git)
 
-        config = []
-
         git_url_to_crates_and_paths = {}
         for dep_name in sorted(dep_to_git.keys()):
             git_conf = dep_to_git[dep_name]
@@ -266,17 +293,7 @@ def _resolve_config(self, dep_to_git) -> str:
                 if crates_to_patch_path:
                     git_url_to_crates_and_paths[git_url] = crates_to_patch_path
 
-        for git_url, crates_to_patch_path in git_url_to_crates_and_paths.items():
-            crates_patches = [
-                '{} = {{ path = "{}" }}'.format(
-                    crate,
-                    crates_to_patch_path[crate].replace("\\", "\\\\"),
-                )
-                for crate in sorted(crates_to_patch_path.keys())
-            ]
-            config.append(f'\n[patch."{git_url}"]\n' + "\n".join(crates_patches))
-
-        return "\n".join(config)
+        return git_url_to_crates_and_paths
 
     def _resolve_dep_to_git(self):
         """
@@ -391,7 +408,7 @@ def _resolve_dep_to_crates(self, build_source_dir, dep_to_git):
                                 print(
                                     f"Patch {self.manifest.name} uses {dep_name} crate {crates}"
                                 )
-                                existing_crates.insert(c)
+                                existing_crates.add(c)
                         dep_to_crates.setdefault(name, set()).update(existing_crates)
         return dep_to_crates