From 4383ed326b2735e92a177493072c046ad55046c3 Mon Sep 17 00:00:00 2001 From: Alex Hornby Date: Thu, 5 Oct 2023 16:18:21 -0700 Subject: [PATCH] fix mononoke linux local cargo build with lld Summary: fix mononoke linux local cargo build with lld During my recent reviewstack changes I noticed that mononoke links fine on macOS, but github PRs are failing on linux with duplicate zstd symbols during linking. To fix this, switch to lld for the final link on linux by setting the cargo options, which needs a getdeps cargo.py change to ensure they aren't overwritten It works locally but looks like its running our of disk space on github. Next change in stack addresses github CI diskspace X-link: https://github.com/facebook/sapling/pull/697 Differential Revision: https://internalfb.com/D49875277 fbshipit-source-id: 743538bf2605db2c21c73c855e3cb367ccd77bed --- build/fbcode_builder/getdeps/cargo.py | 122 ++++++++++++++---------- build/fbcode_builder/manifests/lld | 13 +++ build/fbcode_builder/manifests/mononoke | 6 +- 3 files changed, 87 insertions(+), 54 deletions(-) create mode 100644 build/fbcode_builder/manifests/lld diff --git a/build/fbcode_builder/getdeps/cargo.py b/build/fbcode_builder/getdeps/cargo.py index 09e00a39cf984..bb41cc3e294b1 100644 --- a/build/fbcode_builder/getdeps/cargo.py +++ b/build/fbcode_builder/getdeps/cargo.py @@ -64,7 +64,10 @@ def manifest_dir(self, manifest): def recreate_dir(self, src, dst) -> None: if os.path.isdir(dst): - shutil.rmtree(dst) + if os.path.islink(dst): + os.remove(dst) + else: + shutil.rmtree(dst) shutil.copytree(src, dst) def cargo_config_file(self): @@ -80,10 +83,18 @@ def _create_cargo_config(self): if not os.path.isdir(cargo_config_dir): os.mkdir(cargo_config_dir) - print(f"Writing cargo config for {self.manifest.name} to {cargo_config_file}") - with open(cargo_config_file, "w+") as f: - f.write( - """\ + dep_to_git = self._resolve_dep_to_git() + + if os.path.isfile(cargo_config_file): + with open(cargo_config_file, "r") as f: + print(f"Reading {cargo_config_file}") + cargo_content = f.read() + else: + cargo_content = "" + + new_content = cargo_content + if "# Generated by getdeps.py" not in cargo_content: + new_content += """\ # Generated by getdeps.py [build] target-dir = '''{}''' @@ -92,24 +103,25 @@ def _create_cargo_config(self): debug = false incremental = false """.format( - self.build_dir.replace("\\", "\\\\") - ) + self.build_dir.replace("\\", "\\\\") ) # Point to vendored sources from getdeps manifests - dep_to_git = self._resolve_dep_to_git() for _dep, git_conf in dep_to_git.items(): if "cargo_vendored_sources" in git_conf: - with open(cargo_config_file, "a") as f: - vendored_dir = git_conf["cargo_vendored_sources"].replace( - "\\", "\\\\" - ) - f.write( - f""" -[source."{git_conf["repo_url"]}"] -directory = "{vendored_dir}" -""" - ) + vendored_dir = git_conf["cargo_vendored_sources"].replace("\\", "\\\\") + override = ( + f'[source."{git_conf["repo_url"]}"]\ndirectory = "{vendored_dir}"\n' + ) + if override not in cargo_content: + new_content += override + + if new_content != cargo_content: + with open(cargo_config_file, "w") as f: + print( + f"Writing cargo config for {self.manifest.name} to {cargo_config_file}" + ) + f.write(new_content) if self.build_opts.fbsource_dir: # Point to vendored crates.io if possible @@ -198,23 +210,26 @@ 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: + 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() - if "[package]" not in manifest_content: - # A fake manifest has to be crated to change the virtual - # manifest into a non-virtual. The virtual manifests are limited - # in many ways and the inability to define patches on them is - # one. Check https://github.com/rust-lang/cargo/issues/4934 to - # see if it is resolved. - null_file = "/dev/null" - if self.build_opts.is_windows(): - null_file = "nul" - f.write( - f""" + if os.path.isfile(patch_cargo): + with open(patch_cargo, "r") as f: + manifest_content = f.read() + else: + manifest_content = "" + + new_content = manifest_content + if "[package]" not in manifest_content: + # A fake manifest has to be crated to change the virtual + # manifest into a non-virtual. The virtual manifests are limited + # in many ways and the inability to define patches on them is + # one. Check https://github.com/rust-lang/cargo/issues/4934 to + # see if it is resolved. + null_file = "/dev/null" + if self.build_opts.is_windows(): + null_file = "nul" + new_content += f""" [package] name = "fake_manifest_of_{self.manifest.name}" version = "0.0.0" @@ -222,12 +237,25 @@ def _patchup_workspace(self, dep_to_git) -> None: [lib] path = "{null_file}" """ + 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("\\", "\\\\"), ) - else: - f.write("\n") - f.write(config) - - def _resolve_config(self, dep_to_git) -> str: + 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)) + new_content += "\n".join(config) + if new_content != manifest_content: + with open(patch_cargo, "w") as f: + print(f"writing patch to {patch_cargo}") + f.write(new_content) + + def _resolve_config(self, dep_to_git) -> typing.Dict[str, typing.Dict[str, str]]: """ Returns a configuration to be put inside root Cargo.toml file which patches the dependencies git code with local getdeps versions. @@ -235,8 +263,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] @@ -257,17 +283,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): """ @@ -382,7 +398,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 diff --git a/build/fbcode_builder/manifests/lld b/build/fbcode_builder/manifests/lld new file mode 100644 index 0000000000000..39f5b095213ef --- /dev/null +++ b/build/fbcode_builder/manifests/lld @@ -0,0 +1,13 @@ +[manifest] +name = lld + +[debs] +lld + +[rpms] +lld + +# We use the system lld where needed on linux and default linker elsewhere +[build] +builder = nop + diff --git a/build/fbcode_builder/manifests/mononoke b/build/fbcode_builder/manifests/mononoke index 2508b4e842c47..9df5bbd484e0f 100644 --- a/build/fbcode_builder/manifests/mononoke +++ b/build/fbcode_builder/manifests/mononoke @@ -5,7 +5,7 @@ shipit_project = eden shipit_fbcode_builder = true [git] -repo_url = https://github.com/facebookexperimental/eden.git +repo_url = https://github.com/facebook/sapling.git [build.not(os=windows)] builder = cargo @@ -17,6 +17,7 @@ builder = nop [cargo] build_doc = true workspace_dir = eden/mononoke +cargo_config_file = source/eden/mononoke/.cargo/config [shipit.pathmap] fbcode/configerator/structs/scm/hg = configerator/structs/scm/hg @@ -47,5 +48,8 @@ fb303 fbthrift rust-shed +[dependencies.os=linux] +lld + [dependencies.fb=on] rust