Skip to content

Commit

Permalink
Use llvm-strip with support for our cross-compilation targets
Browse files Browse the repository at this point in the history
When stripping was introduced, it used the system `strip` binary from
GNU which isn't usually built to support cross-compilation.  On my
system I am greeted with the following error when `strip` runs:

    strip: Unable to recognise the format of the input file 'xxx/libevolve_android.so'

But obviously no-one checks process status codes -_-, so this command
"silently" continues without modifying the library.  That should have
left us with a functional yet fat/unstripped library.

When this feature was developed the same error message (and unstripped
library) was likely seen, resulting in a search for how to make the
format recognizable and ultimately having `--target=elf64-little` with
`// I'm told this should always be valid for Android, so use this as the
target` above it.  This is not a complete target and lacks the machine
and operating system, resulting in an invalid library that won't run on
Android.  It was removed in hopes of getting a valid binary, but there
is supposedly still an issue with loading the "unstripped?" library
(besides it not actually being stripped).

Just like all other compiler tools and linkers in `xbuild` use `strip`
from LLVM which has cross-compilation support (we already relied on
that when the `.so` was first compiled and linked...).  This infers the
right target from the file and automatically strips everything (i.e.
`--strip-all` is superfluous).
  • Loading branch information
MarijnS95 committed Jan 6, 2025
1 parent 2a071ca commit b41febe
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 9 deletions.
13 changes: 6 additions & 7 deletions xbuild/src/command/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,12 @@ pub fn build(env: &BuildEnv) -> Result<()> {
std::fs::copy(&lib, unstripped_lib)
.expect("Could not copy lib before stripping its debug symbols");

std::process::Command::new("strip")
.arg("--strip-all")
.arg(&lib)
.spawn()
.expect("Could not strip debug symbols from lib")
.wait()
.expect("Stripping of debug symbols from lib failed");
let mut cmd = std::process::Command::new("llvm-strip");
cmd.arg(&lib);
let status = cmd
.status()
.with_context(|| format!("Could not run `{cmd:?}`"))?;
ensure!(status.success(), "Failed to strip library using `{cmd:?}`");
}

let ndk = env.android_ndk();
Expand Down
3 changes: 2 additions & 1 deletion xbuild/src/command/doctor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ impl Default for Doctor {
checks: vec![
Check::new("clang", Some(VersionCheck::new("--version", 0, 2))),
Check::new("clang++", Some(VersionCheck::new("--version", 0, 2))),
Check::new("llvm-ar", None),
Check::new("llvm-ar", Some(VersionCheck::new("--version", 1, 4))),
Check::new("llvm-lib", None),
Check::new("llvm-strip", Some(VersionCheck::new("--version", 2, 4))),
Check::new("llvm-readobj", Some(VersionCheck::new("--version", 1, 4))),
Check::new("lld", Some(VersionCheck::new("-flavor ld --version", 0, 1))),
Check::new("lld-link", Some(VersionCheck::new("--version", 0, 1))),
Expand Down
2 changes: 1 addition & 1 deletion xbuild/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ mod task;
/// Current NDK version xbuild should use for Android.
///
/// If this version is not available on a users machine xbuild will download it
/// from our releases: ['https://github.com/Traverse-Research/xbuild/releases'].
/// from our releases: https://github.com/Traverse-Research/xbuild/releases.
///
/// The actual version used is determined by whatever the "ubuntu-latest" image has installed.
const ANDROID_NDK_CURRENT: &str = "27.1.12297006";
Expand Down

0 comments on commit b41febe

Please sign in to comment.