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

Fix strip on non-x86 targets #9

Merged
merged 1 commit into from
Sep 29, 2021
Merged

Fix strip on non-x86 targets #9

merged 1 commit into from
Sep 29, 2021

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Sep 27, 2021

There are dedicated stripping tools for architectures other than x86. If they are installed, use them; otherwise, skip strip.

Fixes #8

cc @Shadow53

@Shadow53
Copy link

This looks like it would do the job! Thanks for being so quick about this.

A question, out of curiosity: why run *-strip --version instead of something like which *-strip or I think command -q strip (running the program vs just searching for it)?

@Shadow53
Copy link

I am trying this branch in my CI to test, and I am still getting the same error:

 strip: Unable to recognise the format of the input file `hoard'

This is with the following Rust toolchains:

  • aarch64-unknown-linux-gnu
  • aarch64-unknown-linux-musl
  • arm-unknown-linux-gnueabihf
  • arm-unknown-linux-musleabihf

Notably, the following successfully built:

  • aarch64-linux-android
  • arm-linux-androideabi
  • armv7-linux-androideabi

It looks like the successful ones were not stripped, as aarch64-linux-android does not match the pattern aarch64*-unknown-linux-* and I did not have the right cross-compilation tools installed for the other two.

I will update with another comment if I figure anything else out.

main.sh Outdated
strip="arm-none-eabi-strip"
fi
;;
aarch64*-unknown-linux-*)

Choose a reason for hiding this comment

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

The Android aarch64 target does not contain -unknown

@Shadow53
Copy link

Okay, I think I found the issue, You set strip="..." in the case statement, but around line 111 you call strip instead of $strip. Change it to the latter, and I think the issue I saw will go away.

@taiki-e
Copy link
Owner Author

taiki-e commented Sep 29, 2021

Thanks @Shadow53! I've applied both fixes.

@taiki-e taiki-e force-pushed the strip branch 4 times, most recently from a0c315f to 4b31686 Compare September 29, 2021 13:13
@taiki-e
Copy link
Owner Author

taiki-e commented Sep 29, 2021

A question, out of curiosity: why run *-strip --version instead of something like which *-strip or I think command -q strip (running the program vs just searching for it)?

I tried to choose the one that worked most reliably because their subtle differences in behavior are complicated (koalaman/shellcheck#1162), but in fact strip --version did not work on macos, so I switched to which in latest commit.

@taiki-e
Copy link
Owner Author

taiki-e commented Sep 29, 2021

I've tested that these work (https://github.com/taiki-e/test/actions/runs/1286975387). merging

@taiki-e taiki-e merged commit 112c19d into main Sep 29, 2021
@taiki-e taiki-e deleted the strip branch September 29, 2021 13:38
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.

Cannot strip ARM binaries
2 participants