[PATCH/RFC] git-imap-send: use libcurl for implementation

Post by Bernhard Reiter
Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.

Wow! This sounds lovely. Thanks for working on this.

[...]

Post by Bernhard Reiter
Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones reduces imap-send.c by some 1200 lines of code.
As I don't have access to that many IMAP servers, I haven't been able to
test a variety of parameter combinations. I did test both secure and
insecure (imaps:// and imap://) connections -- this e-mail was generated
that way -- but could e.g. neither test the authMethod nor the tunnel
parameter.

The above two paragraphs belong in the commit message, too, since
they'll be just as useful for someone looking back through the history
as for someone reading the patch on-list today.

[...]

Post by Bernhard Reiter
--- a/INSTALL
+++ b/INSTALL

[...]

Post by Bernhard Reiter
- - "libcurl" library is used by git-http-fetch and git-fetch. You
+ - "libcurl" library is used by git-http-fetch, git-fetch, and
+ git-impap-send. You might also want the "curl" executable for

Typo: s/impap-send/imap-send/

Post by Bernhard Reiter
--- a/Makefile
+++ b/Makefile
@@ -2067,9 +2067,9 @@ endif
git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o http.o GIT-LDFLAGS $(GITLIBS)
- $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+ $(LIBS) $(CURL_LIBCURL)

7.30.0 is only ~1 year old. Does this mean users would need to update
curl in order to build imap-send?

For example, Debian 7.6 ships curl 7.26.0 and CentOS 7 has curl 7.29.0.

Ideally this could be done as an optional feature:

1. Introduce a USE_CURL_FOR_IMAP_SEND makefile variable to take
advantage of the nice new API.

2. (optional) Use the curl_check makefile variable to turn on
USE_CURL_FOR_IMAP_SEND automatically when appropriate.

3. In a few years, when everyone has upgraded, we could simplify by
getting rid of the USE_OPENSSL_FOR_IMAP_SEND code path.

What do you think?

[...]

Post by Bernhard Reiter
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,47 +22,13 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#include "cache.h"

Usual style is to start with a #include of "cache.h" or
"git-compat-util.h". "http.h" including cache.h for itself was an
old mistake. (I'll reply with a patch to fix that.)

[...]

Post by Bernhard Reiter
+#include <curl/curl.h>

http.h already #includes this. Do you use other helpers from
http.h/http.c or do you use libcurl directly? (Just curious.)

Some style nits:

[...]

Post by Bernhard Reiter
+static curl_socket_t opensocket(void *clientp, curlsocktype purpose,
+ struct curl_sockaddr *address)

Long line. Do you have ts=4? (Git uses 8-space tabs. There's some
emacs magic in Documentation/CodingGuidelines. It should be possible
to add similar hints there for other editors if they don't do the
right thing by default.)

Post by Bernhard Reiter
+{
+ curl_socket_t sockfd;
+ (void)purpose;
+ (void)address;

Elsewhere git lets unused parameters be. The unused parameter warning
is too noisy in callback-heavy code (e.g., for_each_ref) so we don't
turn it on.

Post by Bernhard Reiter
+ sockfd = *(curl_socket_t *)clientp;
+ /* the actual externally set socket is passed in via the OPENSOCKETDATA
+ option */

(style nit) Comments in git look like this:

/*
* The actual, externally set socket is passed in via the
* OPENSOCKETDATA option.
*/
return sockfd;

[...]

Post by Bernhard Reiter
+static int sockopt_callback(void *clientp, curl_socket_t curlfd,
+ curlsocktype purpose)
+{
+ /* This return code was added in libcurl 7.21.5 */
+ return CURL_SOCKOPT_ALREADY_CONNECTED;

I'd drop the comment, unless there's some subtlety involved. (E.g.,
is there some other return code that would be more appropriate but was
introudced later or something?)

[...]

Post by Bernhard Reiter
@@ -1368,12 +218,14 @@ static int git_imap_config(const char *key, const char *val, void *cb)
int main(int argc, char **argv)
{
struct strbuf all_msgs = STRBUF_INIT;
- struct strbuf msg = STRBUF_INIT;
+ struct buffer msg = { STRBUF_INIT, 0 };

Ah, ok --- we do use http.c stuff.

[...]

Post by Bernhard Reiter
+ char path[8192];
+ int pathlen;

I realize the old code only had 8192 for the IMAP command buffer,
but could this be a strbuf now, or is there some underlying limit
somewhere else?

[...]

Post by Bernhard Reiter
@@ -1417,31 +269,89 @@ int main(int argc, char **argv)
return 1;
}
+ curl_global_init(CURL_GLOBAL_ALL);

http.c seems to make the same mistake, but should the return value
from this be checked?

Post by Bernhard Reiter
- /* write it to the imap server */
- ctx = imap_open_store(&server);
- if (!ctx) {
- fprintf(stderr, "failed to open store\n");
+ curl = curl_easy_init();
+
+ if (!curl) {
+ fprintf(stderr, "failed to init curl\n");
return 1;

Could do

die("failed to init curl");

for more consistent message format and exit codes (128 for internal
errors, with an error message starting with 'fatal: ').

[...]

Post by Bernhard Reiter
+ if (ends_with(server.host, "/"))
+ pathlen = snprintf (path, sizeof(path), "%s%s", server.host, imap_folder);
+ else
+ pathlen = snprintf (path, sizeof(path), "%s/%s", server.host, imap_folder);
+
+ if (pathlen < 0)
+ die("Fatal: Out of memory");
+ if (pathlen >= sizeof(path))
+ die("imap URL overflow!");

With a strbuf, this could be something like

strbuf_addstr(&path, server.host);
if (!path.len || path.buf[path.len - 1] != '/')
strbuf_addch(&path, '/');
strbuf_addstr(&path, imap_folder);

or

if (ends_with(...))
strbuf_addf(&path, "%s%s", ...);
else
strbuf_addf(...);

Killing the unused ctx->prefix handling is nice. :)

[...]

Post by Bernhard Reiter
+ if (server.tunnel) {
+ const char *argv[] = { server.tunnel, NULL };
+ struct child_process tunnel = {NULL};

(not about this patch) Could use the child_proccess's internal
argv_array:

struct child_process tunnel = {NULL};
argv_array_push(&tunnel.args, server.tunnel);

(about this patch) Would there be a way to make this part reuse the
existing code? The only difference I see is that *srvc has been
renamed to server, which doesn't seem to be related to the change of
transport API from OpenSSL to libcurl.

[...]

Post by Bernhard Reiter
+ curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?

Hmm. curl expects to get a socket it can send(), recv(), setsockopt(),
etc on instead of a pair of fds to read() and write().

I wonder why someone would want to use SSL through a tunnel, though.
Currently it's impossible to get to the SSL codepath when a tunnel
is active (it's in the 'else' block an 'if (srvc->tunnel)'). If that
property is preserved, then we should be safe.

To summarize:

* I like this idea a lot! Using libcurl's imaps:// support directly
means one less dependency to worry about, and using alternate SSL
libraries like gnutls or nss becomes much easier (e.g., see
http://fedoraproject.org/wiki/FedoraCryptoConsolidation for how
that makes configuring certificate trust simpler).

* This would be easier to take if guarded by an #ifdef, so people
stuck on ancient libcurl would still be able to use git (and
ideally still use imap over SSL).

* This shouldn't have to touch the imap.tunnel support. imap-send's
imap.tunnel configuration expects the tunnel to take care of
securing the channel (e.g. by using 'openssl s_client').

* Any potential cleanups noticed along the way are very welcome,
as separate patches.

* As soon as you're ready to roll this out to a wider audience of
testers, let me know, and we can try to get it into shape for
Junio's "next" branch (and hence Debian experimental).

Thanks and hope that helps,
Jonathan

Jeff King

2014-08-17 08:30:22 UTC

Post by Bernhard Reiter
+ curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?

Hmm. curl expects to get a socket it can send(), recv(), setsockopt(),
etc on instead of a pair of fds to read() and write().

I wonder if we could teach run_command to optionally use socketpair()
instead of pipe(). I'm not sure if that would cause problems on Windows,
though.

Post by Jonathan Nieder
I wonder why someone would want to use SSL through a tunnel, though.
Currently it's impossible to get to the SSL codepath when a tunnel is
active (it's in the 'else' block an 'if (srvc->tunnel)'). If that
property is preserved, then we should be safe.

I'm not sure I understand this comment. Even if SSL is not in use,
wouldn't we be passing a regular pipe to curl, which would break?

-Peff

Bernhard Reiter

2014-08-17 12:56:10 UTC

Post by Jeff King

Post by Bernhard Reiter
+ curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?

Hmm. curl expects to get a socket it can send(), recv(), setsockopt(),
etc on instead of a pair of fds to read() and write().

I wonder if we could teach run_command to optionally use socketpair()
instead of pipe().

That sounds like a good idea to me.

Post by Jeff King
I'm not sure if that would cause problems on Windows,
though.

Apparently socketpair is not available there. Googling "socketpair
windows" yields, among a lot of other useful resources, the following
relatively actively maintained ~150 LOC, BSD-3-clause-licensed,
implementation:

https://github.com/ncm/selectable-socketpair

That license is GPL compatible, so should we consider including that
implementation with git? That's the kind of stuff that goes to
compat/win32, right?

One thing to consider: seems like socketpair() gives AF_LOCAL sockets,
so I've asked [1] on the curl ML if that would work or if libcurl needs
an AF_INET one.

Post by Jeff King

I'm not sure I understand this comment. Even if SSL is not in use,
wouldn't we be passing a regular pipe to curl, which would break?

Yeah, we can't do that, and thus would have to keep the handwritten IMAP
implementation just for the tunnel case (allowing to drop only the
OpenSSL specific stuff), see my other email:
http://www.mail-archive.com/***@vger.kernel.org/msg56791.html (the
relevant part is pretty far down at the bottom).

Bernhard

[1] http://curl.haxx.se/mail/lib-2014-08/0131.html

Jeff King

2014-08-17 18:42:52 UTC

Post by Jeff King
I'm not sure if that would cause problems on Windows,
though.

Thanks for researching. Sticking that in compat/ would be our usual
strategy, yes.

Post by Jeff King
I'm not sure I understand this comment. Even if SSL is not in use,
wouldn't we be passing a regular pipe to curl, which would break?

Yeah, we can't do that, and thus would have to keep the handwritten IMAP
implementation just for the tunnel case (allowing to drop only the
relevant part is pretty far down at the bottom).

I'd really love it if we could make this work with tunnels and
eventually get rid of the hand-written imap code entirely. I agree with
Jonathan that we probably need to keep it around a bit for people on
older curl, but dropping it is a good goal in the long run. That code
was forked from the isync project, but mangled enough that we could not
take bug fixes from upstream. As not many people use imap-send, I
suspect it is largely unmaintained and the source of many lurking
bugs[1]. Replacing it with curl's maintained implementation is probably
a good step.

-Peff

[1] That's my somewhat subjective opinion, but I feel like I have seen
several bugs reported in imap-send that had literally been there for
years. And having worked on IMAP implementations in a past life, I
know there are many dark corners of the protocol that vary from
server to server.

Bernhard Reiter

2014-08-19 11:14:11 UTC

Post by Jeff King
[...]

Post by Jeff King
I'm not sure I understand this comment. Even if SSL is not in use,
wouldn't we be passing a regular pipe to curl, which would break?

Yeah, we can't do that, and thus would have to keep the handwritten IMAP
implementation just for the tunnel case (allowing to drop only the
relevant part is pretty far down at the bottom).

I'll work on this as soon as I find some time, but as that will include
changes to run-command.c (and possibly other files?), I'd like to cover
that in a commit of its own. Do you guys think the current patch [1] is
good enough for "official" submission already? If so, do I need some
sort of official review? Documentation/SubmittingPatches says I'm only
supposed to direct it to Junio after the list "reaches consensus", so
I'm wondering how to get there... :-)

Bernhard

Junio C Hamano

2014-08-19 17:13:35 UTC

Post by Jeff King
[...]

Post by Jeff King
I'm not sure I understand this comment. Even if SSL is not in use,
wouldn't we be passing a regular pipe to curl, which would break?

Yeah, we can't do that, and thus would have to keep the handwritten IMAP
implementation just for the tunnel case (allowing to drop only the
relevant part is pretty far down at the bottom).

I would agree with s/a good step/a good goal/ ;-)

Post by Bernhard Reiter
I'll work on this as soon as I find some time, but as that will include
changes to run-command.c (and possibly other files?), I'd like to cover
that in a commit of its own. Do you guys think the current patch [1] is
good enough for "official" submission already?

My impression from reading the discussion in this thread has been
that the patch that started this thread would break the tunneling
code, i.e. is not there yet. Or did you mean some other patch?

The other patch $gmane/255403 from you looked good and I think I
already have a copy queued on 'pu' as f9dc5d65 (git-imap-send:
simplify tunnel construction, 2014-08-13).

Thanks.

[References]

*$gmane/255403*
http://thread.gmane.org/gmane.comp.version-control.git/255220/focus=255403

Bernhard Reiter

2014-08-14 21:46:46 UTC

Post by Bernhard Reiter
[...]

Wow! This sounds lovely. Thanks for working on this.

Well thanks for the friendly welcome and the helpful comments!

I'm attaching a patch where I've applied the fixes you suggested, plus:

* I added the lf_to_crlf conversion to the curl codepath as
communication with another IMAP server I tried was broken without it.

* I added STARTTLS. (That's just the
curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);
line)

* I tested (and fixed) authentication, i.e. the auth_method stuff. As
the corresponding CURLOPT_LOGIN_OPTIONS flag has only been available
starting with curl 7.35.0, I've bumped the required version to that.
(Apparently it was possible to achieve the same effect with a different
option in between versions 7.31.0 and 7.34.0 [1], but I haven't found
yet how. Is it worth the effort?)

* I made that file scope imap_folder a member of struct imap_server_conf
(named folder), which makes some things easier.

Post by Bernhard Reiter
@@ -1417,31 +269,89 @@ int main(int argc, char **argv)
return 1;
}
+ curl_global_init(CURL_GLOBAL_ALL);

http.c seems to make the same mistake,

Patch at http://permalink.gmane.org/gmane.comp.version-control.git/255221

Post by Jonathan Nieder
[...]

Post by Bernhard Reiter
+ if (server.tunnel) {
+ const char *argv[] = { server.tunnel, NULL };
+ struct child_process tunnel = {NULL};

(not about this patch) Could use the child_proccess's internal
struct child_process tunnel = {NULL};
argv_array_push(&tunnel.args, server.tunnel);

Patch at
http://permalink.gmane.org/gmane.comp.version-control.git/255220 (The
patch attached to this mail depends on that one.)

No comments on those patches yet, though.

Post by Jonathan Nieder
(about this patch) Would there be a way to make this part reuse the
existing code? The only difference I see is that *srvc has been
renamed to server, which doesn't seem to be related to the change of
transport API from OpenSSL to libcurl.
[...]

Post by Bernhard Reiter
+ curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?

Now this turns out to be the one major annoyance left, because we only
have those two fds (actually pipes, right?), and not a socket that we
could pass to curl, so we can't use it to talk to the IMAP server. So if
the tunnel parameter is set, we're stuck with the old hand-written IMAP
handling routines, even with USE_CURL_FOR_IMAP set, meaning I can't wrap
as much in #ifdef...#endif blocks as I'd like. :-( BTW, due to two of
the blocks that I do add I get a compiler warning about the curl handle
remaining possibly unitialized :-/
I've removed the curl specific socket handling routines, as we can't use
them anyway for now.

I've asked about passing two pipes instead of a socket to curl on their
ML [1] as this has even been discussed before [2], but unfortunately,
there doesn't seem to be a solution as of yet. I've also asked on SO
[3], but no answers yet.

Post by Jonathan Nieder
[...]
* As soon as you're ready to roll this out to a wider audience of
testers, let me know, and we can try to get it into shape for
Junio's "next" branch (and hence Debian experimental).

Is this one good enough already?

Bernhard

[1] http://sourceforge.net/p/curl/bugs/1372/
[2] http://curl.haxx.se/mail/lib-2014-08/0102.html
[3] http://curl.haxx.se/mail/lib-2011-05/0102.html
[4]
http://stackoverflow.com/questions/25306264/connect-in-and-out-pipes-to-network-socket

Documentation/git-imap-send.txt | 3 +-
INSTALL | 15 +++---
Makefile | 16 +++++-
git.spec.in | 5 +-
imap-send.c | 109
+++++++++++++++++++++++++++++++++++++---
5 files changed, 132 insertions(+), 16 deletions(-)

Junio C Hamano

2014-08-19 17:51:13 UTC

Post by Bernhard Reiter
Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.
Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones would reduce imap-send.c by some 1200 lines of code. For now,
the old ones are wrapped in #ifdefs, and the new functions are enabled
by make if curl's version is >= 7.35.0, from which version on curl's
CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been
available.
As I don't have access to that many IMAP servers, I haven't been able to
test the new code with a wide variety of parameter combinations. I did
test both secure and insecure (imaps:// and imap://) connections and
values of "PLAIN" and "LOGIN" for the authMethod.
---

I think people have missed this one, partly because it was hidden as
an attachment.

I smell a line-wrapped patch but it probably is at my receiving end,
forcing the attachment into a flattened form inside my MUA.

Nice to see git.spec.in updated; I like it when I see that the
author paid attention to details.

Post by Bernhard Reiter
diff --git a/imap-send.c b/imap-send.c
index fb01a9c..a45570d 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,6 +22,10 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#ifdef USE_CURL_FOR_IMAP_SEND
+#define NO_OPENSSL
+#endif
+

This looks strange and stands out like a sore thumb. Do any of our
other sources do this kind of macro tweaking inside C source before
including git-compat-util.h (or its equivalent like cache.h)?

I think what you are trying to do is to change the meaning of
NO_OPENSSL, which merely means "we do not have OpenSSL library and
do not want to link with it", locally to "we may or may not have and
use OpenSSL library elsewhere in Git, but in the code below we do
not want to use the code written to work on top of OpenSSL and
instead use libcurl". Because of that, you define NO_OPENSSL before
including any of our headers to cause us not to include the headers,
and typedef away SSL, for example.

Post by Bernhard Reiter
#include "cache.h"
#include "credential.h"
#include "exec_cmd.h"
@@ -29,6 +33,9 @@
#ifdef NO_OPENSSL
typedef void *SSL;
#endif
+#ifdef USE_CURL_FOR_IMAP_SEND
+#include "http.h"
+#endif

But does it have to be that way? For one thing, doing it this way,
the user has to make a compilation-time choice, but if you separate
these compilation time macro into two, one for "can we even link
with and use OpenSSL?" (which is what NO_OPENSSL is about) and
another for "do we want an ability to talk to imap via libcurl?",
wouldn't it make it possible for you to switch between them at
runtime (e.g. you might want to go over the direct connection when
tunneling, while letting libcurl do the heavy lifting in
non-tunneled operation)?

Post by Bernhard Reiter
@@ -44,9 +51,7 @@ __attribute__((format (printf, 1, 2)))
static void imap_info(const char *, ...);
__attribute__((format (printf, 1, 2)))
static void imap_warn(const char *, ...);
-
static char *next_arg(char **);
-
__attribute__((format (printf, 3, 4)))
static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
@@ -69,6 +74,7 @@ struct imap_server_conf {
char *tunnel;
char *host;
int port;
+ char *folder;
char *user;
char *pass;
int use_ssl;
@@ -82,6 +88,7 @@ static struct imap_server_conf server = {
NULL, /* tunnel */
NULL, /* host */
0, /* port */
+ NULL, /* folder */
NULL, /* user */
NULL, /* pass */
0, /* use_ssl */
@@ -1323,7 +1330,54 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
return 1;
}
-static char *imap_folder;

All of the above seem to have meant well, but these changes are not
about talking with IMAP servers via libcurl. We'd prefer to see
changes like these as preliminary clean-up before the main change as
separate patch(es).

Post by Bernhard Reiter
@@ -1417,18 +1476,48 @@ int main(int argc, char **argv)
}
/* write it to the imap server */
+
+#ifdef USE_CURL_FOR_IMAP_SEND
+ if (!server.tunnel) {
+ curl = setup_curl(&server);
+ curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
+ } else {
+#endif
ctx = imap_open_store(&server);
if (!ctx) {
fprintf(stderr, "failed to open store\n");
return 1;
}
+ ctx->name = server.folder;
+#ifdef USE_CURL_FOR_IMAP_SEND
+ }
+#endif
fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
- ctx->name = imap_folder;
while (1) {
unsigned percent = n * 100 / total;
fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+#ifdef USE_CURL_FOR_IMAP_SEND
+ if (!server.tunnel) {
...
+ }
+ } else {
+#endif
if (!split_msg(&all_msgs, &msg, &ofs))
break;
if (server.use_html)
@@ -1436,11 +1525,19 @@ int main(int argc, char **argv)
r = imap_store_msg(ctx, &msg);
if (r != DRV_OK)
break;
+#ifdef USE_CURL_FOR_IMAP_SEND
+ }
+#endif
n++;
}
fprintf(stderr, "\n");
+#ifdef USE_CURL_FOR_IMAP_SEND
+ curl_easy_cleanup(curl);
+ curl_global_cleanup();
+#else
imap_close_store(ctx);
+#endif
return 0;
}

Ugly. Can we do this with less #ifdef/#else/#endif in the primary
code path?

If we were to keep these two modes as a choice the users have to
make at the compilation time, that is.

Thanks.

Bernhard Reiter

2014-08-25 20:11:11 UTC

Post by Junio C Hamano
This looks strange and stands out like a sore thumb. Do any of our
other sources do this kind of macro tweaking inside C source before
including git-compat-util.h (or its equivalent like cache.h)?

I haven't checked, but I agree that it's desirable to avoid.

Post by Junio C Hamano
I think what you are trying to do is to change the meaning of
NO_OPENSSL, which merely means "we do not have OpenSSL library and
do not want to link with it", locally to "we may or may not have and
use OpenSSL library elsewhere in Git, but in the code below we do
not want to use the code written to work on top of OpenSSL and
instead use libcurl".

"...and we don't want to link to OpenSSL in that case." Yeah.

Post by Junio C Hamano
Because of that, you define NO_OPENSSL before
including any of our headers to cause us not to include the headers,
and typedef away SSL, for example.

The SSL un-typedef'ing was there before, but it's true that I'm defining
NO_OPENSSL on the very top so the included headers don't require OpenSSL
(and so we don't have to link to it later).

Yeah, but I still need to wrap the non-tunneled operation in #ifdefs in
case we don't USE_CURL_FOR_IMAP_SEND in which case we fall back to the
handwritten IMAP code, don't I?

Maybe I'm not getting you entirely right, but if I don't typedef
NO_OPENSSL if USE_CURL_FOR_IMAP_SEND is defined, I don't see any way to
not link to OpenSSL, even if it's not required. I'm including a slightly
modified patch which does that (hoping that I've finally managed to send
a usable patch). Sorry it's nothing breathtakingly better than before,
even after giving this some thought I didn't arrive at a very elegant
new solution... (see below for more on that)

Ugly. Can we do this with less #ifdef/#else/#endif in the primary
code path?

It is ugly, but as much as I'd love to put e.g.

+#ifdef USE_CURL_FOR_IMAP_SEND
+ if (!server.tunnel) {
+ curl = setup_curl(&server);

etc into imap_open_store (and similarly for imap_store_msg etc), I don't
see any easy way to do it; imap_open_store's return type would still be
struct imap_store* in the non-tunneling case, and CURL* otherwise.

So the best I can come up with here is merging some of the #ifdef
blocks, but that means duplicating the code that applies in both cases.
But that isn't any better, is it?

Post by Junio C Hamano
If we were to keep these two modes as a choice the users have to
make at the compilation time, that is.

As stated above, I'm not sure how to do entirely without at least those
two compile time switches (NO_OPENSSL and USE_CURL_FOR_IMAP_SEND).

Sorry, perhaps I missed something obvious; grateful for any hints on how
to do it better.

Bernhard

Documentation/git-imap-send.txt | 3 +-
INSTALL | 15 ++++---
Makefile | 16 ++++++-
git.spec.in | 5 ++-
imap-send.c | 96 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 125 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 7d991d9..9d244c4 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -75,7 +75,8 @@ imap.preformattedHTML::

imap.authMethod::
Specify authenticate method for authentication with IMAP server.
- Current supported method is 'CRAM-MD5' only. If this is not set
+ If you compiled git with the NO_CURL option or if your curl version is
+ < 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set
then 'git imap-send' uses the basic IMAP plaintext LOGIN command.

Examples
diff --git a/INSTALL b/INSTALL
index 6ec7a24..e2770a0 100644
--- a/INSTALL
+++ b/INSTALL
@@ -108,18 +108,21 @@ Issues of note:
so you might need to install additional packages other than Perl
itself, e.g. Time::HiRes.

- - "openssl" library is used by git-imap-send to use IMAP over SSL.
- If you don't need it, use NO_OPENSSL.
+ - "openssl" library is used by git-imap-send to use IMAP over SSL,
+ unless you're using curl >= 7.35.0, in which case that will be
+ used. If you don't need git-imap-send, you can use NO_OPENSSL.

By default, git uses OpenSSL for SHA1 but it will use its own
library (inspired by Mozilla's) with either NO_OPENSSL or
BLK_SHA1. Also included is a version optimized for PowerPC
(PPC_SHA1).

- - "libcurl" library is used by git-http-fetch and git-fetch. You
- might also want the "curl" executable for debugging purposes.
- If you do not use http:// or https:// repositories, you do not
- have to have them (use NO_CURL).
+ - "libcurl" library is used by git-http-fetch, git-fetch, and, if
+ the curl version >= 7.35.0, for git-imap-send. You might also
+ want the "curl" executable for debugging purposes. If you do not
+ use http:// or https:// repositories, and do not want to put
+ patches into an IMAP mailbox, you do not have to have them
+ (use NO_CURL).

- "expat" library; git-http-push uses it for remote lock
management over DAV. Similar to "curl" above, this is optional
diff --git a/Makefile b/Makefile
index 237bc05..c08963c 100644
--- a/Makefile
+++ b/Makefile
@@ -1133,6 +1133,9 @@ ifdef HAVE_ALLOCA_H
BASIC_CFLAGS += -DHAVE_ALLOCA_H
endif

+IMAP_SEND_BUILDDEPS =
+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+
ifdef NO_CURL
BASIC_CFLAGS += -DNO_CURL
REMOTE_CURL_PRIMARY =
@@ -1167,6 +1170,15 @@ else
PROGRAM_OBJS += http-push.o
endif
endif
+ curl_check := $(shell (echo 072300; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)
+ ifeq "$(curl_check)" "072300"
+ USE_CURL_FOR_IMAP_SEND = YesPlease
+ endif
+ ifdef USE_CURL_FOR_IMAP_SEND
+ BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND
+ IMAP_SEND_BUILDDEPS = http.o
+ IMAP_SEND_LDFLAGS += $(CURL_LIBCURL)
+ endif
ifndef NO_EXPAT
ifdef EXPATDIR
BASIC_CFLAGS += -I$(EXPATDIR)/include
@@ -2084,9 +2096,9 @@ endif
git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)

-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
- $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+ $(LIBS) $(IMAP_SEND_LDFLAGS)

git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
diff --git a/git.spec.in b/git.spec.in
index d61d537..9535cc3 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -8,7 +8,7 @@ License: GPL
Group: Development/Tools
URL: http://kernel.org/pub/software/scm/git/
Source: http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
+BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Requires: perl-Git = %{version}-%{release}
@@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT
# No files for you!

%changelog
+* Mon Aug 11 2014 Bernhard Reiter <***@raz.or.at>
+- Require version >= 7.35.0 of curl-devel for IMAP functions.
+
* Sun Sep 18 2011 Jakub Narebski <***@gmail.com>
- Add gitweb manpages to 'gitweb' subpackage

diff --git a/imap-send.c b/imap-send.c
index ad330a6..3343429 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -29,6 +29,9 @@
#ifdef NO_OPENSSL
typedef void *SSL;
#endif
+#ifdef USE_CURL_FOR_IMAP_SEND
+#include "http.h"
+#endif

static const char imap_send_usage[] = "git imap-send < <mbox>";

@@ -1307,6 +1310,55 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
return 1;
}

+#ifdef USE_CURL_FOR_IMAP_SEND
+static CURL *setup_curl(struct imap_server_conf *srvc)
+{
+ CURL *curl;
+ struct strbuf path = STRBUF_INIT;
+ struct strbuf auth = STRBUF_INIT;
+
+ if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
+ die("curl_global_init failed");
+
+ curl = curl_easy_init();
+
+ if (!curl)
+ die("curl_easy_init failed");
+
+ curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
+ curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
+
+ strbuf_addstr(&path, server.host);
+ if (!path.len || path.buf[path.len - 1] != '/')
+ strbuf_addch(&path, '/');
+ strbuf_addstr(&path, server.folder);
+
+ curl_easy_setopt(curl, CURLOPT_URL, path.buf);
+ curl_easy_setopt(curl, CURLOPT_PORT, server.port);
+
+ if (server.auth_method) {
+ strbuf_addstr(&auth, "AUTH=");
+ strbuf_addstr(&auth, server.auth_method);
+ curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);
+ }
+
+ if (server.use_ssl)
+ curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);
+
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);
+
+ curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
+
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
+
+ if (Verbose)
+ curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
+
+ return curl;
+}
+#endif
+
static void git_imap_config(void)
{
const char *val = NULL;
@@ -1347,6 +1399,11 @@ int main(int argc, char **argv)
int r;
int total, n = 0;
int nongit_ok;
+#ifdef USE_CURL_FOR_IMAP_SEND
+ struct buffer msgbuf = { STRBUF_INIT, 0 };
+ CURL *curl;
+ CURLcode res = CURLE_OK;
+#endif /* #ifndef USE_CURL_FOR_IMAP_SEND */

git_extract_argv0_path(argv[0]);

@@ -1391,17 +1448,48 @@ int main(int argc, char **argv)
}

/* write it to the imap server */
+
+#ifdef USE_CURL_FOR_IMAP_SEND
+ if (!server.tunnel) {
+ curl = setup_curl(&server);
+ curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
+ } else {
+#endif
ctx = imap_open_store(&server, server.folder);
if (!ctx) {
fprintf(stderr, "failed to open store\n");
return 1;
}
+ ctx->name = server.folder;
+#ifdef USE_CURL_FOR_IMAP_SEND
+ }
+#endif

fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
while (1) {
unsigned percent = n * 100 / total;

fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+#ifdef USE_CURL_FOR_IMAP_SEND
+ if (!server.tunnel) {
+ int prev_len = msgbuf.buf.len;
+ if (!split_msg(&all_msgs, &msgbuf.buf, &ofs))
+ break;
+ if (server.use_html)
+ wrap_in_html(&msgbuf.buf);
+ lf_to_crlf(&msgbuf.buf);
+
+ curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));
+
+ res = curl_easy_perform(curl);
+
+ if(res != CURLE_OK) {
+ fprintf(stderr, "curl_easy_perform() failed: %s\n",
+ curl_easy_strerror(res));
+ break;
+ }
+ } else {
+#endif
if (!split_msg(&all_msgs, &msg, &ofs))
break;
if (server.use_html)
@@ -1409,11 +1497,19 @@ int main(int argc, char **argv)
r = imap_store_msg(ctx, &msg);
if (r != DRV_OK)
break;
+#ifdef USE_CURL_FOR_IMAP_SEND
+ }
+#endif
n++;
}
fprintf(stderr, "\n");

+#ifdef USE_CURL_FOR_IMAP_SEND
+ curl_easy_cleanup(curl);
+ curl_global_cleanup();
+#else
imap_close_store(ctx);
+#endif

return 0;
}

--
2.1.0.372.g89e5a25

Junio C Hamano

2014-08-25 21:08:48 UTC

Post by Bernhard Reiter
Yeah, but I still need to wrap the non-tunneled operation in #ifdefs in
case we don't USE_CURL_FOR_IMAP_SEND in which case we fall back to the
handwritten IMAP code, don't I?

We do not mind multiple implementations of the same helper function
that are guarded with #ifdef/#endif, and we do use that style quite
a lot. Would it help?

Bernhard Reiter

2014-08-26 22:40:17 UTC

Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.

Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones would reduce imap-send.c by some 1200 lines of code. For now,
the old ones are wrapped in #ifdefs, and the new functions are enabled
by make if curl's version is >= 7.35.0, from which version on curl's
CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been
available.

As I don't have access to that many IMAP servers, I haven't been able to
test the new code with a wide variety of parameter combinations. I did
test both secure and insecure (imaps:// and imap://) connections and
values of "PLAIN" and "LOGIN" for the authMethod.

Signed-off-by: Bernhard Reiter <***@raz.or.at>
---
I rebased the patch on the pu branch, hope that was the right thing to do.

Bernhard

Documentation/git-imap-send.txt | 3 +-
INSTALL | 15 ++--
Makefile | 16 +++-
git.spec.in | 5 +-
imap-send.c | 165 +++++++++++++++++++++++++++++++++-------
5 files changed, 167 insertions(+), 37 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 7d991d9..9d244c4 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -75,7 +75,8 @@ imap.preformattedHTML::

imap.authMethod::
Specify authenticate method for authentication with IMAP server.
- Current supported method is 'CRAM-MD5' only. If this is not set
+ If you compiled git with the NO_CURL option or if your curl version is
+ < 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set
then 'git imap-send' uses the basic IMAP plaintext LOGIN command.

Examples
diff --git a/INSTALL b/INSTALL
index 6ec7a24..e2770a0 100644
--- a/INSTALL
+++ b/INSTALL
@@ -108,18 +108,21 @@ Issues of note:
so you might need to install additional packages other than Perl
itself, e.g. Time::HiRes.

- - "openssl" library is used by git-imap-send to use IMAP over SSL.
- If you don't need it, use NO_OPENSSL.
+ - "openssl" library is used by git-imap-send to use IMAP over SSL,
+ unless you're using curl >= 7.35.0, in which case that will be
+ used. If you don't need git-imap-send, you can use NO_OPENSSL.

By default, git uses OpenSSL for SHA1 but it will use its own
library (inspired by Mozilla's) with either NO_OPENSSL or
BLK_SHA1. Also included is a version optimized for PowerPC
(PPC_SHA1).

- - "libcurl" library is used by git-http-fetch and git-fetch. You
- might also want the "curl" executable for debugging purposes.
- If you do not use http:// or https:// repositories, you do not
- have to have them (use NO_CURL).
+ - "libcurl" library is used by git-http-fetch, git-fetch, and, if
+ the curl version >= 7.35.0, for git-imap-send. You might also
+ want the "curl" executable for debugging purposes. If you do not
+ use http:// or https:// repositories, and do not want to put
+ patches into an IMAP mailbox, you do not have to have them
+ (use NO_CURL).

- "expat" library; git-http-push uses it for remote lock
management over DAV. Similar to "curl" above, this is optional
diff --git a/Makefile b/Makefile
index 237bc05..c08963c 100644
--- a/Makefile
+++ b/Makefile
@@ -1133,6 +1133,9 @@ ifdef HAVE_ALLOCA_H
BASIC_CFLAGS += -DHAVE_ALLOCA_H
endif

+IMAP_SEND_BUILDDEPS =
+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+
ifdef NO_CURL
BASIC_CFLAGS += -DNO_CURL
REMOTE_CURL_PRIMARY =
@@ -1167,6 +1170,15 @@ else
PROGRAM_OBJS += http-push.o
endif
endif
+ curl_check := $(shell (echo 072300; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)
+ ifeq "$(curl_check)" "072300"
+ USE_CURL_FOR_IMAP_SEND = YesPlease
+ endif
+ ifdef USE_CURL_FOR_IMAP_SEND
+ BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND
+ IMAP_SEND_BUILDDEPS = http.o
+ IMAP_SEND_LDFLAGS += $(CURL_LIBCURL)
+ endif
ifndef NO_EXPAT
ifdef EXPATDIR
BASIC_CFLAGS += -I$(EXPATDIR)/include
@@ -2084,9 +2096,9 @@ endif
git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)

-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
- $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+ $(LIBS) $(IMAP_SEND_LDFLAGS)

git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
diff --git a/git.spec.in b/git.spec.in
index d61d537..9535cc3 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -8,7 +8,7 @@ License: GPL
Group: Development/Tools
URL: http://kernel.org/pub/software/scm/git/
Source: http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
+BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Requires: perl-Git = %{version}-%{release}
@@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT
# No files for you!

%changelog
+* Mon Aug 11 2014 Bernhard Reiter <***@raz.or.at>
+- Require version >= 7.35.0 of curl-devel for IMAP functions.
+
* Sun Sep 18 2011 Jakub Narebski <***@gmail.com>
- Add gitweb manpages to 'gitweb' subpackage

diff --git a/imap-send.c b/imap-send.c
index ad330a6..a170cbb 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -29,6 +29,9 @@
#ifdef NO_OPENSSL
typedef void *SSL;
#endif
+#ifdef USE_CURL_FOR_IMAP_SEND
+#include "http.h"
+#endif

static const char imap_send_usage[] = "git imap-send < <mbox>";

@@ -1338,14 +1341,138 @@ static void git_imap_config(void)
git_config_get_string("imap.authmethod", &server.auth_method);
}

-int main(int argc, char **argv)
-{
- struct strbuf all_msgs = STRBUF_INIT;
+static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {
struct strbuf msg = STRBUF_INIT;
struct imap_store *ctx = NULL;
int ofs = 0;
int r;
- int total, n = 0;
+ int n = 0;
+
+ ctx = imap_open_store(server, server->folder);
+ if (!ctx) {
+ fprintf(stderr, "failed to open store\n");
+ return 1;
+ }
+ ctx->name = server->folder;
+
+ fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
+ while (1) {
+ unsigned percent = n * 100 / total;
+
+ fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+
+ if (!split_msg(all_msgs, &msg, &ofs))
+ break;
+ if (server->use_html)
+ wrap_in_html(&msg);
+ r = imap_store_msg(ctx, &msg);
+ if (r != DRV_OK)
+ break;
+ n++;
+ }
+ fprintf(stderr, "\n");
+
+ imap_close_store(ctx);
+
+ return 0;
+}
+
+#ifdef USE_CURL_FOR_IMAP_SEND
+static CURL *setup_curl(struct imap_server_conf *srvc)
+{
+ CURL *curl;
+ struct strbuf path = STRBUF_INIT;
+ struct strbuf auth = STRBUF_INIT;
+
+ if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
+ die("curl_global_init failed");
+
+ curl = curl_easy_init();
+
+ if (!curl)
+ die("curl_easy_init failed");
+
+ curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
+ curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
+
+ strbuf_addstr(&path, server.host);
+ if (!path.len || path.buf[path.len - 1] != '/')
+ strbuf_addch(&path, '/');
+ strbuf_addstr(&path, server.folder);
+
+ curl_easy_setopt(curl, CURLOPT_URL, path.buf);
+ curl_easy_setopt(curl, CURLOPT_PORT, server.port);
+
+ if (server.auth_method) {
+ strbuf_addstr(&auth, "AUTH=");
+ strbuf_addstr(&auth, server.auth_method);
+ curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);
+ }
+
+ if (server.use_ssl)
+ curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);
+
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);
+
+ curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
+
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
+
+ if (Verbose)
+ curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
+
+ return curl;
+}
+
+static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {
+ int ofs = 0;
+ int n = 0;
+ struct buffer msgbuf = { STRBUF_INIT, 0 };
+ CURL *curl;
+ CURLcode res = CURLE_OK;
+
+ curl = setup_curl(server);
+ curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
+
+ fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
+ while (1) {
+ unsigned percent = n * 100 / total;
+
+ fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+
+ int prev_len = msgbuf.buf.len;
+ if (!split_msg(all_msgs, &msgbuf.buf, &ofs))
+ break;
+ if (server->use_html)
+ wrap_in_html(&msgbuf.buf);
+ lf_to_crlf(&msgbuf.buf);
+
+ curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));
+
+ res = curl_easy_perform(curl);
+
+ if(res != CURLE_OK) {
+ fprintf(stderr, "curl_easy_perform() failed: %s\n",
+ curl_easy_strerror(res));
+ break;
+ }
+
+ n++;
+ }
+ fprintf(stderr, "\n");
+
+ curl_easy_cleanup(curl);
+ curl_global_cleanup();
+
+ return 0;
+}
+#endif
+
+int main(int argc, char **argv)
+{
+ struct strbuf all_msgs = STRBUF_INIT;
+ int total;
int nongit_ok;

git_extract_argv0_path(argv[0]);
@@ -1391,29 +1518,13 @@ int main(int argc, char **argv)
}

