libcurl exhibits poor behaviour when compiled without support for c-ares (USE="adns"), which can (and has) result in very hard to diagnose faults in other packages. Example: https://bugreports.qt.io/browse/QTBUG-79886 Good description of the issue: https://stackoverflow.com/a/10755612/1872867 Reproducible: Sometimes Steps to Reproduce: 1. Compile an application against libcurl without c-ares support 2. Perform DNS lookups which fail 3. Possibility of a crash Actual Results: See description Expected Results: See description
Yeah, this sounds reasonable. I wanted to avoid pulling in more dependencies but didn't know about this issue. Locally I've always used c-ares and never hit it. I'll enable it.
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=60b7aaa7a70149751446237d7207ff1ca20c256e commit 60b7aaa7a70149751446237d7207ff1ca20c256e Author: Anthony G. Basile <blueness@gentoo.org> AuthorDate: 2022-09-01 14:00:05 +0000 Commit: Anthony G. Basile <blueness@gentoo.org> CommitDate: 2022-09-01 14:05:18 +0000 net-misc/curl: version bump 7.85.0 We also enable USE=adns by default. Closes: https://bugs.gentoo.org/699632 Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Anthony G. Basile <blueness@gentoo.org> net-misc/curl/Manifest | 2 + net-misc/curl/curl-7.85.0.ebuild | 288 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 290 insertions(+)
Please note I added this to 7.85.0, the version that just came out and is currently not-stable. I did not back port the fix to 7.84.0 because that's the latest stable and curl is a critical package, so caution is in order.
From the peanut gallery, as a user of a ~x86_64 system on a 64 hardware thread system. If you're going to enable this by default, please make it take precedence over USE=threads rather than having the flags block each other. My understanding is that the only thing that USE=threads configures in curl is which dns resolver implementation to use, and so a choice of "adns" and "threads" are mutually exclusive. The problem is that this forces anyone with "threads" in their default use flags to special case curl, for essentially no good reason -- my general impression is that almost everyone who wants to tell packages to "use more threads if you think it might help, even at the cost of general efficiency" would still want curl to use ares rather than a threadpool. If this were configured by a sufficiently specific flag (e.g. "threaded-dns-resolver") there wouldn't be a problem. (Without doing any historical spelunking here, I'm guessing that curl gained the ares based DNS resolution more recently than the ebuild started using USE=threads -- I generally agree that "threads" makes sense to configure this behavior if you're just choosing between whether or not to use a threadpool to parallelize blocking DNS resolution, but that isn't the case today.)
(In reply to David Klempner from comment #4) > From the peanut gallery, as a user of a ~x86_64 system on a 64 hardware > thread system. > > If you're going to enable this by default, please make it take precedence > over USE=threads rather than having the flags block each other. > > My understanding is that the only thing that USE=threads configures in curl > is which dns resolver implementation to use, and so a choice of "adns" and > "threads" are mutually exclusive. The problem is that this forces anyone > with "threads" in their default use flags to special case curl, for > essentially no good reason -- my general impression is that almost everyone > who wants to tell packages to "use more threads if you think it might help, > even at the cost of general efficiency" would still want curl to use ares > rather than a threadpool. Actually, this is a great example of why it's not a good idea to put USE=threads in make.conf. Usually, upstreams always have threads support, or the "threads" toggle means enabling some experimental or legacy implementation, or not having "threads" on enables some experimental or legacy implementation again. So, we should really be phasing out the flag entirely (which I've wanted to do for a while). But it may work for now to just remove USE=threads or mask it for curl.
(In reply to Sam James from comment #5) > (In reply to David Klempner from comment #4) > > From the peanut gallery, as a user of a ~x86_64 system on a 64 hardware > > thread system. > > > > If you're going to enable this by default, please make it take precedence > > over USE=threads rather than having the flags block each other. > > > > My understanding is that the only thing that USE=threads configures in curl > > is which dns resolver implementation to use, and so a choice of "adns" and > > "threads" are mutually exclusive. The problem is that this forces anyone > > with "threads" in their default use flags to special case curl, for > > essentially no good reason -- my general impression is that almost everyone > > who wants to tell packages to "use more threads if you think it might help, > > even at the cost of general efficiency" would still want curl to use ares > > rather than a threadpool. > > Actually, this is a great example of why it's not a good idea to put > USE=threads in make.conf. Usually, upstreams always have threads support, or > the "threads" toggle means enabling some experimental or legacy > implementation, or not having "threads" on enables some experimental or > legacy implementation again. > > So, we should really be phasing out the flag entirely (which I've wanted to > do for a while). > > But it may work for now to just remove USE=threads or mask it for curl. Given this... Upstream default to using both adns & pthreads when available, so we'll keep USE=+adns, but I think we should drop USE=threads given USE=-threads is not an upstream configuration they default to and I don't really think it serves any purpose. I'm not sure why this is in the ebuild: > threads? ( !adns ) It seems to build fine for me with both enabled. I guess the idea is that at runtime, maybe ares/adns is used > threads if chosen, but that's fine with us, and in that case, the REQUIRED_USE isn't relevant anymore (it gives us the desired behaviour without).
Thrown up a PR at https://github.com/gentoo/gentoo/pull/27111 with what I'm thinking.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=bd4d42f83c774c36bf879a5b7ec89d373546743e commit bd4d42f83c774c36bf879a5b7ec89d373546743e Author: Sam James <sam@gentoo.org> AuthorDate: 2022-09-01 21:42:26 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2022-09-03 00:34:04 +0000 net-misc/curl: drop USE=threads USE=threads is a terrible USE flag (not the fault of net-misc/curl or its maintainer) and we're looking to get rid of it entirely. This is motivated further by the fact users have USE=threads in make.conf seeking performance improvements and such (which is understandable) but as per below, it can lead to conflicting and inconsistent effects. (In particular, anyone with USE="threads" in make.conf (don't do this for the reasons below!) will end up with a REQUIRED_USE issue between the now-default USE=adns and USE=threads). There are a few possibilities with threads support: 1. Upstreams always have threads support (hence USE=threads is only affecting a "handful" of packages anyway, not every single package, as many will just always be using pthreads and there's no way of disabling it); 2. The "threads" toggle means enabling some experimental or legacy implementation (think e.g. dev-lang/perl, which uses USE=ithreads for this, as it breaks ABI) (also applies to net-misc/curl given USE=threads doesn't help with the adns functionality many applications desire, see linked bug); 3. Not having "threads" on enables some experimental or legacy implementation again (this *again* applies to net-misc/curl given USE="-adns -threads" leaves us with a dodgy implementation as well). Commit 60b7aaa7a70149751446237d7207ff1ca20c256e enabled adns by default (IUSE=+adns). This commit drops USE=threads entirely and makes the default pthreads with adns opt-in with IUSE=+adns still. Now, the ebuild previously had REQUIRED_USE="threads? ( !adns )". I can't see a reason for this other than making the previous situation "meaningful", i.e. allowing people to decisively choose which is used at runtime, as while curl can build with both threads and adns, I think adns ends up preferred at runtime. With this change and given all of the above, we actually want adns to take priority if it's set, as threads will just be the default, so drop the REQUIRED_USE. Bug: https://bugs.gentoo.org/699632 See: 60b7aaa7a70149751446237d7207ff1ca20c256e Signed-off-by: Sam James <sam@gentoo.org> Closes: https://github.com/gentoo/gentoo/pull/27111 Signed-off-by: Sam James <sam@gentoo.org> net-misc/curl/{curl-7.85.0.ebuild => curl-7.85.0-r1.ebuild} | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
