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
libngspice: add darwin to platforms #95513
Conversation
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.
Thanks for fixing the Darwin build! In these cases it's better to use platforms.unix
, since it will make future support for other Unix systems easier.
@@ -21,6 +21,6 @@ stdenv.mkDerivation rec { | |||
homepage = "http://ngspice.sourceforge.net"; | |||
license = with licenses; [ bsd3 gpl2Plus lgpl2Plus ]; # See https://sourceforge.net/p/ngspice/ngspice/ci/master/tree/COPYING | |||
maintainers = with maintainers; [ bgamari ]; | |||
platforms = platforms.linux; | |||
platforms = with platforms; darwin ++ linux; |
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.
platforms = with platforms; darwin ++ linux; | |
platforms = with platforms.unix; |
Simpler and is likely to work with any unix.
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.
Thanks for the swift reply and improvement suggestion. I'm new to nix always keen to learn how things are done in an idiomatic way.
Change applied to PR.
bea7895
to
5b58365
Compare
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.
Thanks! LGTM
Motivation for this change
This change adresses issue #86039 and enables builds on darwin. I don't see why this wasn't enabled in the first place, as libngspice is cross-platform. Homebrew also has macOS-formula for it.
Simply adding platforms.darwin was enough to make it compile for me on macOS 10.15.5. This change should not affect builds on other platforms.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)