/* write it to the imap server */
- ctx = imap_open_store(&server, server.folder);
- if (!ctx) {
- fprintf(stderr, "failed to open store\n");
- return 1;
- }
-
- fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
- while (1) {
- unsigned percent = n * 100 / total;
-
- fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
- if (!split_msg(&all_msgs, &msg, &ofs))
- break;
- if (server.use_html)
- wrap_in_html(&msg);
- r = imap_store_msg(ctx, &msg);
- if (r != DRV_OK)
- break;
- n++;
- }
- fprintf(stderr, "\n");

- imap_close_store(ctx);
+ if (server.tunnel)
+ return append_msgs_to_imap(&server, &all_msgs, total);

- return 0;
+#ifndef USE_CURL_FOR_IMAP_SEND
+ return append_msgs_to_imap(&server, &all_msgs, total);
+#else
+ return curl_append_msgs_to_imap(&server, &all_msgs, total);
+#endif
}

--
2.1.0.372.g32d59d1.dirty

Junio C Hamano

2014-08-27 17:20:40 UTC

https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions
says that this was introduced as of 7.34.0, though.

Post by Bernhard Reiter
As I don't have access to that many IMAP servers, I haven't been able to
test the new code with a wide variety of parameter combinations. I did
test both secure and insecure (imaps:// and imap://) connections and
values of "PLAIN" and "LOGIN" for the authMethod.

Perhaps CC'ing those who have touched git-imap-send code over the
years and asking for their help testing might help?

Post by Bernhard Reiter
---
I rebased the patch on the pu branch, hope that was the right thing to do.

Usually I would appreciate a patch for a new feature not meant for
the maintenance tracks to be based on 'master', so that it can go to
the next release without having to wait other changes that may
conflict with it and that may not yet be ready.

I will try to apply this one to 'pu', rebase it on 'master' to make
sure the result does not depend on the other topics in flight, and
then merge it back to 'pu'.

Thanks; some comments below.

Post by Bernhard Reiter
Documentation/git-imap-send.txt | 3 +-
INSTALL | 15 ++--
Makefile | 16 +++-
git.spec.in | 5 +-
imap-send.c | 165 +++++++++++++++++++++++++++++++++-------
5 files changed, 167 insertions(+), 37 deletions(-)
diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 7d991d9..9d244c4 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
Specify authenticate method for authentication with IMAP server.
- Current supported method is 'CRAM-MD5' only. If this is not set
+ If you compiled git with the NO_CURL option or if your curl version is
+ < 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set
then 'git imap-send' uses the basic IMAP plaintext LOGIN command.

Hmph, so there is no option that lets me say "I know my libcurl is
new enough but I have some reason not to want to use the new code to
interact with my imap server", at compile time or (more preferrably)
at runtime?

Post by Bernhard Reiter
diff --git a/INSTALL b/INSTALL
index 6ec7a24..e2770a0 100644
--- a/INSTALL
+++ b/INSTALL
so you might need to install additional packages other than Perl
itself, e.g. Time::HiRes.
- - "openssl" library is used by git-imap-send to use IMAP over SSL.
- If you don't need it, use NO_OPENSSL.
+ - "openssl" library is used by git-imap-send to use IMAP over SSL,
+ unless you're using curl >= 7.35.0, in which case that will be
+ used. If you don't need git-imap-send, you can use NO_OPENSSL.

The last sentence makes it unclear which of the following is true:

- I have sufficiently new libcurl. I cannot say NO_OPENSSL because
I do need git-imap-send.

- I have sufficiently new libcurl, so "openssl" is not used by
git-imap send for me. I can say NO_OPENSSL.

Perhaps

- git-imap-send needs the OpenSSL library to talk IMAP over SSL if
you are using libCurl older than 7.35.0. Otherwise you can use
NO_OPENSSL without losing git-imap-send.

Post by Bernhard Reiter
diff --git a/git.spec.in b/git.spec.in
index d61d537..9535cc3 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -8,7 +8,7 @@ License: GPL
Group: Development/Tools
URL: http://kernel.org/pub/software/scm/git/
Source: http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
+BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}

