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

llvmPackages_16.libunwind: fix aarch64-android build #321586

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexfmpe
Copy link
Member

@alexfmpe alexfmpe commented Jun 21, 2024

Description of changes

@axelkar @reckenrode

Ran into some failures when trying to build pkgsCross.aarch64-android.hello or anything really.
I don't know the repercussions of any of this, mostly took things from comments in #269077

Should this target staging or so even being android specific?

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@alexfmpe
Copy link
Member Author

Seems to also fix the non-prebuild version of #147084

@rrbutani rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label Jun 21, 2024
@github-actions github-actions bot removed the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label Jun 22, 2024
Copy link
Contributor

@paparodeo paparodeo left a comment

Choose a reason for hiding this comment

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

I don't know the repercussions of any of this, mostly took things from comments in #269077

i think a better description on why this is needed and what the repercussions are needed before this gets committed.

Comment on lines +70 to +71
LDFLAGS = "-unwindlib=none";

Copy link
Contributor

Choose a reason for hiding this comment

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

this seems out of place -- most / all options for the llvm toolchains are selected via cmake defines. why is this one different, and why is it ok for it to be unconditional. there needs to be a comment providing why this is required.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think a comment explaining this would be great. I've been trying to use nixpkgs-review but on every system I try it crashes heh.

@axelkar
Copy link
Contributor

axelkar commented Jun 24, 2024

Buiding hello works, but it doesn't run on an Android device.

$ file ./result/bin/hello
./result/bin/hello: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /system/bin/linker64, not stripped

# on Android device
/data/local/tmp $ ./hello
CANNOT LINK EXECUTABLE "./hello": library "libunwind.so.1" not found: needed by main executable

Edit: Building .\#pkgsCross.aarch64-android-prebuilt.hello works and runs:

$ file result/bin/hello 
result/bin/hello: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /system/bin/linker64, not stripped

# on Android device
/data/local/tmp $ ./hello
Hello, world!

@FliegendeWurst FliegendeWurst added 6.topic: exotic Exotic hardware or software platform 2.status: needs-changes This PR needs changes by the author labels Dec 12, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: needs-changes This PR needs changes by the author 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: exotic Exotic hardware or software platform 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants