Skip to content

Commit

Permalink
fix mononoke linux local cargo build with lld (facebookincubator#6923)
Browse files Browse the repository at this point in the history
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: facebook/sapling#697

Test Plan:
Install the new lld system dep with `sudo ./build/fbcode_builder/getdeps.py install-system-deps --recursive mononoke`

Run a build locally with `./build/fbcode_builder/getdeps.py --allow-system-packages build --src-dir=. mononoke`

Before,  zstd link errors like https://github.com/facebook/sapling/actions/runs/5432912652/jobs/9880272333:
```
          /usr/bin/ld: /home/alex/local/tmp/toolbox/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/build/mononoke/debug/deps/libzstd_sys-837ebc89eb1d31b6.rlib(zstd_lazy.o): in function `ZSTD_compressBlock_btlazy2_extDict':
          zstd_lazy.c:(.text.ZSTD_compressBlock_btlazy2_extDict+0x0): multiple definition of `ZSTD_compressBlock_btlazy2_extDict'; /home/alex/local/tmp/toolbox/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/build/mononoke/debug/deps/libzstd_legacy_mononoke_sys-78a66a56bd363eb2.rlib(zstd_lazy.o):zstd_lazy.c:(.text.ZSTD_compressBlock_btlazy2_extDict+0x0): first defined here
          collect2: error: ld returned 1 exit status
```

After, works:
```
   Compiling mononoke v0.1.0 (/home/alex/local/tmp/toolbox/fbcode_builder_getdeps-ZhomeZalexZlocalZsaplingZbuildZfbcode_builder/build/mononoke/source/eden/mononoke/server)
    Finished dev [unoptimized] target(s) in 1m 17s
```

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/sapling/pull/697).
* facebook/sapling#693
* facebook/sapling#696
* facebook/sapling#692
* facebook/sapling#691
* facebook/sapling#682
* facebook/sapling#689
* __->__ facebook/sapling#697
* facebook/sapling#706
* facebook/sapling#730

Differential Revision: D49875277

Pulled By: genevievehelsel
  • Loading branch information
ahornby authored and facebook-github-bot committed Oct 5, 2023
1 parent 0de536f commit 33bf894
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 54 deletions.
122 changes: 69 additions & 53 deletions build/fbcode_builder/getdeps/cargo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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 = '''{}'''
Expand All @@ -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
Expand Down Expand Up @@ -198,45 +210,59 @@ 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"
[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.
See https://doc.rust-lang.org/cargo/reference/manifest.html#the-patch-section
"""
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]
Expand All @@ -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):
"""
Expand Down Expand Up @@ -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

Expand Down
13 changes: 13 additions & 0 deletions build/fbcode_builder/manifests/lld
Original file line number Diff line number Diff line change
@@ -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

6 changes: 5 additions & 1 deletion build/fbcode_builder/manifests/mononoke
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -47,5 +48,8 @@ fb303
fbthrift
rust-shed

[dependencies.os=linux]
lld

[dependencies.fb=on]
rust

0 comments on commit 33bf894

Please sign in to comment.