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

gcc: remove aligned_alloc hack on Darwin #226089

Closed
wants to merge 1 commit into from

Conversation

eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Apr 13, 2023

In a similar vein as PR #226048 this hack doesn't seem necessary anymore and introduces build differences for cross-compilation depending on build platform.

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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.

In a similar vein as PR NixOS#226048 this hack doesn't seem necessary anymore
and introduces build differences across linux/darwin.
@eliasnaur eliasnaur requested a review from matthewbauer as a code owner April 13, 2023 22:59
@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Apr 13, 2023
@wegank
Copy link
Member

wegank commented Apr 14, 2023

Really? aligned_alloc isn't available on macOS 10.12. Have you tried to build stdenv on x86_64-darwin?

@eliasnaur
Copy link
Contributor Author

eliasnaur commented Apr 14, 2023

Really? aligned_alloc isn't available on macOS 10.12. Have you tried to build stdenv on x86_64-darwin?

I built

nix build github:eliasnaur/nixpkgs/master#stdenv

successfully. https://github.com/eliasnaur/nixpkgs includes this PR: eliasnaur@b420b22.

@wegank
Copy link
Member

wegank commented Apr 14, 2023

Oh, sorry, I meant gcc:

$ nix build github:eliasnaur/nixpkgs/master#gcc --system x86_64-darwin

@eliasnaur
Copy link
Contributor Author

eliasnaur commented Apr 15, 2023

Yes, this:

$ nix build github:eliasnaur/nixpkgs/master#gcc

completed successfully on an x86_64-darwin macbook running macOS 13.2.1.

@wegank
Copy link
Member

wegank commented Apr 15, 2023

Nice.

@trofi
Copy link
Contributor

trofi commented Apr 15, 2023

If I understand the comment the idea is to unconditionally disable the feature to make sure the final result does not depend on macos host. Do all macos versions now implement aligned_malloc? Do we use hermetic headers now? (AFAIU we don't). Or will we start requiring aligned_malloc for all macos users who pull from hydra?

@eliasnaur
Copy link
Contributor Author

If I understand the comment the idea is to unconditionally disable the feature to make sure the final result does not depend on macos host. Do all macos versions now implement aligned_malloc? Do we use hermetic headers now? (AFAIU we don't). Or will we start requiring aligned_malloc for all macos users who pull from hydra?

Good points. I prepared #226290 which is a compromise: non-cross gcc should continue to run on macOS 10.12, but cross-gcc would produce the same output as Linux gcc (but require macOS 10.15 to run, something newer to build).

I must admit I'm surprised this is even an issue: why doesn't a cross gcc use the target standard library/headers for checking aligned_alloc? Is that a (fundamental) gcc problem or a nixpkgs problem? If the latter, I'd love to help fixing this properly.

@trofi
Copy link
Contributor

trofi commented Apr 15, 2023

If I understand the comment the idea is to unconditionally disable the feature to make sure the final result does not depend on macos host. Do all macos versions now implement aligned_malloc? Do we use hermetic headers now? (AFAIU we don't). Or will we start requiring aligned_malloc for all macos users who pull from hydra?

Good points. I prepared #226290 which is a compromise: non-cross gcc should continue to run on macOS 10.12, but cross-gcc would produce the same output as Linux gcc (but require macOS 10.15 to run, something newer to build).

I must admit I'm surprised this is even an issue: why doesn't a cross gcc use the target standard library/headers for checking aligned_alloc? Is that a (fundamental) gcc problem or a nixpkgs problem? If the latter, I'd love to help fixing this properly.

gcc runs multiple ./configure calls internally with various --build=/--host=/--target= options. Cor compiler itself it uses build's headers and/or host's headers. For target libraries it usually uses target headers.

It has more to do with limited export ac_cv_func_aligned_alloc=no interface that affects all of the ./configure calls at the same time.

@eliasnaur
Copy link
Contributor Author

Superseded by #226290.

@eliasnaur eliasnaur closed this Apr 16, 2023
@eliasnaur eliasnaur deleted the remove-aligned-alloc branch April 16, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants