On Wed, Feb 01 2023, Jeff King wrote: > On Wed, Feb 01, 2023 at 03:22:24PM -0800, Junio C Hamano wrote: > >> > Let's also hide the old --curl and --no-curl options, and die if >> > "--no-curl" is provided. >> >> In other words, if we are building imap-send, we sure know cURL is >> there, and there is no need to tell a running imap-send not to use >> cURL to talk to the IMAP service? I am not sure the linkage of this >> change with the rest of the patch. Isn't that a totally orthogonal >> issue? Your imap-send might be cURL enabled, but unless we stop to >> ship with our own IMAP routines compiled into imap-send, --no-curl >> does have a purpose. >> >> Or did you just forget to document that we stop to ship with our own >> IMAP routines in the above? If so, as long as it is made a bit more >> prominent in the proposed log message in a reroll, I would be happy >> with such a change rolled into the same patch. > > FWIW, I had the same urge as Ævar, to drop the non-curl support > completely, and was puzzled that his patch did not have a big code > deletion. ;) FWIW I arrived at this from looking at the mandatory $(shell)-outs in the Makefile, and wasn't looking to drop the OpenSSL code. Then in looking at that, I found that we could probably make the curl dependency mandatory. > The problem is that the tunnel mode still relies on the non-curl code. > There was a series to address that a while ago: > > https://lore.kernel.org/git/ab866314-608b-eaca-b335-12cffe165526@xxxxxxxxxxxxxxxxxxxxxx/ > > but it ran into the problem that curl did not support PREAUTH > connections (which is one of the main points of tunneling). It looks > like that got added to curl via their befaa7b14f, which is in curl > 7.56.0 from 2017. That's not old enough for us to require for http, but > might be OK for a marginal component like the tunneling mode of > imap-send. > > I think there was also some question of how you even get the tunnel > going. Curl really wants to have a single socket descriptor, not two > pipe descriptors, so there may have to be some trickery with > socketpair(). There's more discussion in the linked thread. That's neat, I didn't know about that attempt. > So I think there's a path forward here for getting rid of the legacy > code (and I'd be really happy to see it gone; it's imported code that > does not seem super well maintained by us). But until we do that, > disabling --no-curl doesn't seem like that big a win, if that code can > all still be triggered for tunnel mode. I think the biggest win is that we're dropping the dual curl/OpenSSL codepath for everything except the "tunnel" mode, which is really obscure compared to the already-obscure functionality of the main "imap-send" tool. It would also get us part of the way to e.g. depending on 7.56.0, as a hard dependency on curl (and a newer version than we usually depend on) would reveal if anyone's got an issue with that stepping stone.
