-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
opencv3: Enable darwin build #25219
opencv3: Enable darwin build #25219
Conversation
@knedlsepp, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bjornfor, @abbradar and @basvandijk to be potential reviewers. |
@@ -147,6 +149,6 @@ stdenv.mkDerivation rec { | |||
homepage = http://opencv.org/; | |||
license = stdenv.lib.licenses.bsd3; | |||
maintainers = with stdenv.lib.maintainers; [viric flosse mdaiter]; | |||
platforms = with stdenv.lib.platforms; linux; | |||
platforms = with stdenv.lib.platforms; [ linux darwin ]; |
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.
I think this needs to be linux ++ darwin
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 we found out recently that having darwin as variable name above break this. So you need to do:
platforms = with stdenv.lib; platforms.linux ++ platforms.darwin;
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.
Yeah, we also generally try to avoid passing in all of darwin
and instead pass in the individual Frameworks we use. There's probably a better way to do this of course 😄
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.
Move darwin.* to pkgs! 😈
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.
Probably makes sense, eventually, but there are some overlaps that are awkward
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.
I feel like we should reach a consensus on inherit vs using the darwin attributeset directly. Neither of them has a clear advantage, but I would prefer one of these to be used more prominently throughout nixpkgs.
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.
Pushed your suggestions.
cc30c0d
to
68c6e5b
Compare
Motivation for this change
This should make opencv3 build on darwin.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)