Re: [PATCH] imap-send: replace auto-probe libcurl with hard dependency

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 1, 2023, 23:56 UTC
Message-ID: <230202.86zg9xormj.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqlelhx973.fsf@gitster.g>

On Wed, Feb 01 2023, Junio C Hamano wrote:

Show 9 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Change the "imap-send" command to have a hard dependency on libcurl,
>> before this it had an optional dependency on both libcurl and OpenSSL,
>> now only the OpenSSL dependency is optional.
>>
>> This simplifies our dependency matrix my getting rid of yet another
>
> "my" -> "by", I think.

Thanks, I'll fix that in a re-roll, pending the below...

Show 23 quoted lines

>> special-case. Given the prevalence of libcurl and portability of
>> libcurl it seems reasonable to say that "git imap-send" cannot be used
>> without libcurl, almost everyone building git needs to be able to push
>> or pull over http(s), so they'll be building with libcurl already.
>
> OK.
>
>> So let's remove the previous "USE_CURL_FOR_IMAP_SEND" knob. Whether we
>> build git-imap-send or not is now controlled by the "NO_CURL"
>> knob.
>
> OK.
>
>> 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.

The equivalent of USE_CURL_FOR_IMAP_SEND is now always true, and that's what "--curl" would enable.

The "--no-curl" option would then have us use the OpenSSL codepath, but that'll no longer be supported, we'll always use curl.

The link to the rest of the patch is then that "USE_CURL_FOR_IMAP_SEND" and "curl_check" etc. was needed to check if we had curl with imap-send, now we declare that we'll always need it.

And the link to the thread-at-large is that Jiang Xin's upthread version moves those checks from the Makefile into the code itself, I agree that wolud be an improvement, but if we're happy to just make it a hard dependency we won't need it there either...

> 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.

I'm not sure what you mean here, we still ship with the same routines, we just always take the "curl" codepath for the non-tunnel codepath now.

Is this perhaps confusion because while we do make curl mandatory, we're not dropping the OpenSSL code? That's because we're dropping its use for the non-tunnel codepath, but unfortunately for the tunnel case we'll still need it.

So we only make curl mandatory, but OpenSSL remains optional.