Skip to content

Commit 72d8c16

Browse files
committed
Use llvm-strip with support for our cross-compilation targets
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).
1 parent 2a071ca commit 72d8c16

File tree

3 files changed

+9
-9
lines changed

3 files changed

+9
-9
lines changed

xbuild/src/command/build.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,12 @@ pub fn build(env: &BuildEnv) -> Result<()> {
9797
std::fs::copy(&lib, unstripped_lib)
9898
.expect("Could not copy lib before stripping its debug symbols");
9999

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

109108
let ndk = env.android_ndk();

xbuild/src/command/doctor.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ impl Default for Doctor {
1818
checks: vec![
1919
Check::new("clang", Some(VersionCheck::new("--version", 0, 2))),
2020
Check::new("clang++", Some(VersionCheck::new("--version", 0, 2))),
21-
Check::new("llvm-ar", None),
21+
Check::new("llvm-ar", Some(VersionCheck::new("--version", 1, 4))),
2222
Check::new("llvm-lib", None),
23+
Check::new("llvm-strip", Some(VersionCheck::new("--version", 2, 4))),
2324
Check::new("llvm-readobj", Some(VersionCheck::new("--version", 1, 4))),
2425
Check::new("lld", Some(VersionCheck::new("-flavor ld --version", 0, 1))),
2526
Check::new("lld-link", Some(VersionCheck::new("--version", 0, 1))),

xbuild/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ mod task;
4040
/// Current NDK version xbuild should use for Android.
4141
///
4242
/// If this version is not available on a users machine xbuild will download it
43-
/// from our releases: ['https://github.com/Traverse-Research/xbuild/releases'].
43+
/// from our releases: https://github.com/Traverse-Research/xbuild/releases.
4444
///
4545
/// The actual version used is determined by whatever the "ubuntu-latest" image has installed.
4646
const ANDROID_NDK_CURRENT: &str = "27.1.12297006";

0 commit comments

Comments
 (0)