This is very iffy. It incompatible with the body of the patch where
you allow older curl library and because you depend on openssl-devel
you wouldn't lose imap-send.

Post by Bernhard Reiter
@@ -1391,29 +1518,13 @@ int main(int argc, char **argv)
}
/* write it to the imap server */
- ctx = imap_open_store(&server, server.folder);
- if (!ctx) {
- fprintf(stderr, "failed to open store\n");
- return 1;
- }
-
- fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
- while (1) {
- unsigned percent = n * 100 / total;
-
- fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
- if (!split_msg(&all_msgs, &msg, &ofs))
- break;
- if (server.use_html)
- wrap_in_html(&msg);
- r = imap_store_msg(ctx, &msg);
- if (r != DRV_OK)
- break;
- n++;
- }
- fprintf(stderr, "\n");
- imap_close_store(ctx);
+ if (server.tunnel)
+ return append_msgs_to_imap(&server, &all_msgs, total);
- return 0;
+#ifndef USE_CURL_FOR_IMAP_SEND
+ return append_msgs_to_imap(&server, &all_msgs, total);
+#else
+ return curl_append_msgs_to_imap(&server, &all_msgs, total);
+#endif
}

Much more nicely done. It appears that you could already turn these
#ifndef/#else/#endif into a runtime conditional, allowing:

