curl: 8.5.0 -> 8.6.0, build with libpsl#285295
Conversation
We plan to enable libpsl support on curl, but including valgrind here causes a cyclic dependency on curl. As the tests have been disabled on various platforms already, this is probably the cheapest place to resolve the dependency chain.
And make sure we pass `--without-libpsl` when it is disabled. https://daniel.haxx.se/blog/2024/01/10/psl-in-curl/
mweinelt
added
the
1.severity: security
ofborg
bot
added
10.rebuild-darwin-stdenv
Fixed, I forgot to add the postPatch block :) |
`tests` seem to be something else on darwin, leading to an eval failure
on the instantiation of this test.
```
… while evaluating the attribute 'tests.fetchpatch'
at ./pkgs/tools/networking/curl/default.nix:182:7:
181| withCheck = finalAttrs.finalPackage.overrideAttrs (_: { doCheck = true; });
182| fetchpatch = tests.fetchpatch.simple.override { fetchpatch = (fetchpatch.override { fetchurl = useThisCurl fetchurl; }) // { version = 1; }; };
| ^
183| curlpp = useThisCurl curlpp;
… while calling a functor (an attribute set with a '__functor' attribute)
at ./pkgs/tools/networking/curl/default.nix:182:20:
181| withCheck = finalAttrs.finalPackage.overrideAttrs (_: { doCheck = true; });
182| fetchpatch = tests.fetchpatch.simple.override { fetchpatch = (fetchpatch.override { fetchurl = useThisCurl fetchurl; }) // { version = 1; }; };
| ^
183| curlpp = useThisCurl curlpp;
(stack trace truncated; use '--show-trace' to show the full trace)
error: value is a function while a set was expected
```
|
@ofborg eval |
|
This built successfully for me on x86_64-linux and aarch64-darwin. If you have specific packages you'd like me to test let me know, but I wasn't planning on rebuilding everything to run a review on this. |
|
Check the top post, I built it on all relevant platforms already. |
delroth
added
the
12.approvals: 1
|
Built |
|
Some edge-case doesn't work, apparently and |
|
Not entirely sure where this goes wrong.
|
|
libpsl depends on libunistring and libidn2. Static libraries don't embed dependency information, so you have to use some out-of-band mechanism to find their dependencies; usually libtool or pkg-config. curl is not using pkg-config to find libpsl, so it doesn't pick up the dependencies. |
|
curl does use pkg-config for a bunch of other libraries, so I think it's a case of getting it to do that for libpsl as well. |
|
I'm talking to upstream, to get the ball rolling on pkg-config support. In the meantime I tried the following, but it fails the same. Not really my area of expertise I must admit. diff --git a/pkgs/tools/networking/curl/default.nix b/pkgs/tools/networking/curl/default.nix
index 9cbf6dc1b0fb..71af49be852a 100644
--- a/pkgs/tools/networking/curl/default.nix
+++ b/pkgs/tools/networking/curl/default.nix
@@ -19,7 +19,7 @@
, idnSupport ? false, libidn2
, ldapSupport ? false, openldap
, opensslSupport ? zlibSupport, openssl
-, pslSupport ? false, libpsl
+, pslSupport ? false, libpsl, libunistring
, rtmpSupport ? false, rtmpdump
, scpSupport ? zlibSupport && !stdenv.isSunOS && !stdenv.isCygwin, libssh2
, wolfsslSupport ? false, wolfssl
@@ -145,6 +145,11 @@ stdenv.mkDerivation (finalAttrs: {
CXX = "${stdenv.cc.targetPrefix}c++";
CXXCPP = "${stdenv.cc.targetPrefix}c++ -E";
+ NIX_CFLAGS_COMPILE = lib.optionalString (pslSupport && stdenv.hostPlatform.isStatic)
+ "-I${lib.getDev libunistring}/include -I${lib.getDev libidn2}/include";
+ NIX_CFLAGS_LINK = lib.optionalString (pslSupport && stdenv.hostPlatform.isStatic)
+ "-L${lib.getLib libunistring}/lib -L${lib.getLib libidn2}/lib";
+
# takes 14 minutes on a 24 core and because many other packages depend on curl
# they cannot be run concurrently and are a bottleneck
# tests are available in passthru.tests.withCheckI also tried adding libpsl itself to cflags, no dice. |
|
The search paths ( Can you link to discussion with upstream? |
|
The discussion happened on IRC. They are adding pkg-config support to their todo in curl/curl#12919. |
|
Do you have any insight into how likely things from the curl TODO are to end up being done? Having a NIX_LDFLAGS hack around is not great for maintainability, because dependencies of libraries can change. I guess for curl we'd probably at least notice via nixStatic though… |
|
Same error with: NIX_CFLAGS_COMPILE = lib.optionalString (pslSupport && stdenv.hostPlatform.isStatic)
"-I${lib.getDev libpsl}/include -I${lib.getDev libunistring}/include -I${lib.getDev libidn2}/include";
NIX_LDFLAGS = lib.optionalString (pslSupport && stdenv.hostPlatform.isStatic)
"-L${lib.getLib libpsl}/lib -L${lib.getLib libunistring}/lib -L${lib.getLib libidn2}/lib -lunistring -lidn2";
It looks like things are regularly removed from the TODO, because they've gotten implemented. I do not have a timeline or an idea how long this could take. I agree that this situation is less than ideal. The alternative would be to skip libpsl on static builds for now. |
Link: NixOS#285295 (comment) Fixes: 996b4eb ("curl: build with public suffix list support")
|
This also brings Python into the curl closure: ❯ grep -R sxr2igfkwhxbagri49b8krmcqz168sim-python3-3.11.8 /nix/store/nwfsn1in46wmgn6ssm2j8famwis5yisz-libpsl-0.21.5 |
|
Like this? #292260 |
8 participants
Description of changes
Updates curl to 8.6.0
Enables libpsl on
curl.Disables testing libpsl with valgrind, easiest way to resolve cyclic dependencies.
Status: Running build tests
Things done
curlFullandcurl.testscurlFullandcurl.testscurlcurlFullandcurl.testsw/ofetchpatch, which does not evalnix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.