Le 24/08/2017 à 15:53, Jeff King a écrit :
On Thu, Aug 24, 2017 at 10:00:47AM +0200, Nicolas Morey-Chaisemartin wrote:
quoted
quoted
Yes, I agree. I was hoping when we started this discussion that we were
more ready to switch to curl-by-default. But sadly, that isn't close to
being the case. But hopefully we can at least end up with logic that
lets us use it in the easy cases (no tunneling) and falls back in the
harder ones.
I opened a bug upstream and they already fixed this.
https://github.com/curl/curl/pull/1820
Cool! That's much faster than I had expected. :)
quoted
At least bleeding edge curl user should be able to use this.
I'm not sure where to go with these patches now.
1) There does not seem to be an easy/clean workaround for the lack of socketpair on windows.
Fidling with a loopback AF_UNIX?AF_LOCAL socket should work but it
means creating a socket file somewhere which pulls a lot of potential
issues (where to put it ? Post-mortem cleanup ? Parallel imap-send ?)
Even if you create a non-anonymous socket and connect to both ends, I'm
not sure how it works to pass that to the spawned child. IIRC, our
run_command emulation cannot pass arbitrary descriptors to the child
processes (but I don't know the details of why that is the case, or if
there are windows-specific calls we could be making to work around it).
Well as long as we can map it on a fd, the dup2 trickery should allow to remap whatever solution we pick to stdin/stdout.
Could this code be put in a #ifndef WINDOWS ?
quoted
2) The PREAUTH support won't largely be available for a while (curl,
release, distro, etc.)
- If this is the main use case, it does not make much sense to puch
curl; tunneling support without this. I could push the code and only
enable the curl tunneling for the next curl release ?
Meaning no one (or close to no one) would use this until some later
This also means very little testing (apart from mine) until the next
curl version gets widely available
- If this is not the main case (or at least the non PREAUTH is
important enough), it would make sense to get this changes in.
But it would probably need some more to code to either fallback to
legacy mode when curl failed (due to PREAUTH) or detect PREAUTH and
directly use the legacy mode.
It seems like we should be able to hit the cases that we know work out
of the box, and just hold back the default for the others. Like:
static int use_curl_auto(void)
{
#ifndef USE_CURL_FOR_IMAP_SEND
/* not built; we cannot use it */
return 0;
#else
if (srvc->tunnel) {
#if LIBCURL_VERSION < ...
/* no preauth support */
return 0;
#else
return 1;
#endif /* LIBCURL_VERSION < ... */
}
... other checks go here ...
#endif /* USE_CURL */
}
...
int use_curl = -1; /* auto */
... set use_curl to 0/1 from --curl/--no-curl command line */
if (use_curl < 0)
use_curl = use_curl_auto();
I'm not sure what other cases are left. But over time we'd hope that
use_curl_auto() would shrink to just "return 1", at which point
everybody is using it (and we can drop the fallback code).
This code works but I'm not that confortable getting code into master that will have been pretty much untested (I doubt there are many git pu/next user that run the bleeding edge curl on their setup)
and that may just break down once curl gets updated.
It has only been tested using the example line from imap-send man page which is a tiny coverage and I'm sure there are some IMAP server with funky interpreation of the standard out there (who said Exchange?)
Nicolas