- At compile-time, can be built with the four combinations

(1) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=No
(2) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=Yes
(3) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=No
(4) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=Yes

- The first two variants can support --with-curl/--without-curl and
choose between curl_append/append. When run --without-curl, it
may lose some auth-methods and for variant (1) SSL is not
supported.

or am I mis-reading the patch?

Bernhard Reiter

2014-10-12 15:22:20 UTC

Sorry for not getting back to this any sooner, I've been pretty busy
recently with Other Projects(tm).

[...] For now,
the old ones are wrapped in #ifdefs, and the new functions are enabled
by make if curl's version is >= 7.35.0, from which version on curl's
CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been
available.

https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions
says that this was introduced as of 7.34.0, though.

Strange, I thought I recalled having seen that in
http://curl.haxx.se/libcurl/c/CURLOPT_LOGIN_OPTIONS.html but it clearly
says 7.34.0 there too. I've now changed all occurrences of 7.35.0 to
7.34.0 (and the corresponding hex value in the Makefile).

As I don't have access to that many IMAP servers, I haven't been able to
test the new code with a wide variety of parameter combinations. I did
test both secure and insecure (imaps:// and imap://) connections and
values of "PLAIN" and "LOGIN" for the authMethod.

Perhaps CC'ing those who have touched git-imap-send code over the
years and asking for their help testing might help?

CC'ing them (going back about 2 years, which already makes the list
quite long) and the people who have taken part in the initial discussion
on this feature in August. And the related Debian bug.

Please test this, folks!

---
I rebased the patch on the pu branch, hope that was the right thing to do.

Usually I would appreciate a patch for a new feature not meant for
the maintenance tracks to be based on 'master', so that it can go to
the next release without having to wait other changes that may
conflict with it and that may not yet be ready.
I will try to apply this one to 'pu', rebase it on 'master' to make
sure the result does not depend on the other topics in flight, and
then merge it back to 'pu'.

Okay, I'll stick to master. I've rebased on master now that the first
couple related patches are there anyway.

Post by Junio C Hamano
[...]

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 7d991d9..9d244c4 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
Specify authenticate method for authentication with IMAP server.
- Current supported method is 'CRAM-MD5' only. If this is not set
+ If you compiled git with the NO_CURL option or if your curl version is
+ < 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set
then 'git imap-send' uses the basic IMAP plaintext LOGIN command.

Added a runtime option, see below.

diff --git a/INSTALL b/INSTALL
index 6ec7a24..e2770a0 100644
--- a/INSTALL
+++ b/INSTALL
so you might need to install additional packages other than Perl
itself, e.g. Time::HiRes.
- - "openssl" library is used by git-imap-send to use IMAP over SSL.
- If you don't need it, use NO_OPENSSL.
+ - "openssl" library is used by git-imap-send to use IMAP over SSL,
+ unless you're using curl >= 7.35.0, in which case that will be
+ used. If you don't need git-imap-send, you can use NO_OPENSSL.

- I have sufficiently new libcurl. I cannot say NO_OPENSSL because
I do need git-imap-send.
- I have sufficiently new libcurl, so "openssl" is not used by
git-imap send for me. I can say NO_OPENSSL.
Perhaps
- git-imap-send needs the OpenSSL library to talk IMAP over SSL if
you are using libCurl older than 7.35.0. Otherwise you can use
NO_OPENSSL without losing git-imap-send.

Fixed.

diff --git a/git.spec.in b/git.spec.in
index d61d537..9535cc3 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -8,7 +8,7 @@ License: GPL
Group: Development/Tools
URL: http://kernel.org/pub/software/scm/git/
Source: http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
+BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}

This is very iffy. It incompatible with the body of the patch where
you allow older curl library and because you depend on openssl-devel
you wouldn't lose imap-send.

Okay, removed the version requirement.

@@ -1391,29 +1518,13 @@ int main(int argc, char **argv)
[...]

Much more nicely done. It appears that you could already turn these
- At compile-time, can be built with the four combinations
(1) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=No
(2) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=Yes
(3) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=No
(4) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=Yes
- The first two variants can support --with-curl/--without-curl and
choose between curl_append/append. When run --without-curl, it
may lose some auth-methods and for variant (1) SSL is not
supported.
or am I mis-reading the patch?

You're reading it correctly; thanks for the hint about the runtime switch.

I've now implemented it using parse_options, so the switch is called
--[no-]curl. I hope parse_options isn't overkill, but as I wanted to
keep other options -- namely verbose and quiet switches, useful for
debugging curl -- I would've had to look through all argv[] items
manually, so I figured it made sense to us an already existing API for
that instead.

