-
-
Notifications
You must be signed in to change notification settings - Fork 452
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
pycurl: Add curl as a depedency #1284
Conversation
overrides/default.nix
Outdated
@@ -1853,6 +1853,13 @@ lib.composeManyExtensions [ | |||
} | |||
); | |||
|
|||
pycurl = super.pycurl.overridePythonAttrs ( | |||
old: { | |||
propagatedBuildInputs = (old.propagatedBuildInputs or [ ]) ++ [ pkgs.curl ]; |
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 should not need to be propagated—after PycURL is built, the C headers for curl are not needed to use PycURL.
pycurl = super.pycurl.overridePythonAttrs ( | ||
old: { | ||
propagatedBuildInputs = (old.propagatedBuildInputs or [ ]) ++ [ pkgs.curl ]; | ||
nativeBuildInputs = (old.nativeBuildInputs or [ ]) ++ [ pkgs.curl ]; |
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.
Similarly to the above comment. I believe all you need is just specify buildInputs
and add pkgs.curl
there.
nativeBuildInputs
is meant for tooling that are typically tools that are needed during build time. Its purpose is clearer when you do cross compiling. So you would specify there things like cmake
, for python it would be setuptools
, poetry-core
etc.
BTW: I see netcdf4
using propagatedBuildInputs
but that likely is incorrect.
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 may need to be native for the curl-config
script. (But still needn’t be propagated.)
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.
Then the solution is to add curl as both a build input and a native input.
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.
ah, my bad.
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.
Thank you for the review and explanation regarding the build inputs. I'll try to make the changes tomorrow.
overrides/default.nix
Outdated
@@ -1853,6 +1853,13 @@ lib.composeManyExtensions [ | |||
} | |||
); | |||
|
|||
pycurl = super.pycurl.overridePythonAttrs ( | |||
old: { | |||
buildInputs = (old.propagatedBuildInputs or [ ]) ++ [ pkgs.curl ]; |
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.
@bzadm you should also rename old.propagatedBuildInputs
to old.buildInputs
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! Would you also be willing to create tests for this package? If so, see
tests/default.nix
for the test entry and- the
tests/mutmut
directory for a module test ortests/matplotlib-post-3-7
for a library test.
Adds support for pycurl via override. Tested on GNU/Linux with NixOS. Fixes #1283
Test
pyproject.toml
:flake.nix
:test.py
:Test the build and functionality: