Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(nix): create si-fs-standalone Nix pkg for a Nix-free binary #5472

Merged
merged 4 commits into from
Feb 13, 2025

Conversation

fnichol
Copy link
Contributor

@fnichol fnichol commented Feb 13, 2025

This change introduces a new Nix derivation helper
(standaloneBinaryDerivation) which uses a binDerivation-built
package and prepares its main binary to be runnable without the full Nix
package closure.

Note that at the moment, this very much depends on the
dynamic libraries at play and whether or not the executing system has
the correct versions. At the moment, we're targetting the si-fs binary
which only depends on libc and libgcc. Future work will attempt to
make these binaries more flexible and portable where possible.

This change introduces a new Nix derivation helper
(`standaloneBinaryDerivation`) which uses a `binDerivation`-built
package and prepares its main binary to be runnable without the full Nix
package closure.

Note that at the moment, this very much depends on the
dynamic libraries at play and whether or not the executing system has
the correct versions. At the moment, we're targetting the `si-fs` binary
which only depends on `libc` and `libgcc`. Future work will attempt to
make these binaries more flexible and portable where possible.

Signed-off-by: Fletcher Nichol <[email protected]>
Copy link

Dependency Review

✅ No vulnerabilities or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

@fnichol
Copy link
Contributor Author

fnichol commented Feb 13, 2025

I was testing this directly with nix build -L .#si-fs-standalone which builds the new .#si-fs package and then uses that to build the .#si-fs-standalone package, which you can check out in ./result/bin/si-fs and see the manipulated ELF headers with:

readelf -ld ./result/bin/si-fs

or possibly with some more helpful calls to patchelf:

# should report a linker under /lib64 and not /nix/store
patchelf --print-interpreter ./result/bin/si-fs

# should report the typical libc/libm/libgcc/ld-linux gang with nothing extra
patchelf --print-needed ./result/bin/si-fs

# should be empty/not set
patchelf --print-rpath ./result/bin/si-fs

@fnichol
Copy link
Contributor Author

fnichol commented Feb 13, 2025

The merge-queue pipeline will rebuild all artifacts and should be reasonable proof that everything remains buildable as before. The changes here didn't change existing behavior although there were formatting updates.

Comment on lines +131 to +152
postFixup =
""
+ pkgs.lib.optionalString (pkgs.stdenv.isDarwin) ''
nix_lib="$(otool -L "$out/bin/$name" \
| grep libiconv.dylib \
| awk '{print $1}'
)"
install_name_tool \
-change \
"$nix_lib" \
/usr/lib/libiconv.2.dylib \
"$out/bin/$name" \
2>/dev/null
''
+ pkgs.lib.optionalString (pkgs.stdenv.isLinux) ''
patchelf \
--set-interpreter "${systemInterpreter}" \
--remove-rpath \
"$out/bin/${bin}"
'';
dontPatchELF = true;
dontAutoPatchELF = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify my understanding, the idea here is to undo the autopatching done in the binDerivation to just use the system's libc bits, yeah? Out of curiostiy, why do we need a separate derivation for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially yeah. The default in Nix is to use patchelf to hardcode the linker path, set RPATH (i.e. where to look for libs), etc., and that's what we want to un-do. At some level I need a way to specify that I only want certain package targets to mangle themselves like this, not all packages (as we are relying on properly built Nix packages in our omnibus packages). At the moment this other derivation is using the output of a prior built package, but I may change this in the future to build is entirely (i.e. perform the buck2 build) but may play with which libs are present at build time (that is, can we provide an older Glibc at build time to help with backwards compat, etc).

And how did I arrive here? I partially resurrected the tricks and strategies used for the old launcher (

si/flake.nix

Lines 241 to 286 in 5534c19

# Builds a standalone binary that is divorced from the Nix build and
# runtime environment
si-binary = buck2Derivation {
pathPrefix = "bin";
pkgName = "si";
stdBuildPhase = ''
# TODO(fnichol): we currently have a dynamically linked library of
# libc++abi being looked for in a `/nix/store` path when this
# binary is compiled on macOS, so for the meantime until this is
# fixed, we're going to perform a Cargo build of the binary (it
# does not suffer from this issue).
cargo build --bin "$name" --release
cp -pv "target/release/$name" "build/$name-$system"
'';
installPhase = ''
mkdir -pv "$out/bin"
cp -pv "build/$name-$system" "$out/bin/$name"
'';
dontPatchELF = true;
dontAutoPatchELF = true;
postFixup =
""
+ pkgs.lib.optionalString (pkgs.stdenv.isDarwin) ''
nix_lib="$(otool -L "$out/bin/$name" \
| grep libiconv.dylib \
| awk '{print $1}'
)"
install_name_tool \
-change \
"$nix_lib" \
/usr/lib/libiconv.2.dylib \
"$out/bin/$name" \
2>/dev/null
''
+ pkgs.lib.optionalString (pkgs.stdenv.isLinux) ''
patchelf \
--set-interpreter "${systemInterpreter}" \
--remove-rpath \
"$out/bin/$name"
''
+ ''
mkdir -pv "$out/tarballs"
tar cvf "$out/tarballs/$name-$system.tar" -C "$out/bin" "$name"
gzip -9 "$out/tarballs/$name-$system.tar"
'';
};
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay. Totally makes sense.

@fnichol fnichol added this pull request to the merge queue Feb 13, 2025
Merged via the queue into main with commit 63067d9 Feb 13, 2025
10 checks passed
@fnichol fnichol deleted the fnichol/si-fs-nix-build branch February 13, 2025 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants