-
-
Notifications
You must be signed in to change notification settings - Fork 451
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
override: fix pyarrow #1738
override: fix pyarrow #1738
Conversation
14aa76e
to
9f782b8
Compare
overrides/default.nix
Outdated
@@ -2241,8 +2241,8 @@ lib.composeManyExtensions [ | |||
); | |||
|
|||
ARROW_HOME = _arrow-cpp; | |||
arrowCppVersion = lib.versions.majorMinor _arrow-cpp; | |||
pyArrowVersion = lib.versions.majorMinor prev.pyarrow; | |||
arrowCppVersion = lib.versions.majorMinor (_arrow-cpp.version or _arrow-cpp); |
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 don't understand what this is supposed to do.
The or
case would pass the derivation to majorMinor
, not the version string?
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.
Well, passing the derivation is what the current override apparently does
See
poetry2nix/overrides/default.nix
Line 2247 in 1dcd9fd
arrowCppVersion = lib.versions.majorMinor _arrow-cpp; |
But then, I don't think majorminor ever took a derivation.
So I assumed that in the past, it must have been a version string.
I figured I'd code it defensively, if it has a .version, use that, and if it doesn't it must be the version string to actually work with majorminor, so pass that.
But there's of course a good chance that I misunderstood something about the original code and the nixpgks it lived in.
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 am also hitting this error right now. I think this code never worked in the past. Just change both lines to .version
. Backwards compatibility is not needed.
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.
@TyberiusPrime I would really love to see this merged. Can you remove the or
and just use .version
? I think this PR would be ready to go then.
I just used this PR to try and make my example-flink-job usable on MacOS and things did not work. Some context is that nix run gitlab:usmcamp0811/dotfiles/ed1c16132787a5bb048de61249420962513a50f3#example-flink-job.python and he got this error: nix run --refresh gitlab:usmcamp0811/dotfiles#example-flink-job.python
error:
… while calling the 'derivationStrict' builtin
at /derivation-internal.nix:9:12:
8|
9| strict = derivationStrict drvAttrs;
| ^
10|
… while evaluating derivation 'python3-3.11.9-env'
whose name attribute is located at /nix/store/sj9yrq21wbbfr5715hys3laa2qd6x471-source/pkgs/stdenv/generic/make-derivation.nix:333:7
… while evaluating attribute 'passAsFile' of derivation 'python3-3.11.9-env'
at /nix/store/sj9yrq21wbbfr5715hys3laa2qd6x471-source/pkgs/build-support/trivial-builders/default.nix:69:9:
68| inherit buildCommand name;
69| passAsFile = [ "buildCommand" ]
| ^
70| ++ (derivationArgs.passAsFile or [ ]);
(stack trace truncated; use '--show-trace' to show the full trace)
error: arrow-cpp version (16.0) mismatches pyarrow version (11.0)
otherwise this ran fine on all my Linux boxes... |
The fact that you even get that error message shows that the fix did work. You probably will have to use a wheel though if you need a pyarrow that does not match nixpkgs' arrow-cpp. |
9f782b8
to
6fe8d24
Compare
6fe8d24
to
1f53f2f
Compare
Removed the defensive coding as requested. |
@adisbladis Can you take another look here? |
[ ] There's an automated test for this change
I have trouble building anything with pyarrow against nixpkgs 24.05. Might be a change in lib.versions.majorMinor?
Anyway, this fix should work in both new and old.