Skip to content

Remove -nostdlibinc flag from clang when building for android-prebuilt#389028

Merged
alexfmpe merged 1 commit intoNixOS:masterfrom
alexfmpe:android-no-nostdlib
Apr 30, 2025
Merged

Remove -nostdlibinc flag from clang when building for android-prebuilt#389028
alexfmpe merged 1 commit intoNixOS:masterfrom
alexfmpe:android-no-nostdlib

Conversation

@alexfmpe
Copy link
Member

#356162 caused libffi, gmp, hello and likely others to fail for pkgsCross.aarch64-android-prebuilt with errors about standard headers.

> ../src/closures.c:418:10: fatal error: 'sys/types.h' file not found
> #include <sys/types.h>

Looking at the pre-existing comment, I don't see how my /usr/include would be relevant given I'm building from nixos, so it's likely this is sysroot related, and indeed the standard files can be seen in

ls $(nix-build -A pkgsCross.aarch64-android-prebuilt.buildPackages.androidndkPkgs.binaries)/toolchain/sysroot/usr/include`

IIUC this is a workaround that actually re-introduces the leaking concern for non-nixos, but I don't know what the proper fix is nor why this surfaced on android in particular.

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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 alexfmpe requested a review from emilazy March 11, 2025 17:32
@nix-owners nix-owners bot requested a review from Ericson2314 March 11, 2025 17:33
@alexfmpe
Copy link
Member Author

cc @pwaller (apparenty can't add as reviewer?)

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Mar 11, 2025
@emilazy
Copy link
Member

emilazy commented Mar 11, 2025

Is this the prebuilt Android toolchain? It presumably regressed because it was previously in an LLVM patch, which don’t get applied to prebuilt toolchains.

@alexfmpe alexfmpe changed the title Remove -nostdlibinc flag from clang when building for android Remove -nostdlibinc flag from clang when building for android-prebuilt Mar 11, 2025
Copy link
Contributor

@pwaller pwaller left a comment

Choose a reason for hiding this comment

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

Not knowing much about the android toolchain and how it is wired in nixpkgs, looks like a reasonable fix to me to get things working again at least.

The with -nostdlibinc nix cc wrapper assumes that it is feeding the stdlib paths into the compiler. If that's not the case, as appears here, then -nostdinc is not wanted.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Apr 20, 2025
@alexfmpe alexfmpe merged commit c00afdf into NixOS:master Apr 30, 2025
32 checks passed
@drupol
Copy link
Contributor

drupol commented Apr 30, 2025

This PR broke the linter CI. Going to provide a fix now.

@drupol
Copy link
Contributor

drupol commented Apr 30, 2025

Fix @ #402978

drupol added a commit to drupol/nixpkgs that referenced this pull request Apr 30, 2025
@drupol drupol mentioned this pull request Apr 30, 2025
13 tasks
@drupol
Copy link
Contributor

drupol commented Apr 30, 2025

@alexfmpe

Thank you for the PR, but next time make sure to follow the commit convention and PR title, I think this would have been avoided.

In the meantime, don't worry, I've fixed it.

@alexfmpe
Copy link
Member Author

Oh lint was green when I first opened, didn't notice there were new nix-fmt rules since then.

make sure to follow the commit convention and PR title, I think this would have been avoided.

Do you mean prefixing with cc-wrapper: ? True, I did forget that, but I'm a bit lost on how that relates to the lint issue?

@alexfmpe alexfmpe deleted the android-no-nostdlib branch June 14, 2025 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants