Skip to content

Commit

Permalink
lld for mononoke on linux to fix github PR builds
Browse files Browse the repository at this point in the history
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.

Regenerated the actions file to include the change with: ```./build/fbcode_builder/getdeps.py --allow-system-packages generate-github-actions --os-type=linux --src-dir=. --output-dir=.github/workflows --job-name="Mononoke " --job-file-prefix=mononoke_ mononoke
```

There was a small bug in getdeps.py regen that meant it missed the rust declaration so also included a fix for that

Test plan:

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
```
  • Loading branch information
ahornby committed Jul 10, 2023
1 parent dfe8e16 commit 60ab39c
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 17 deletions.
32 changes: 20 additions & 12 deletions .github/workflows/mononoke_linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ jobs:
run: sudo python3 build/fbcode_builder/getdeps.py --allow-system-packages install-system-deps --recursive mononoke
- name: Install Rust Stable
uses: dtolnay/rust-toolchain@stable
- name: Fetch lld
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests lld
- name: Fetch ninja
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests ninja
- name: Fetch cmake
Expand Down Expand Up @@ -59,18 +61,20 @@ jobs:
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests libtool
- name: Fetch libsodium
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests libsodium
- name: Fetch libffi
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests libffi
- name: Fetch ncurses
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests ncurses
- name: Fetch python
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests python
- name: Fetch xz
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests xz
- name: Fetch folly
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests folly
- name: Fetch fizz
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests fizz
- name: Fetch mvfst
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests mvfst
- name: Fetch libffi
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests libffi
- name: Fetch ncurses
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests ncurses
- name: Fetch python
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests python
- name: Fetch wangle
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests wangle
- name: Fetch fbthrift
Expand All @@ -79,6 +83,8 @@ jobs:
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages fetch --no-tests fb303
- 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
- name: Build ninja
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests ninja
- name: Build cmake
Expand Down Expand Up @@ -117,18 +123,20 @@ jobs:
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests libtool
- name: Build libsodium
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests libsodium
- name: Build libffi
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests libffi
- name: Build ncurses
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests ncurses
- name: Build python
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests python
- name: Build xz
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests xz
- name: Build folly
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests folly
- name: Build fizz
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests fizz
- name: Build mvfst
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests mvfst
- name: Build libffi
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests libffi
- name: Build ncurses
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests ncurses
- name: Build python
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests python
- name: Build wangle
run: python3 build/fbcode_builder/getdeps.py --allow-system-packages build --no-tests wangle
- name: Build fbthrift
Expand Down
10 changes: 7 additions & 3 deletions build/fbcode_builder/getdeps.py
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,8 @@ def write_job_for_platform(self, platform, args): # noqa: C901
# We do this by looking at the builder type in the manifest file
# rather than creating a builder and checking its type because we
# don't know enough to create the full builder instance here.
if manifest.get("build", "builder", ctx=manifest_ctx) == "nop":
builder_name = manifest.get("build", "builder", ctx=manifest_ctx)
if builder_name == "nop":
return None

# We want to be sure that we're running things with python 3
Expand Down Expand Up @@ -1039,12 +1040,15 @@ def write_job_for_platform(self, platform, args): # noqa: C901
main_repo_url = manifest.get_repo_url(manifest_ctx)
has_same_repo_dep = False

done_rust = False

for m in projects:
if m != manifest:
if m.name == "rust":
if ( m.name == "rust" or builder_name == "cargo" ) and not done_rust:
out.write(" - name: Install Rust Stable\n")
out.write(" uses: dtolnay/rust-toolchain@stable\n")
else:
done_rust = True
if not m.name == "rust":
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)
Expand Down
13 changes: 11 additions & 2 deletions build/fbcode_builder/getdeps/cargo.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,18 @@ def _create_cargo_config(self):
if not os.path.isdir(cargo_config_dir):
os.mkdir(cargo_config_dir)

dep_to_git = self._resolve_dep_to_git()

if os.path.isfile(cargo_config_file):
with open(cargo_config_file, "r") as f:
content = f.read()
if "# Generated by getdeps.py" in content:
# Don't duplicate generated part
return dep_to_git

print(f"Writing cargo config for {self.manifest.name} to {cargo_config_file}")
with open(cargo_config_file, "w+") as f:
# append so we don't lose any existing config
with open(cargo_config_file, "a+") as f:
f.write(
"""\
# Generated by getdeps.py
Expand All @@ -97,7 +107,6 @@ def _create_cargo_config(self):
)

# 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:
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

4 changes: 4 additions & 0 deletions build/fbcode_builder/manifests/mononoke
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions eden/mononoke/.cargo/config
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# default ld on linux has duplicate symbol error, lld handles it fine. TODO: remove once need for https://github.com/mitrandir77/zstd-rs fork is gone
[target.x86_64-unknown-linux-gnu]
rustflags = ["-C", "link-arg=-fuse-ld=lld"]

0 comments on commit 60ab39c

Please sign in to comment.