Re: [PATCH v3 4/4] imap-send: use curl by default

[Junio C Hamano <gitster@pobox.com>]

Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com> writes:

> Now that curl is enable by default,

s/enable/&d/; 

But it is unclear what the above really means.  You certainly do not
mean that [PATCH 1-3/4] somewhere tweaked our Makefile to always use
libcurl and makes Git fail to build without it, but the above sounds
as if that were the case.

> use the curl implementation
> for imap too.

The Makefile for a long time by default set USE_CURL_FOR_IMAP_SEND
to YesPlease when the version of cURL we have is recent enough, I
think.  So I am not sure what you want to add with this change.

> The goal is to validate feature parity between the legacy and
> the curl implementation, deprecate thee legacy implementation

s/thee/the/;

> later on and in the long term, hopefully drop it altogether.
>
> Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com>
> ---
>  imap-send.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Hmph, the patch itself is also confusing.

> diff --git a/imap-send.c b/imap-send.c
> index a74d011a9..58c191704 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -35,11 +35,11 @@ typedef void *SSL;
>  #include "http.h"
>  #endif
>  
> -#if defined(USE_CURL_FOR_IMAP_SEND) && defined(NO_OPENSSL)
> -/* only available option */
> +#if defined(USE_CURL_FOR_IMAP_SEND)
> +/* Always default to curl if it's available. */
>  #define USE_CURL_DEFAULT 1

The original says "we want to use CURL, if Makefile tells us to
*AND* if Makefile tells us not to use OpenSSL", which does not make
much sense to me.  I wonder if the original is buggy and should have
been using "|| defined(NO_OPENSSL)" there instead.  

Perhaps that is the bug you are fixing with this patch, and all the
talk about curl is default we saw above is a red herring?  If that
is the case, then instead of removing, turning "&&" into "||" may be
a better fix.  I dunno.

>  #else
> -/* strictly opt in */
> +/* We don't have curl, so continue to use the historical implementation */
>  #define USE_CURL_DEFAULT 0
>  #endif