Re: [PATCH v14 05/10] imap-send: gracefully fail if CRAM-MD5 authentication is requested without OpenSSL

From: Aditya Garg <gargaditya08@live.com>
Date: Jun 7, 2025, 17:13 UTC
Message-ID: <PN3PR01MB9597C03C260EC7F5F9237A5FB869A@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>
In-Reply-To: <xmqqy0u3zhxl.fsf@gitster.g>

Show 26 quoted lines

> On 7 Jun 2025, at 9:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Aditya Garg <gargaditya08@live.com> writes:
> 
>> Unlike PLAIN, XOAUTH2 and OAUTHBEARER, CRAM-MD5 authentication is not
>> supported by libcurl and requires OpenSSL. If the user tries to use
>> CRAM-MD5 authentication without OpenSSL, the previous behaviour was to
>> attempt to authenticate and fail with a die(error). Handle this in a
>> better way by first checking if OpenSSL is available and then attempting
>> to authenticate. If OpenSSL is not available, print an error message and
>> exit gracefully.
>> 
>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>> ---
>> imap-send.c | 61 ++++++++++++++++++-----------------------------------
>> 1 file changed, 20 insertions(+), 41 deletions(-)
> 
> This is a good thing to do, but I would have expected that it would
> come a lot earlier in the series, perhaps immediately after 01/10
> fixes the copy-and-paste bug.  If this is moved earlier in the
> series, it would need to introduce the try_auth_method() helper at
> the same time.  Since there is no new authentication methods
> introduced at that stage in the series yet, it would be quite
> straight-forward to read and understand the patch, and on top of
> such a solidified ground, the series can add OAuth2.0 and PLAIN
> support on top.

I understand what you said is the ideal way to do, but since the cram md5 patch came up much later, I found it easier to place it at this place. I usually try to avoid as much conflicts as possible while rebasing, since I fear breaking something. But if I *have* to move it above, please let me know.

Thanks