On Wed, Feb 01 2023, Jeff King wrote:
On Wed, Feb 01, 2023 at 03:22:24PM -0800, Junio C Hamano wrote:
quoted
quoted
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@morey-chaisemartin.com/ (local)
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.