Nicolas Morey-Chaisemartin [off-list ref] 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 <redacted>
---
imap-send.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Hmph, the patch itself is also confusing.
quoted hunk
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
Le 23/08/2017 à 22:28, Junio C Hamano a écrit :
Nicolas Morey-Chaisemartin [off-list ref] writes:
quoted
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.
quoted
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.
It did but apart from allowing the compilation of the code and enabling the --curl option to do something, it had no impact on the default runtime.
quoted
The goal is to validate feature parity between the legacy and
the curl implementation, deprecate thee legacy implementation
s/thee/the/;
Yes this is confusing.
In the current state, even if build against a curl version supporting imap, curl is not used by default at runtime (unless OpenSSL was not available).
This patch changes this behavior.
quoted
later on and in the long term, hopefully drop it altogether.
Signed-off-by: Nicolas Morey-Chaisemartin <redacted>
---
imap-send.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Hmph, the patch itself is also confusing.
quoted
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.
I think the idea for this was that curl should be used when curl is available and OpenSSL is not (curl being the only solution for secured authentication in this case)
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.
As said before, the goal of this patch is to enable curl by default at runtime when it has been "enabled" at compile time.
I'll reword
Is something like this clearer ?
Author: Nicolas Morey-Chaisemartin [off-list ref]
Date: Sun Aug 6 21:30:15 2017 +0200
imap-send: use curl by default when possible
Set curl as the runtime default when it is available.
When linked against older curl versions (< 7_34_0) or without curl,
use the legacy imap implementation.
The goal is to validate feature parity between the legacy and
the curl implementation, deprecate the legacy implementation
later on and in the long term, hopefully drop it altogether.
Signed-off-by: Nicolas Morey-Chaisemartin [off-list ref]