-
-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
kitty: fix aarch64-darwin build #153450
kitty: fix aarch64-darwin build #153450
Conversation
@@ -72,6 +72,10 @@ buildPythonApplication rec { | |||
# Causes build failure due to warning | |||
hardeningDisable = lib.optional stdenv.cc.isClang "strictoverflow"; | |||
|
|||
preBuild = lib.optionalString (lib.versionAtLeast stdenv.hostPlatform.darwinMinVersion "11" && stdenv.isDarwin) '' | |||
MACOSX_DEPLOYMENT_TARGET=10.16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this has any effect? You are not using export
here so it would be only accessible to bash but no child processes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All I can say is that without it the build doesn't work on my aarch64-darwin
machine and there are other packages that use the same approach in preBuild
and preConfigure
. I don't know if the fact that this works is accidental or not.
There is a different PR that also aims to fix this but in a different way: #137512. It was was also never merged. |
I actually saw this PR but I must've forgotten about it since then. If the other PR does it in a proper way I will happily close this one. |
I think the other PR is more future-proof. I don't know why it's not being merged though. |
I'll close this one again then |
Motivation for this change
I don't want to steal someone else's work but I was about to create a PR to bump
kitty
to0.24.0
but then I was reminded that it doesn't even build on myaarch64-darwin
machine. But someone else had already fixed that, the PR was just never merged and eventually closed by the creator. I tried the fix on my machine and it works, so I thought I'd create a new PR for it. Feel free to close mine and re-open yours or I can also make you co-author, or main author (if that's possible) of the commit. @bobrikThings done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes