-
-
Notifications
You must be signed in to change notification settings - Fork 14.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
WIP: wxwidgets: fix build on darwin #150908
Conversation
@peterhoeg while in some sense the |
Additionally, the reason I only enabled darwin builds on
--enable-universal-binaries being considered an invalid option, while it worked fine for 3.0.
|
@@ -7,8 +7,8 @@ | |||
, libGLU, libGL | |||
, compat24 ? false, compat26 ? true, unicode ? true | |||
, withGtk2 ? true | |||
, withWebKit ? false, webkitgtk | |||
, AGL, Carbon, Cocoa, Kernel, QTKit | |||
, withWebKit ? !stdenv.isDarwin, webkitgtk # Disable webkitgtk on darwin pending #126101 |
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.
Actually, the original value was already false, so setting it to !stdenv.isDarwin
would enable it on non-darwin. And I don't see any override in all-packages.nix
, withWebKit ? !stdenv.isDarwin, webkitgtk # Disable webkitgtk on darwin pending #126101 | |
, withWebKit ? false, webkitgtk |
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.
Good catch. Does that mean that the original version would be preferable (to modify buildInputs to ++ optional (withWebKit && !stdenv.isDarwin) webkitgtk # Disable webkitgtk on darwin pending #126101
)? That way all-packages
doesn't need a bunch of darwin overrides for each way it gets called? If so, can update accordingly.
Usually autotools only prints warnings on seeing unknown options. What is the error that you see? |
Also, just to make sure, you really want |
Error is here:
|
@veprbl I think I may have missed this very obvious solution before embarking on this workaround. I'm not actually sure that GTK is necessary for the use cases I'm envisioning. I need to test dependencies accordingly, but to your point this entire PR might actually be redundant. |
Well, then this option should not passed.
To avoid such confusion, we probably need to add a generic |
In the interest of not over-complicating something that isn't particularly broken or creating yet more churn over something which probably isn't top of the list for ways to make the ecosystem less confusing for newbies, I'm inclined to simply close this PR and leave it as a bit of keyword-appropriate guidance for someone who finds themselves in a similar situation. |
Motivation for this change
darwin packaging depending on wxwidgets fail due to #126101. Rather than forcing all upstream dependencies not needing webkitgtk to exclude it, it may be simpler to exclude here until fixed. Additionally, AVFoundation appears to be needed on my x86 machine to complete a build.
Things 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