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
boost: Do not force numpy support from 1.65 #33359
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.
Instead of doing a mass rebuild, we can just change the fallback value like so. I think that would be better?
@@ -9,6 +9,4 @@ callPackage ./generic.nix (args // rec { | |||
sha256 = "9807a5d16566c57fd74fb522764e0b134a8bbe6b6e8967b83afefd30dcd3be81"; | |||
}; | |||
|
|||
enableNumpy = args.enableNumpy or true; |
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.
enableNumpy = args.enableNumpy or
stdenv.hostPlatform == stdenv.buildPlatform;`
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.
Yes could do this but it is not ideal: the same logic is repeated in the generic.nix and for each version. And the proper logic would also take into account if enablePython is passed into args: if that is false, enableNumpy should be false too.
Ideally it could all be handled in generic.nix?
@@ -9,6 +9,4 @@ callPackage ./generic.nix (args // rec { | |||
sha256 = "5721818253e6a0989583192f96782c4a98eb6204965316df9f5ad75819225ca9"; | |||
}; | |||
|
|||
enableNumpy = args.enableNumpy or true; |
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.
diito
(enableNumpy = args.enableNumpy or
stdenv.hostPlatform == stdenv.buildPlatform;`)
810c4be
to
aaafcdc
Compare
@@ -11,7 +11,7 @@ | |||
, enableShared ? !(hostPlatform.libc == "msvcrt") # problems for now | |||
, enableStatic ? !enableShared | |||
, enablePython ? hostPlatform == buildPlatform | |||
, enableNumpy ? false | |||
, enableNumpy ? enablePython && stdenv.lib.versionAtLeast version "1.65" |
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 points in https://github.com/NixOS/nixpkgs/pull/33359/files#r15934008 . How does this look?
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.
This more directly implements 19dbfb6's description "In order to manipulate Python arrays numpy is needed from boost 1.65 on".
The reason is that if cross compiling (or for other reasons) python bindings as a whole are turned off. Those two lines then trigger assertion errors unless manually overridden for cross compilation. This way: 1. The `enableNumpy` default respects the `enablePython deafult. 2. Cross works by default 3. Absurd manual overrides still break as they should 4. The `>= 1.65` logic is direct and not a maintaince gotcha.
aaafcdc
to
00b038a
Compare
Waited for @GrahamcOfBorg to confirm no rebuilds. |
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!
That’s great, thanks
From: Frederik Rietdijk [mailto:notifications@github.com]
Sent: 03 January 2018 07:38
To: NixOS/nixpkgs <nixpkgs@noreply.github.com>
Cc: bnikolic <bojan@bnikolic.co.uk>; Mention <mention@noreply.github.com>
Subject: Re: [NixOS/nixpkgs] boost: Do not force numpy support from 1.65 (#33359)
@FRidh commented on this pull request.
Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#33359 (review)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AAYienNQP7SlCHdkyewTPcNZ71QwYX_sks5tGy5qgaJpZM4RRLqg> .
|
Motivation for this change
The reason is that if cross compiling (or for other reasons) python bindings as a whole are turned off. Those two lines then trigger assertion errors unless manually overridden for cross compilation.
This way:
enableNumpy
default respects the `enablePython deafult.>= 1.65
logic is direct and not a maintaince gotcha.Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)