Le 24/08/2017 à 15:53, Jeff King a écrit : > On Thu, Aug 24, 2017 at 10:00:47AM +0200, Nicolas Morey-Chaisemartin wrote: > >>> 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. :) > >> 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 ? > >> 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