Bernhard

Post by Junio C Hamano
From 218c47580330d5ac38875c30e14dc3049d1ce3c5 Mon Sep 17 00:00:00 2001

From: Bernhard Reiter <***@raz.or.at>
Date: Wed, 13 Aug 2014 23:41:40 +0200
Subject: [PATCH] git-imap-send: use libcurl for implementation

Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.

Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones would reduce imap-send.c by some 1200 lines of code. For now,
the old ones are wrapped in #ifdefs, and the new functions are enabled
by make if curl's version is >= 7.34.0, from which version on curl's
CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been
available. The low-level functions will still be used for tunneling
into the server for now.

As I don't have access to that many IMAP servers, I haven't been able to
test the new code with a wide variety of parameter combinations. I did
test both secure and insecure (imaps:// and imap://) connections and
values of "PLAIN" and "LOGIN" for the authMethod.

Signed-off-by: Bernhard Reiter <***@raz.or.at>
---
Documentation/git-imap-send.txt | 26 +++++-
INSTALL | 15 ++--
Makefile | 16 +++-
imap-send.c | 189 +++++++++++++++++++++++++++++++++-------
4 files changed, 204 insertions(+), 42 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 7d991d9..1c24e1f 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder
SYNOPSIS
--------
[verse]
-'git imap-send'
+'git imap-send' [-v] [-q] --[no-]curl

DESCRIPTION
@@ -25,6 +25,26 @@ Typical usage is something like:

git format-patch --signoff --stdout --attach origin | git imap-send

+OPTIONS
+-------
+-v::
+--verbose::
+ Be verbose.
+
+-q::
+--quiet::
+ Be quiet.
+
+--curl::
+ Use libcurl to communicate with the IMAP server, unless tunneling
+ into it. Only available if git was built with the
+ USE_CURL_FOR_IMAP_SEND option set, in which case this is the
+ default behavior.
+
+--no-curl::
+ Talk to the IMAP server using git's own IMAP routines instead of
+ using libcurl. Only available git was built with the
+ USE_CURL_FOR_IMAP_SEND option set; implicitly assumed otherwise.

CONFIGURATION
-------------
@@ -75,7 +95,9 @@ imap.preformattedHTML::

imap.authMethod::
Specify authenticate method for authentication with IMAP server.
- Current supported method is 'CRAM-MD5' only. If this is not set
+ If git was built with the NO_CURL option, or if your curl version is
+ < 7.34.0, or if you're running git-imap-send with the --no-curl
+ option, the only supported method is 'CRAM-MD5'. If this is not set
then 'git imap-send' uses the basic IMAP plaintext LOGIN command.

Examples
diff --git a/INSTALL b/INSTALL
index 6ec7a24..ffb071e 100644
--- a/INSTALL
+++ b/INSTALL
@@ -108,18 +108,21 @@ Issues of note:
so you might need to install additional packages other than Perl
itself, e.g. Time::HiRes.

- - "openssl" library is used by git-imap-send to use IMAP over SSL.
- If you don't need it, use NO_OPENSSL.
+ - git-imap-send needs the OpenSSL library to talk IMAP over SSL if
+ you are using libcurl older than 7.34.0. Otherwise you can use
+ NO_OPENSSL without losing git-imap-send.

By default, git uses OpenSSL for SHA1 but it will use its own
library (inspired by Mozilla's) with either NO_OPENSSL or
BLK_SHA1. Also included is a version optimized for PowerPC
(PPC_SHA1).

- - "libcurl" library is used by git-http-fetch and git-fetch. You
- might also want the "curl" executable for debugging purposes.
- If you do not use http:// or https:// repositories, you do not
- have to have them (use NO_CURL).
+ - "libcurl" library is used by git-http-fetch, git-fetch, and, if
+ the curl version >= 7.34.0, for git-imap-send. You might also
+ want the "curl" executable for debugging purposes. If you do not
+ use http:// or https:// repositories, and do not want to put
+ patches into an IMAP mailbox, you do not have to have them
+ (use NO_CURL).

- "expat" library; git-http-push uses it for remote lock
management over DAV. Similar to "curl" above, this is optional
diff --git a/Makefile b/Makefile
index f34a2d4..69b2fbf 100644
--- a/Makefile
+++ b/Makefile
@@ -992,6 +992,9 @@ ifdef HAVE_ALLOCA_H
BASIC_CFLAGS += -DHAVE_ALLOCA_H
endif

+IMAP_SEND_BUILDDEPS =
+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+
ifdef NO_CURL
BASIC_CFLAGS += -DNO_CURL
REMOTE_CURL_PRIMARY =
@@ -1026,6 +1029,15 @@ else
PROGRAM_OBJS += http-push.o
endif
endif
+ curl_check := $(shell (echo 072200; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)
+ ifeq "$(curl_check)" "072200"
+ USE_CURL_FOR_IMAP_SEND = YesPlease
+ endif
+ ifdef USE_CURL_FOR_IMAP_SEND
+ BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND
+ IMAP_SEND_BUILDDEPS = http.o
+ IMAP_SEND_LDFLAGS += $(CURL_LIBCURL)
+ endif
ifndef NO_EXPAT
ifdef EXPATDIR
BASIC_CFLAGS += -I$(EXPATDIR)/include
@@ -1892,9 +1904,9 @@ endif
git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)

-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
- $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+ $(LIBS) $(IMAP_SEND_LDFLAGS)

git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
diff --git a/imap-send.c b/imap-send.c
index 70bcc7a..9cc80ae 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -26,11 +26,31 @@
#include "credential.h"
#include "exec_cmd.h"
#include "run-command.h"
+#include "parse-options.h"
#ifdef NO_OPENSSL
typedef void *SSL;
#endif
+#ifdef USE_CURL_FOR_IMAP_SEND
+#include "http.h"
+#endif

-static const char imap_send_usage[] = "git imap-send < <mbox>";
+static int Verbose, Quiet;
+#ifdef USE_CURL_FOR_IMAP_SEND
+static int use_curl = 1; // on by default; set later by parse_options
+#else
+static int use_curl = 0; // not available
+#endif
+
+static const char * const imap_send_usage[] = { "git imap-send <options> < <mbox>", NULL };
+
+static struct option imap_send_options[] = {
+#ifdef USE_CURL_FOR_IMAP_SEND
+ OPT_BOOL(0, "curl", &use_curl, "use libcurl to communicate with the IMAP server"),
+#endif
+ OPT__VERBOSE(&Verbose, "be verbose"),
+ OPT__QUIET(&Quiet, "be quiet"),
+ OPT_END()
+};

#undef DRV_OK
#define DRV_OK 0
@@ -38,8 +58,6 @@ static const char imap_send_usage[] = "git imap-send < <mbox>";
#define DRV_BOX_BAD -2
#define DRV_STORE_BAD -3

-static int Verbose, Quiet;
-
__attribute__((format (printf, 1, 2)))
static void imap_info(const char *, ...);
__attribute__((format (printf, 1, 2)))
@@ -1338,22 +1356,145 @@ static void git_imap_config(void)
git_config_get_string("imap.authmethod", &server.auth_method);
}

-int main(int argc, char **argv)
-{
- struct strbuf all_msgs = STRBUF_INIT;
+static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {
struct strbuf msg = STRBUF_INIT;
struct imap_store *ctx = NULL;
int ofs = 0;
int r;
- int total, n = 0;
+ int n = 0;
+
+ ctx = imap_open_store(server, server->folder);
+ if (!ctx) {
+ fprintf(stderr, "failed to open store\n");
+ return 1;
+ }
+ ctx->name = server->folder;
+
+ fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
+ while (1) {
+ unsigned percent = n * 100 / total;
+
+ fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+
+ if (!split_msg(all_msgs, &msg, &ofs))
+ break;
+ if (server->use_html)
+ wrap_in_html(&msg);
+ r = imap_store_msg(ctx, &msg);
+ if (r != DRV_OK)
+ break;
+ n++;
+ }
+ fprintf(stderr, "\n");
+
+ imap_close_store(ctx);
+
+ return 0;
+}
+
+#ifdef USE_CURL_FOR_IMAP_SEND
+static CURL *setup_curl(struct imap_server_conf *srvc)
+{
+ CURL *curl;
+ struct strbuf path = STRBUF_INIT;
+ struct strbuf auth = STRBUF_INIT;
+
+ if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
+ die("curl_global_init failed");
+
+ curl = curl_easy_init();
+
+ if (!curl)
+ die("curl_easy_init failed");
+
+ curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
+ curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
+
+ strbuf_addstr(&path, server.host);
+ if (!path.len || path.buf[path.len - 1] != '/')
+ strbuf_addch(&path, '/');
+ strbuf_addstr(&path, server.folder);
+
+ curl_easy_setopt(curl, CURLOPT_URL, path.buf);
+ curl_easy_setopt(curl, CURLOPT_PORT, server.port);
+
+ if (server.auth_method) {
+ strbuf_addstr(&auth, "AUTH=");
+ strbuf_addstr(&auth, server.auth_method);
+ curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);
+ }
+
+ if (server.use_ssl)
+ curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);
+
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);
+
+ curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
+
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
+
+ if (Verbose)
+ curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
+
+ return curl;
+}
+
+static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {
+ int ofs = 0;
+ int n = 0;
+ struct buffer msgbuf = { STRBUF_INIT, 0 };
+ CURL *curl;
+ CURLcode res = CURLE_OK;
+
+ curl = setup_curl(server);
+ curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
+
+ fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
+ while (1) {
+ unsigned percent = n * 100 / total;
+
+ fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+
+ int prev_len = msgbuf.buf.len;
+ if (!split_msg(all_msgs, &msgbuf.buf, &ofs))
+ break;
+ if (server->use_html)
+ wrap_in_html(&msgbuf.buf);
+ lf_to_crlf(&msgbuf.buf);
+
+ curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));
+
+ res = curl_easy_perform(curl);
+
+ if(res != CURLE_OK) {
+ fprintf(stderr, "curl_easy_perform() failed: %s\n",
+ curl_easy_strerror(res));
+ break;
+ }
+
+ n++;
+ }
+ fprintf(stderr, "\n");
+
+ curl_easy_cleanup(curl);
+ curl_global_cleanup();
+
+ return 0;
+}
+#endif
+
+int main(int argc, char **argv)
+{
+ struct strbuf all_msgs = STRBUF_INIT;
+ int total;
int nongit_ok;

git_extract_argv0_path(argv[0]);

- git_setup_gettext();
+ argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);

- if (argc != 1)
- usage(imap_send_usage);
+ git_setup_gettext();

setup_git_directory_gently(&nongit_ok);
git_imap_config();
@@ -1391,29 +1532,13 @@ int main(int argc, char **argv)
}

/* write it to the imap server */
- ctx = imap_open_store(&server, server.folder);
- if (!ctx) {
- fprintf(stderr, "failed to open store\n");
- return 1;
- }

- fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
- while (1) {
- unsigned percent = n * 100 / total;
+ if (server.tunnel)
+ return append_msgs_to_imap(&server, &all_msgs, total);

- fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
- if (!split_msg(&all_msgs, &msg, &ofs))
- break;
- if (server.use_html)
- wrap_in_html(&msg);
- r = imap_store_msg(ctx, &msg);
- if (r != DRV_OK)
- break;
- n++;
+ if (use_curl) {
+ return curl_append_msgs_to_imap(&server, &all_msgs, total);
+ } else {
+ return append_msgs_to_imap(&server, &all_msgs, total);
}
- fprintf(stderr, "\n");
-
- imap_close_store(ctx);
-
- return 0;
}

--
2.1.2.375.g218c475

Bernhard Reiter

2014-10-22 15:05:12 UTC

*ping*
Hope I didn't mess up formatting again...
Or do I need to top-post, as the original thread is too old to keep pos=
ting to it?

Bernhard

-------- Weitergeleitete Nachricht --------
Betreff: Re: [PATCH] git-imap-send: use libcurl for implementation
Datum: Sun, 12 Oct 2014 17:22:20 +0200
Von: Bernhard Reiter <***@raz.or.at>
An: Junio C Hamano <***@pobox.com>
Kopie (CC): ***@vger.kernel.org, Jonathan Nieder <***@gmail.com>, =
Jeff King <***@peff.net>, ***@bugs.debian.org, Ren=E9 Scharfe <l.s.=
***@web.de>, Tony Finch <***@dotat.at>, Tanay Abhra <***@gmail.com>,=
Dan Albert <***@google.com>, Jeremy Huddleston <***@apple.c=
om>, David Aguilar <***@gmail.com>, Michael Haggerty <***@alum.m=
it.edu>, Oswald Buddenhagen <***@kde.org>

Sorry for not getting back to this any sooner, I've been pretty busy
recently with Other Projects(tm).

=20

[...] For now,
the old ones are wrapped in #ifdefs, and the new functions are enabl=

by make if curl's version is >=3D 7.35.0, from which version on curl=

CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has b=

een

available.

=20
https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-ve=

rsions

says that this was introduced as of 7.34.0, though.

As I don't have access to that many IMAP servers, I haven't been abl=

e to

test the new code with a wide variety of parameter combinations. I d=

test both secure and insecure (imaps:// and imap://) connections and
values of "PLAIN" and "LOGIN" for the authMethod.

=20
Perhaps CC'ing those who have touched git-imap-send code over the
years and asking for their help testing might help?

CC'ing them (going back about 2 years, which already makes the list
quite long) and the people who have taken part in the initial discussio=
n
on this feature in August. And the related Debian bug.

Please test this, folks!

---
I rebased the patch on the pu branch, hope that was the right thing =

to do.

=20
Usually I would appreciate a patch for a new feature not meant for
the maintenance tracks to be based on 'master', so that it can go to
the next release without having to wait other changes that may
conflict with it and that may not yet be ready.
=20
I will try to apply this one to 'pu', rebase it on 'master' to make
sure the result does not depend on the other topics in flight, and
then merge it back to 'pu'.

Okay, I'll stick to master. I've rebased on master now that the first
couple related patches are there anyway.

[...]

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-ima=

p-send.txt

index 7d991d9..9d244c4 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
=20
Specify authenticate method for authentication with IMAP server.
- Current supported method is 'CRAM-MD5' only. If this is not set
+ If you compiled git with the NO_CURL option or if your curl versio=

n is

+ < 7.35.0, the only supported method is 'CRAM-MD5'. If this is not =

set

then 'git imap-send' uses the basic IMAP plaintext LOGIN command.

=20
Hmph, so there is no option that lets me say "I know my libcurl is
new enough but I have some reason not to want to use the new code to
interact with my imap server", at compile time or (more preferrably)
at runtime?

Added a runtime option, see below.

diff --git a/INSTALL b/INSTALL
index 6ec7a24..e2770a0 100644
--- a/INSTALL
+++ b/INSTALL
so you might need to install additional packages other than Perl
itself, e.g. Time::HiRes.
=20
- - "openssl" library is used by git-imap-send to use IMAP over SSL.
- If you don't need it, use NO_OPENSSL.
+ - "openssl" library is used by git-imap-send to use IMAP over SSL,
+ unless you're using curl >=3D 7.35.0, in which case that will be
+ used. If you don't need git-imap-send, you can use NO_OPENSSL.

=20
=20
- I have sufficiently new libcurl. I cannot say NO_OPENSSL because
I do need git-imap-send.
=20
- I have sufficiently new libcurl, so "openssl" is not used by
git-imap send for me. I can say NO_OPENSSL.
=20
Perhaps
=20
- git-imap-send needs the OpenSSL library to talk IMAP over SSL if
you are using libCurl older than 7.35.0. Otherwise you can use
NO_OPENSSL without losing git-imap-send.

=46ixed.

tar.gz

-BuildRequires: zlib-devel >=3D 1.2, openssl-devel, curl-devel, expa=

t-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}

+BuildRequires: zlib-devel >=3D 1.2, openssl-devel, curl-devel >=3D =

7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0=
=2E3}

=20
This is very iffy. It incompatible with the body of the patch where
you allow older curl library and because you depend on openssl-devel
you wouldn't lose imap-send.

Okay, removed the version requirement.

@@ -1391,29 +1518,13 @@ int main(int argc, char **argv)
[...]

=20
Much more nicely done. It appears that you could already turn these
=20
- At compile-time, can be built with the four combinations
=20
(1) USE_CURL_FOR_IMAP_SEND=3DYes NO_OPENSSL=3DNo
(2) USE_CURL_FOR_IMAP_SEND=3DYes NO_OPENSSL=3DYes
(3) USE_CURL_FOR_IMAP_SEND=3DNo NO_OPENSSL=3DNo
(4) USE_CURL_FOR_IMAP_SEND=3DNo NO_OPENSSL=3DYes
=20
- The first two variants can support --with-curl/--without-curl and
choose between curl_append/append. When run --without-curl, it
may lose some auth-methods and for variant (1) SSL is not
supported.
=20
or am I mis-reading the patch?

You're reading it correctly; thanks for the hint about the runtime swit=
ch.

I've now implemented it using parse_options, so the switch is called
--[no-]curl. I hope parse_options isn't overkill, but as I wanted to
keep other options -- namely verbose and quiet switches, useful for
debugging curl -- I would've had to look through all argv[] items
manually, so I figured it made sense to us an already existing API for
that instead.

Bernhard

From 218c47580330d5ac38875c30e14dc3049d1ce3c5 Mon Sep 17 00:00:00 2001

=46rom: Bernhard Reiter <***@raz.or.at>
Date: Wed, 13 Aug 2014 23:41:40 +0200
Subject: [PATCH] git-imap-send: use libcurl for implementation

Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.

Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones would reduce imap-send.c by some 1200 lines of code. For now,
the old ones are wrapped in #ifdefs, and the new functions are enabled
by make if curl's version is >=3D 7.34.0, from which version on curl's
CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been
available. The low-level functions will still be used for tunneling
into the server for now.

As I don't have access to that many IMAP servers, I haven't been able t=
o
test the new code with a wide variety of parameter combinations. I did
test both secure and insecure (imaps:// and imap://) connections and
values of "PLAIN" and "LOGIN" for the authMethod.

Signed-off-by: Bernhard Reiter <***@raz.or.at>
---
Documentation/git-imap-send.txt | 26 +++++-
INSTALL | 15 ++--
Makefile | 16 +++-
imap-send.c | 189 ++++++++++++++++++++++++++++++++=
+-------
4 files changed, 204 insertions(+), 42 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-s=
end.txt
index 7d991d9..1c24e1f 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin=
to an IMAP folder
SYNOPSIS
--------
[verse]
-'git imap-send'
+'git imap-send' [-v] [-q] --[no-]curl
=20
=20
DESCRIPTION
@@ -25,6 +25,26 @@ Typical usage is something like:
=20
git format-patch --signoff --stdout --attach origin | git imap-send
=20
+OPTIONS
+-------
+-v::
+--verbose::
+ Be verbose.
+
+-q::
+--quiet::
+ Be quiet.
+
+--curl::
+ Use libcurl to communicate with the IMAP server, unless tunneling
+ into it. Only available if git was built with the
+ USE_CURL_FOR_IMAP_SEND option set, in which case this is the
+ default behavior.
+
+--no-curl::
+ Talk to the IMAP server using git's own IMAP routines instead of
+ using libcurl. Only available git was built with the
+ USE_CURL_FOR_IMAP_SEND option set; implicitly assumed otherwise.
=20
CONFIGURATION
-------------
@@ -75,7 +95,9 @@ imap.preformattedHTML::
=20
imap.authMethod::
Specify authenticate method for authentication with IMAP server.
- Current supported method is 'CRAM-MD5' only. If this is not set
+ If git was built with the NO_CURL option, or if your curl version is
+ < 7.34.0, or if you're running git-imap-send with the --no-curl
+ option, the only supported method is 'CRAM-MD5'. If this is not set
then 'git imap-send' uses the basic IMAP plaintext LOGIN command.
=20
Examples
diff --git a/INSTALL b/INSTALL
index 6ec7a24..ffb071e 100644
--- a/INSTALL
+++ b/INSTALL
@@ -108,18 +108,21 @@ Issues of note:
so you might need to install additional packages other than Perl
itself, e.g. Time::HiRes.
=20
- - "openssl" library is used by git-imap-send to use IMAP over SSL.
- If you don't need it, use NO_OPENSSL.
+ - git-imap-send needs the OpenSSL library to talk IMAP over SSL if
+ you are using libcurl older than 7.34.0. Otherwise you can use
+ NO_OPENSSL without losing git-imap-send.
=20
By default, git uses OpenSSL for SHA1 but it will use its own
library (inspired by Mozilla's) with either NO_OPENSSL or
BLK_SHA1. Also included is a version optimized for PowerPC
(PPC_SHA1).
=20
- - "libcurl" library is used by git-http-fetch and git-fetch. You
- might also want the "curl" executable for debugging purposes.
- If you do not use http:// or https:// repositories, you do not
- have to have them (use NO_CURL).
+ - "libcurl" library is used by git-http-fetch, git-fetch, and, if
+ the curl version >=3D 7.34.0, for git-imap-send. You might also
+ want the "curl" executable for debugging purposes. If you do not
+ use http:// or https:// repositories, and do not want to put
+ patches into an IMAP mailbox, you do not have to have them
+ (use NO_CURL).
=20
- "expat" library; git-http-push uses it for remote lock
management over DAV. Similar to "curl" above, this is optional
diff --git a/Makefile b/Makefile
index f34a2d4..69b2fbf 100644
--- a/Makefile
+++ b/Makefile
@@ -992,6 +992,9 @@ ifdef HAVE_ALLOCA_H
BASIC_CFLAGS +=3D -DHAVE_ALLOCA_H
endif
=20
+IMAP_SEND_BUILDDEPS =3D
+IMAP_SEND_LDFLAGS =3D $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO=
)
+
ifdef NO_CURL
BASIC_CFLAGS +=3D -DNO_CURL
REMOTE_CURL_PRIMARY =3D
@@ -1026,6 +1029,15 @@ else
PROGRAM_OBJS +=3D http-push.o
endif
endif
+ curl_check :=3D $(shell (echo 072200; curl-config --vernum) 2>/dev/nu=
ll | sort -r | sed -ne 2p)
+ ifeq "$(curl_check)" "072200"
+ USE_CURL_FOR_IMAP_SEND =3D YesPlease
+ endif
+ ifdef USE_CURL_FOR_IMAP_SEND
+ BASIC_CFLAGS +=3D -DUSE_CURL_FOR_IMAP_SEND
+ IMAP_SEND_BUILDDEPS =3D http.o
+ IMAP_SEND_LDFLAGS +=3D $(CURL_LIBCURL)
+ endif
ifndef NO_EXPAT
ifdef EXPATDIR
BASIC_CFLAGS +=3D -I$(EXPATDIR)/include
@@ -1892,9 +1904,9 @@ endif
git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^=
) $(LIBS)
=20
-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITL=
IBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^=
) \
- $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+ $(LIBS) $(IMAP_SEND_LDFLAGS)
=20
git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITL=
IBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^=
) \
diff --git a/imap-send.c b/imap-send.c
index 70bcc7a..9cc80ae 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -26,11 +26,31 @@
#include "credential.h"
#include "exec_cmd.h"
#include "run-command.h"
+#include "parse-options.h"
#ifdef NO_OPENSSL
typedef void *SSL;
#endif
+#ifdef USE_CURL_FOR_IMAP_SEND
+#include "http.h"
+#endif
=20
-static const char imap_send_usage[] =3D "git imap-send < <mbox>";
+static int Verbose, Quiet;
+#ifdef USE_CURL_FOR_IMAP_SEND
+static int use_curl =3D 1; // on by default; set later by parse_option=
s
+#else
+static int use_curl =3D 0; // not available
+#endif
+
+static const char * const imap_send_usage[] =3D { "git imap-send <opti=
ons> < <mbox>", NULL };
+
+static struct option imap_send_options[] =3D {
+#ifdef USE_CURL_FOR_IMAP_SEND
+ OPT_BOOL(0, "curl", &use_curl, "use libcurl to communicate with the I=
MAP server"),
+#endif
+ OPT__VERBOSE(&Verbose, "be verbose"),
+ OPT__QUIET(&Quiet, "be quiet"),
+ OPT_END()
+};
=20
#undef DRV_OK
#define DRV_OK 0
@@ -38,8 +58,6 @@ static const char imap_send_usage[] =3D "git imap-sen=
d < <mbox>";
#define DRV_BOX_BAD -2
#define DRV_STORE_BAD -3
=20
-static int Verbose, Quiet;
-
__attribute__((format (printf, 1, 2)))
static void imap_info(const char *, ...);
__attribute__((format (printf, 1, 2)))
@@ -1338,22 +1356,145 @@ static void git_imap_config(void)
git_config_get_string("imap.authmethod", &server.auth_method);
}
=20
-int main(int argc, char **argv)
-{
- struct strbuf all_msgs =3D STRBUF_INIT;
+static int append_msgs_to_imap(struct imap_server_conf *server, struct=
strbuf* all_msgs, int total) {
struct strbuf msg =3D STRBUF_INIT;
struct imap_store *ctx =3D NULL;
int ofs =3D 0;
int r;
- int total, n =3D 0;
+ int n =3D 0;
+
+ ctx =3D imap_open_store(server, server->folder);
+ if (!ctx) {
+ fprintf(stderr, "failed to open store\n");
+ return 1;
+ }
+ ctx->name =3D server->folder;
+
+ fprintf(stderr, "sending %d message%s\n", total, (total !=3D 1) ? "s"=
: "");
+ while (1) {
+ unsigned percent =3D n * 100 / total;
+
+ fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+
+ if (!split_msg(all_msgs, &msg, &ofs))
+ break;
+ if (server->use_html)
+ wrap_in_html(&msg);
+ r =3D imap_store_msg(ctx, &msg);
+ if (r !=3D DRV_OK)
+ break;
+ n++;
+ }
+ fprintf(stderr, "\n");
+
+ imap_close_store(ctx);
+
+ return 0;
+}
+
+#ifdef USE_CURL_FOR_IMAP_SEND
+static CURL *setup_curl(struct imap_server_conf *srvc)
+{
+ CURL *curl;
+ struct strbuf path =3D STRBUF_INIT;
+ struct strbuf auth =3D STRBUF_INIT;
+
+ if (curl_global_init(CURL_GLOBAL_ALL) !=3D CURLE_OK)
+ die("curl_global_init failed");
+
+ curl =3D curl_easy_init();
+
+ if (!curl)
+ die("curl_easy_init failed");
+
+ curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
+ curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
+
+ strbuf_addstr(&path, server.host);
+ if (!path.len || path.buf[path.len - 1] !=3D '/')
+ strbuf_addch(&path, '/');
+ strbuf_addstr(&path, server.folder);
+
+ curl_easy_setopt(curl, CURLOPT_URL, path.buf);
+ curl_easy_setopt(curl, CURLOPT_PORT, server.port);
+
+ if (server.auth_method) {
+ strbuf_addstr(&auth, "AUTH=3D");
+ strbuf_addstr(&auth, server.auth_method);
+ curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);
+ }
+
+ if (server.use_ssl)
+ curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);
+
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);
+ curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);
+
+ curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
+
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
+
+ if (Verbose)
+ curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
+
+ return curl;
+}
+
+static int curl_append_msgs_to_imap(struct imap_server_conf *server, s=
truct strbuf* all_msgs, int total) {
+ int ofs =3D 0;
+ int n =3D 0;
+ struct buffer msgbuf =3D { STRBUF_INIT, 0 };
+ CURL *curl;
+ CURLcode res =3D CURLE_OK;
+
+ curl =3D setup_curl(server);
+ curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
+
+ fprintf(stderr, "sending %d message%s\n", total, (total !=3D 1) ? "s"=
: "");
+ while (1) {
+ unsigned percent =3D n * 100 / total;
+
+ fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+
+ int prev_len =3D msgbuf.buf.len;
+ if (!split_msg(all_msgs, &msgbuf.buf, &ofs))
+ break;
+ if (server->use_html)
+ wrap_in_html(&msgbuf.buf);
+ lf_to_crlf(&msgbuf.buf);
+
+ curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf=
=2Ebuf.len-prev_len));
+
+ res =3D curl_easy_perform(curl);
+
+ if(res !=3D CURLE_OK) {
+ fprintf(stderr, "curl_easy_perform() failed: %s\n",
+ curl_easy_strerror(res));
+ break;
+ }
+
+ n++;
+ }
+ fprintf(stderr, "\n");
+
+ curl_easy_cleanup(curl);
+ curl_global_cleanup();
+
+ return 0;
+}
+#endif
+
+int main(int argc, char **argv)
+{
+ struct strbuf all_msgs =3D STRBUF_INIT;
+ int total;
int nongit_ok;
=20
git_extract_argv0_path(argv[0]);
=20
- git_setup_gettext();
+ argc =3D parse_options(argc, (const char **)argv, "", imap_send_optio=
ns, imap_send_usage, 0);
=20
- if (argc !=3D 1)
- usage(imap_send_usage);
+ git_setup_gettext();
=20
setup_git_directory_gently(&nongit_ok);
git_imap_config();
@@ -1391,29 +1532,13 @@ int main(int argc, char **argv)
}
=20
/* write it to the imap server */
- ctx =3D imap_open_store(&server, server.folder);
- if (!ctx) {
- fprintf(stderr, "failed to open store\n");
- return 1;
- }
=20
- fprintf(stderr, "sending %d message%s\n", total, (total !=3D 1) ? "s"=
: "");
- while (1) {
- unsigned percent =3D n * 100 / total;
+ if (server.tunnel)
+ return append_msgs_to_imap(&server, &all_msgs, total);
=20
- fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
- if (!split_msg(&all_msgs, &msg, &ofs))
- break;
- if (server.use_html)
- wrap_in_html(&msg);
- r =3D imap_store_msg(ctx, &msg);
- if (r !=3D DRV_OK)
- break;
- n++;
+ if (use_curl) {
+ return curl_append_msgs_to_imap(&server, &all_msgs, total);
+ } else {
+ return append_msgs_to_imap(&server, &all_msgs, total);
}
- fprintf(stderr, "\n");
-
- imap_close_store(ctx);
-
- return 0;
}
--=20
2.1.2.375.g218c475

Junio C Hamano

2014-10-22 17:47:27 UTC