{"thread":{"id":"37420","subject":"[PATCH] git-imap-send: use libcurl for implementation","startedAt":"2014-08-26T22:40:17Z","lastAt":"2014-11-10T17:19:00Z","messageCount":12,"participants":["Bernhard Reiter","Junio C Hamano","Torsten Bögershausen","Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"248356","messageId":"53FD0CD1.7030801@raz.or.at","threadId":"37420","inReplyTo":null,"subject":"[PATCH] git-imap-send: use libcurl for implementation","fromName":"Bernhard Reiter","fromEmail":"ockham@raz.or.at","sentAt":"2014-08-26T22:40:17Z","receivedAt":"2014-08-26T22:40:17Z","isPatch":true,"sender":{"key":"ockham@raz.or.at","avatar":"https://avatars.githubusercontent.com/u/96308?v=4"},"body":"Use libcurl's high-level API functions to implement git-imap-send\ninstead of the previous low-level OpenSSL-based functions.\n\nSince version 7.30.0, libcurl's API has been able to communicate with\nIMAP servers. Using those high-level functions instead of the current\nones would reduce imap-send.c by some 1200 lines of code. For now,\nthe old ones are wrapped in #ifdefs, and the new functions are enabled\nby make if curl's version is >= 7.35.0, from which version on curl's\nCURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\navailable.\n\nAs I don't have access to that many IMAP servers, I haven't been able to\ntest the new code with a wide variety of parameter combinations. I did\ntest both secure and insecure (imaps:// and imap://) connections and\nvalues of \"PLAIN\" and \"LOGIN\" for the authMethod.\n\nSigned-off-by: Bernhard Reiter \n---\nI rebased the patch on the pu branch, hope that was the right thing to do.\n\nBernhard\n\n Documentation/git-imap-send.txt | 3 +-\n INSTALL | 15 ++--\n Makefile | 16 +++-\n git.spec.in | 5 +-\n imap-send.c | 165 +++++++++++++++++++++++++++++++++-------\n 5 files changed, 167 insertions(+), 37 deletions(-)\n\ndiff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\nindex 7d991d9..9d244c4 100644\n--- a/Documentation/git-imap-send.txt\n+++ b/Documentation/git-imap-send.txt\n@@ -75,7 +75,8 @@ imap.preformattedHTML::\n \n imap.authMethod::\n \tSpecify authenticate method for authentication with IMAP server.\n-\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n+\tIf you compiled git with the NO_CURL option or if your curl version is\n+\t< 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set\n \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n \n Examples\ndiff --git a/INSTALL b/INSTALL\nindex 6ec7a24..e2770a0 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -108,18 +108,21 @@ Issues of note:\n \t so you might need to install additional packages other than Perl\n \t itself, e.g. Time::HiRes.\n \n-\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n-\t If you don't need it, use NO_OPENSSL.\n+\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL,\n+\t unless you're using curl >= 7.35.0, in which case that will be\n+\t used. If you don't need git-imap-send, you can use NO_OPENSSL.\n \n \t By default, git uses OpenSSL for SHA1 but it will use its own\n \t library (inspired by Mozilla's) with either NO_OPENSSL or\n \t BLK_SHA1. Also included is a version optimized for PowerPC\n \t (PPC_SHA1).\n \n-\t- \"libcurl\" library is used by git-http-fetch and git-fetch. You\n-\t might also want the \"curl\" executable for debugging purposes.\n-\t If you do not use http:// or https:// repositories, you do not\n-\t have to have them (use NO_CURL).\n+\t- \"libcurl\" library is used by git-http-fetch, git-fetch, and, if\n+\t the curl version >= 7.35.0, for git-imap-send. You might also\n+\t want the \"curl\" executable for debugging purposes. If you do not\n+\t use http:// or https:// repositories, and do not want to put\n+\t patches into an IMAP mailbox, you do not have to have them\n+\t (use NO_CURL).\n \n \t- \"expat\" library; git-http-push uses it for remote lock\n \t management over DAV. Similar to \"curl\" above, this is optional\ndiff --git a/Makefile b/Makefile\nindex 237bc05..c08963c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1133,6 +1133,9 @@ ifdef HAVE_ALLOCA_H\n \tBASIC_CFLAGS += -DHAVE_ALLOCA_H\n endif\n \n+IMAP_SEND_BUILDDEPS =\n+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\n ifdef NO_CURL\n \tBASIC_CFLAGS += -DNO_CURL\n \tREMOTE_CURL_PRIMARY =\n@@ -1167,6 +1170,15 @@ else\n \t\t\tPROGRAM_OBJS += http-push.o\n \t\tendif\n \tendif\n+\tcurl_check := $(shell (echo 072300; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)\n+\tifeq \"$(curl_check)\" \"072300\"\n+\t\tUSE_CURL_FOR_IMAP_SEND = YesPlease\n+\tendif\n+\tifdef USE_CURL_FOR_IMAP_SEND\n+\t\tBASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND\n+\t\tIMAP_SEND_BUILDDEPS = http.o\n+\t\tIMAP_SEND_LDFLAGS += $(CURL_LIBCURL)\n+\tendif\n \tifndef NO_EXPAT\n \t\tifdef EXPATDIR\n \t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n@@ -2084,9 +2096,9 @@ endif\n git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n \n-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\t\t$(LIBS) $(IMAP_SEND_LDFLAGS)\n \n git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\ndiff --git a/git.spec.in b/git.spec.in\nindex d61d537..9535cc3 100644\n--- a/git.spec.in\n+++ b/git.spec.in\n@@ -8,7 +8,7 @@ License: \tGPL\n Group: \t\tDevelopment/Tools\n URL: \t\thttp://kernel.org/pub/software/scm/git/\n Source: \thttp://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz\n-BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n+BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n BuildRoot:\t%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)\n \n Requires:\tperl-Git = %{version}-%{release}\n@@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT\n # No files for you!\n \n %changelog\n+* Mon Aug 11 2014 Bernhard Reiter \n+- Require version >= 7.35.0 of curl-devel for IMAP functions.\n+\n * Sun Sep 18 2011 Jakub Narebski \n - Add gitweb manpages to 'gitweb' subpackage\n \ndiff --git a/imap-send.c b/imap-send.c\nindex ad330a6..a170cbb 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -29,6 +29,9 @@\n #ifdef NO_OPENSSL\n typedef void *SSL;\n #endif\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+#include \"http.h\"\n+#endif\n \n static const char imap_send_usage[] = \"git imap-send < \";\n \n@@ -1338,14 +1341,138 @@ static void git_imap_config(void)\n \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n }\n \n-int main(int argc, char **argv)\n-{\n-\tstruct strbuf all_msgs = STRBUF_INIT;\n+static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n \tstruct strbuf msg = STRBUF_INIT;\n \tstruct imap_store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n-\tint total, n = 0;\n+\tint n = 0;\n+\n+\tctx = imap_open_store(server, server->folder);\n+\tif (!ctx) {\n+\t\tfprintf(stderr, \"failed to open store\\n\");\n+\t\treturn 1;\n+\t}\n+\tctx->name = server->folder;\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tif (!split_msg(all_msgs, &msg, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msg);\n+\t\tr = imap_store_msg(ctx, &msg);\n+\t\tif (r != DRV_OK)\n+\t\t\tbreak;\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\timap_close_store(ctx);\n+\n+\treturn 0;\n+}\n+\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static CURL *setup_curl(struct imap_server_conf *srvc)\n+{\n+\tCURL *curl;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf auth = STRBUF_INIT;\n+\n+\tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n+\t\tdie(\"curl_global_init failed\");\n+\n+\tcurl = curl_easy_init();\n+\n+\tif (!curl)\n+\t\tdie(\"curl_easy_init failed\");\n+\n+\tcurl_easy_setopt(curl, CURLOPT_USERNAME, server.user);\n+\tcurl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);\n+\n+\tstrbuf_addstr(&path, server.host);\n+\tif (!path.len || path.buf[path.len - 1] != '/')\n+\t\tstrbuf_addch(&path, '/');\n+\tstrbuf_addstr(&path, server.folder);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n+\tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n+\n+\tif (server.auth_method) {\n+\t\tstrbuf_addstr(&auth, \"AUTH=\");\n+\t\tstrbuf_addstr(&auth, server.auth_method);\n+\t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n+\t}\n+\n+\tif (server.use_ssl)\n+\t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n+\n+\tif (Verbose)\n+\t\tcurl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);\n+\n+\treturn curl;\n+}\n+\n+static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n+\tint ofs = 0;\n+\tint n = 0;\n+\tstruct buffer msgbuf = { STRBUF_INIT, 0 };\n+\tCURL *curl;\n+\tCURLcode res = CURLE_OK;\n+\n+\tcurl = setup_curl(server);\n+\tcurl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tint prev_len = msgbuf.buf.len;\n+\t\tif (!split_msg(all_msgs, &msgbuf.buf, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msgbuf.buf);\n+\t\tlf_to_crlf(&msgbuf.buf);\n+\n+\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));\n+\n+\t\tres = curl_easy_perform(curl);\n+\n+\t\tif(res != CURLE_OK) {\n+\t\t\tfprintf(stderr, \"curl_easy_perform() failed: %s\\n\",\n+\t\t\t\t\tcurl_easy_strerror(res));\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\tcurl_easy_cleanup(curl);\n+\tcurl_global_cleanup();\n+\n+\treturn 0;\n+}\n+#endif\n+\n+int main(int argc, char **argv)\n+{\n+\tstruct strbuf all_msgs = STRBUF_INIT;\n+\tint total;\n \tint nongit_ok;\n \n \tgit_extract_argv0_path(argv[0]);\n@@ -1391,29 +1518,13 @@ int main(int argc, char **argv)\n \t}\n \n \t/* write it to the imap server */\n-\tctx = imap_open_store(&server, server.folder);\n-\tif (!ctx) {\n-\t\tfprintf(stderr, \"failed to open store\\n\");\n-\t\treturn 1;\n-\t}\n-\n-\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n-\twhile (1) {\n-\t\tunsigned percent = n * 100 / total;\n-\n-\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n-\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n-\t\t\tbreak;\n-\t\tif (server.use_html)\n-\t\t\twrap_in_html(&msg);\n-\t\tr = imap_store_msg(ctx, &msg);\n-\t\tif (r != DRV_OK)\n-\t\t\tbreak;\n-\t\tn++;\n-\t}\n-\tfprintf(stderr, \"\\n\");\n \n-\timap_close_store(ctx);\n+\tif (server.tunnel)\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n \n-\treturn 0;\n+#ifndef USE_CURL_FOR_IMAP_SEND\n+\treturn append_msgs_to_imap(&server, &all_msgs, total);\n+#else\n+\treturn curl_append_msgs_to_imap(&server, &all_msgs, total);\n+#endif\n }\n-- \n2.1.0.372.g32d59d1.dirty\n"},{"id":"248415","messageId":"xmqqoav5etnb.fsf@gitster.dls.corp.google.com","threadId":"37420","inReplyTo":"53FD0CD1.7030801@raz.or.at","subject":"Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-08-27T17:20:40Z","receivedAt":"2014-08-27T17:20:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bernhard Reiter writes:\n\n> Use libcurl's high-level API functions to implement git-imap-send\n> instead of the previous low-level OpenSSL-based functions.\n>\n> Since version 7.30.0, libcurl's API has been able to communicate with\n> IMAP servers. Using those high-level functions instead of the current\n> ones would reduce imap-send.c by some 1200 lines of code. For now,\n> the old ones are wrapped in #ifdefs, and the new functions are enabled\n> by make if curl's version is >= 7.35.0, from which version on curl's\n> CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\n> available.\n\nhttps://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions\nsays that this was introduced as of 7.34.0, though.\n\n> As I don't have access to that many IMAP servers, I haven't been able to\n> test the new code with a wide variety of parameter combinations. I did\n> test both secure and insecure (imaps:// and imap://) connections and\n> values of \"PLAIN\" and \"LOGIN\" for the authMethod.\n\nPerhaps CC'ing those who have touched git-imap-send code over the\nyears and asking for their help testing might help?\n\n> Signed-off-by: Bernhard Reiter \n> ---\n> I rebased the patch on the pu branch, hope that was the right thing to do.\n\nUsually I would appreciate a patch for a new feature not meant for\nthe maintenance tracks to be based on 'master', so that it can go to\nthe next release without having to wait other changes that may\nconflict with it and that may not yet be ready.\n\nI will try to apply this one to 'pu', rebase it on 'master' to make\nsure the result does not depend on the other topics in flight, and\nthen merge it back to 'pu'.\n\nThanks; some comments below.\n\n> Documentation/git-imap-send.txt | 3 +-\n> INSTALL | 15 ++--\n> Makefile | 16 +++-\n> git.spec.in | 5 +-\n> imap-send.c | 165 +++++++++++++++++++++++++++++++++-------\n> 5 files changed, 167 insertions(+), 37 deletions(-)\n>\n> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\n> index 7d991d9..9d244c4 100644\n> --- a/Documentation/git-imap-send.txt\n> +++ b/Documentation/git-imap-send.txt\n> @@ -75,7 +75,8 @@ imap.preformattedHTML::\n> \n> imap.authMethod::\n> \tSpecify authenticate method for authentication with IMAP server.\n> -\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n> +\tIf you compiled git with the NO_CURL option or if your curl version is\n> +\t< 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set\n> \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n\nHmph, so there is no option that lets me say \"I know my libcurl is\nnew enough but I have some reason not to want to use the new code to\ninteract with my imap server\", at compile time or (more preferrably)\nat runtime?\n\n> diff --git a/INSTALL b/INSTALL\n> index 6ec7a24..e2770a0 100644\n> --- a/INSTALL\n> +++ b/INSTALL\n> @@ -108,18 +108,21 @@ Issues of note:\n> \t so you might need to install additional packages other than Perl\n> \t itself, e.g. Time::HiRes.\n> \n> -\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n> -\t If you don't need it, use NO_OPENSSL.\n> +\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL,\n> +\t unless you're using curl >= 7.35.0, in which case that will be\n> +\t used. If you don't need git-imap-send, you can use NO_OPENSSL.\n\nThe last sentence makes it unclear which of the following is true:\n\n - I have sufficiently new libcurl. I cannot say NO_OPENSSL because\n I do need git-imap-send.\n\n - I have sufficiently new libcurl, so \"openssl\" is not used by\n git-imap send for me. I can say NO_OPENSSL.\n\nPerhaps\n\n - git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n you are using libCurl older than 7.35.0. Otherwise you can use\n NO_OPENSSL without losing git-imap-send.\n\n> diff --git a/git.spec.in b/git.spec.in\n> index d61d537..9535cc3 100644\n> --- a/git.spec.in\n> +++ b/git.spec.in\n> @@ -8,7 +8,7 @@ License: \tGPL\n> Group: \t\tDevelopment/Tools\n> URL: \t\thttp://kernel.org/pub/software/scm/git/\n> Source: \thttp://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz\n> -BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n> +BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n\nThis is very iffy. It incompatible with the body of the patch where\nyou allow older curl library and because you depend on openssl-devel\nyou wouldn't lose imap-send.\n\n> @@ -1391,29 +1518,13 @@ int main(int argc, char **argv)\n> \t}\n> \n> \t/* write it to the imap server */\n> -\tctx = imap_open_store(&server, server.folder);\n> -\tif (!ctx) {\n> -\t\tfprintf(stderr, \"failed to open store\\n\");\n> -\t\treturn 1;\n> -\t}\n> -\n> -\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n> -\twhile (1) {\n> -\t\tunsigned percent = n * 100 / total;\n> -\n> -\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n> -\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n> -\t\t\tbreak;\n> -\t\tif (server.use_html)\n> -\t\t\twrap_in_html(&msg);\n> -\t\tr = imap_store_msg(ctx, &msg);\n> -\t\tif (r != DRV_OK)\n> -\t\t\tbreak;\n> -\t\tn++;\n> -\t}\n> -\tfprintf(stderr, \"\\n\");\n> \n> -\timap_close_store(ctx);\n> +\tif (server.tunnel)\n> +\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n> \n> -\treturn 0;\n> +#ifndef USE_CURL_FOR_IMAP_SEND\n> +\treturn append_msgs_to_imap(&server, &all_msgs, total);\n> +#else\n> +\treturn curl_append_msgs_to_imap(&server, &all_msgs, total);\n> +#endif\n> }\n\nMuch more nicely done. It appears that you could already turn these\n#ifndef/#else/#endif into a runtime conditional, allowing:\n\n - At compile-time, can be built with the four combinations\n\n (1) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=No\n (2) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=Yes\n (3) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=No\n (4) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=Yes\n\n - The first two variants can support --with-curl/--without-curl and\n choose between curl_append/append. When run --without-curl, it\n may lose some auth-methods and for variant (1) SSL is not\n supported.\n\nor am I mis-reading the patch?\n \n"},{"id":"250531","messageId":"543A9CAC.5030404@raz.or.at","threadId":"37420","inReplyTo":"xmqqoav5etnb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Bernhard Reiter","fromEmail":"ockham@raz.or.at","sentAt":"2014-10-12T15:22:20Z","receivedAt":"2014-10-12T15:22:20Z","isPatch":true,"sender":{"key":"ockham@raz.or.at","avatar":"https://avatars.githubusercontent.com/u/96308?v=4"},"body":"Sorry for not getting back to this any sooner, I've been pretty busy\nrecently with Other Projects(tm).\n\nAm 2014-08-27 um 19:20 schrieb Junio C Hamano:\n> Bernhard Reiter writes:\n> \n>> [...] For now,\n>> the old ones are wrapped in #ifdefs, and the new functions are enabled\n>> by make if curl's version is >= 7.35.0, from which version on curl's\n>> CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\n>> available.\n> \n> https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions\n> says that this was introduced as of 7.34.0, though.\n\nStrange, I thought I recalled having seen that in\nhttp://curl.haxx.se/libcurl/c/CURLOPT_LOGIN_OPTIONS.html but it clearly\nsays 7.34.0 there too. I've now changed all occurrences of 7.35.0 to\n7.34.0 (and the corresponding hex value in the Makefile).\n\n>> As I don't have access to that many IMAP servers, I haven't been able to\n>> test the new code with a wide variety of parameter combinations. I did\n>> test both secure and insecure (imaps:// and imap://) connections and\n>> values of \"PLAIN\" and \"LOGIN\" for the authMethod.\n> \n> Perhaps CC'ing those who have touched git-imap-send code over the\n> years and asking for their help testing might help?\n\nCC'ing them (going back about 2 years, which already makes the list\nquite long) and the people who have taken part in the initial discussion\non this feature in August. And the related Debian bug.\n\nPlease test this, folks!\n\n>> Signed-off-by: Bernhard Reiter \n>> ---\n>> I rebased the patch on the pu branch, hope that was the right thing to do.\n> \n> Usually I would appreciate a patch for a new feature not meant for\n> the maintenance tracks to be based on 'master', so that it can go to\n> the next release without having to wait other changes that may\n> conflict with it and that may not yet be ready.\n> \n> I will try to apply this one to 'pu', rebase it on 'master' to make\n> sure the result does not depend on the other topics in flight, and\n> then merge it back to 'pu'.\n\nOkay, I'll stick to master. I've rebased on master now that the first\ncouple related patches are there anyway.\n\n> [...]\n>>\n>> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\n>> index 7d991d9..9d244c4 100644\n>> --- a/Documentation/git-imap-send.txt\n>> +++ b/Documentation/git-imap-send.txt\n>> @@ -75,7 +75,8 @@ imap.preformattedHTML::\n>> \n>> imap.authMethod::\n>> \tSpecify authenticate method for authentication with IMAP server.\n>> -\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n>> +\tIf you compiled git with the NO_CURL option or if your curl version is\n>> +\t< 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set\n>> \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n> \n> Hmph, so there is no option that lets me say \"I know my libcurl is\n> new enough but I have some reason not to want to use the new code to\n> interact with my imap server\", at compile time or (more preferrably)\n> at runtime?\n\nAdded a runtime option, see below.\n\n>> diff --git a/INSTALL b/INSTALL\n>> index 6ec7a24..e2770a0 100644\n>> --- a/INSTALL\n>> +++ b/INSTALL\n>> @@ -108,18 +108,21 @@ Issues of note:\n>> \t so you might need to install additional packages other than Perl\n>> \t itself, e.g. Time::HiRes.\n>> \n>> -\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n>> -\t If you don't need it, use NO_OPENSSL.\n>> +\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL,\n>> +\t unless you're using curl >= 7.35.0, in which case that will be\n>> +\t used. If you don't need git-imap-send, you can use NO_OPENSSL.\n> \n> The last sentence makes it unclear which of the following is true:\n> \n> - I have sufficiently new libcurl. I cannot say NO_OPENSSL because\n> I do need git-imap-send.\n> \n> - I have sufficiently new libcurl, so \"openssl\" is not used by\n> git-imap send for me. I can say NO_OPENSSL.\n> \n> Perhaps\n> \n> - git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n> you are using libCurl older than 7.35.0. Otherwise you can use\n> NO_OPENSSL without losing git-imap-send.\n\nFixed.\n\n>> diff --git a/git.spec.in b/git.spec.in\n>> index d61d537..9535cc3 100644\n>> --- a/git.spec.in\n>> +++ b/git.spec.in\n>> @@ -8,7 +8,7 @@ License: \tGPL\n>> Group: \t\tDevelopment/Tools\n>> URL: \t\thttp://kernel.org/pub/software/scm/git/\n>> Source: \thttp://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz\n>> -BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n>> +BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n> \n> This is very iffy. It incompatible with the body of the patch where\n> you allow older curl library and because you depend on openssl-devel\n> you wouldn't lose imap-send.\n\nOkay, removed the version requirement.\n\n>> @@ -1391,29 +1518,13 @@ int main(int argc, char **argv)\n>> [...]\n> \n> Much more nicely done. It appears that you could already turn these\n> #ifndef/#else/#endif into a runtime conditional, allowing:\n> \n> - At compile-time, can be built with the four combinations\n> \n> (1) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=No\n> (2) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=Yes\n> (3) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=No\n> (4) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=Yes\n> \n> - The first two variants can support --with-curl/--without-curl and\n> choose between curl_append/append. When run --without-curl, it\n> may lose some auth-methods and for variant (1) SSL is not\n> supported.\n> \n> or am I mis-reading the patch?\n\nYou're reading it correctly; thanks for the hint about the runtime switch.\n\nI've now implemented it using parse_options, so the switch is called\n--[no-]curl. I hope parse_options isn't overkill, but as I wanted to\nkeep other options -- namely verbose and quiet switches, useful for\ndebugging curl -- I would've had to look through all argv[] items\nmanually, so I figured it made sense to us an already existing API for\nthat instead.\n\nBernhard\n\n>From 218c47580330d5ac38875c30e14dc3049d1ce3c5 Mon Sep 17 00:00:00 2001\nFrom: Bernhard Reiter \nDate: Wed, 13 Aug 2014 23:41:40 +0200\nSubject: [PATCH] git-imap-send: use libcurl for implementation\n\nUse libcurl's high-level API functions to implement git-imap-send\ninstead of the previous low-level OpenSSL-based functions.\n\nSince version 7.30.0, libcurl's API has been able to communicate with\nIMAP servers. Using those high-level functions instead of the current\nones would reduce imap-send.c by some 1200 lines of code. For now,\nthe old ones are wrapped in #ifdefs, and the new functions are enabled\nby make if curl's version is >= 7.34.0, from which version on curl's\nCURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\navailable. The low-level functions will still be used for tunneling\ninto the server for now.\n\nAs I don't have access to that many IMAP servers, I haven't been able to\ntest the new code with a wide variety of parameter combinations. I did\ntest both secure and insecure (imaps:// and imap://) connections and\nvalues of \"PLAIN\" and \"LOGIN\" for the authMethod.\n\nSigned-off-by: Bernhard Reiter \n---\n Documentation/git-imap-send.txt | 26 +++++-\n INSTALL | 15 ++--\n Makefile | 16 +++-\n imap-send.c | 189 +++++++++++++++++++++++++++++++++-------\n 4 files changed, 204 insertions(+), 42 deletions(-)\n\ndiff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\nindex 7d991d9..1c24e1f 100644\n--- a/Documentation/git-imap-send.txt\n+++ b/Documentation/git-imap-send.txt\n@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder\n SYNOPSIS\n --------\n [verse]\n-'git imap-send'\n+'git imap-send' [-v] [-q] --[no-]curl\n \n \n DESCRIPTION\n@@ -25,6 +25,26 @@ Typical usage is something like:\n \n git format-patch --signoff --stdout --attach origin | git imap-send\n \n+OPTIONS\n+-------\n+-v::\n+--verbose::\n+\tBe verbose.\n+\n+-q::\n+--quiet::\n+\tBe quiet.\n+\n+--curl::\n+\tUse libcurl to communicate with the IMAP server, unless tunneling\n+\tinto it. Only available if git was built with the\n+\tUSE_CURL_FOR_IMAP_SEND option set, in which case this is the\n+\tdefault behavior.\n+\n+--no-curl::\n+\tTalk to the IMAP server using git's own IMAP routines instead of\n+\tusing libcurl. Only available git was built with the\n+\tUSE_CURL_FOR_IMAP_SEND option set; implicitly assumed otherwise.\n \n CONFIGURATION\n -------------\n@@ -75,7 +95,9 @@ imap.preformattedHTML::\n \n imap.authMethod::\n \tSpecify authenticate method for authentication with IMAP server.\n-\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n+\tIf git was built with the NO_CURL option, or if your curl version is\n+\t< 7.34.0, or if you're running git-imap-send with the --no-curl\n+\toption, the only supported method is 'CRAM-MD5'. If this is not set\n \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n \n Examples\ndiff --git a/INSTALL b/INSTALL\nindex 6ec7a24..ffb071e 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -108,18 +108,21 @@ Issues of note:\n \t so you might need to install additional packages other than Perl\n \t itself, e.g. Time::HiRes.\n \n-\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n-\t If you don't need it, use NO_OPENSSL.\n+\t- git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n+\t you are using libcurl older than 7.34.0. Otherwise you can use\n+\t NO_OPENSSL without losing git-imap-send.\n \n \t By default, git uses OpenSSL for SHA1 but it will use its own\n \t library (inspired by Mozilla's) with either NO_OPENSSL or\n \t BLK_SHA1. Also included is a version optimized for PowerPC\n \t (PPC_SHA1).\n \n-\t- \"libcurl\" library is used by git-http-fetch and git-fetch. You\n-\t might also want the \"curl\" executable for debugging purposes.\n-\t If you do not use http:// or https:// repositories, you do not\n-\t have to have them (use NO_CURL).\n+\t- \"libcurl\" library is used by git-http-fetch, git-fetch, and, if\n+\t the curl version >= 7.34.0, for git-imap-send. You might also\n+\t want the \"curl\" executable for debugging purposes. If you do not\n+\t use http:// or https:// repositories, and do not want to put\n+\t patches into an IMAP mailbox, you do not have to have them\n+\t (use NO_CURL).\n \n \t- \"expat\" library; git-http-push uses it for remote lock\n \t management over DAV. Similar to \"curl\" above, this is optional\ndiff --git a/Makefile b/Makefile\nindex f34a2d4..69b2fbf 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -992,6 +992,9 @@ ifdef HAVE_ALLOCA_H\n \tBASIC_CFLAGS += -DHAVE_ALLOCA_H\n endif\n \n+IMAP_SEND_BUILDDEPS =\n+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\n ifdef NO_CURL\n \tBASIC_CFLAGS += -DNO_CURL\n \tREMOTE_CURL_PRIMARY =\n@@ -1026,6 +1029,15 @@ else\n \t\t\tPROGRAM_OBJS += http-push.o\n \t\tendif\n \tendif\n+\tcurl_check := $(shell (echo 072200; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)\n+\tifeq \"$(curl_check)\" \"072200\"\n+\t\tUSE_CURL_FOR_IMAP_SEND = YesPlease\n+\tendif\n+\tifdef USE_CURL_FOR_IMAP_SEND\n+\t\tBASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND\n+\t\tIMAP_SEND_BUILDDEPS = http.o\n+\t\tIMAP_SEND_LDFLAGS += $(CURL_LIBCURL)\n+\tendif\n \tifndef NO_EXPAT\n \t\tifdef EXPATDIR\n \t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n@@ -1892,9 +1904,9 @@ endif\n git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n \n-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\t\t$(LIBS) $(IMAP_SEND_LDFLAGS)\n \n git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\ndiff --git a/imap-send.c b/imap-send.c\nindex 70bcc7a..9cc80ae 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -26,11 +26,31 @@\n #include \"credential.h\"\n #include \"exec_cmd.h\"\n #include \"run-command.h\"\n+#include \"parse-options.h\"\n #ifdef NO_OPENSSL\n typedef void *SSL;\n #endif\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+#include \"http.h\"\n+#endif\n \n-static const char imap_send_usage[] = \"git imap-send < \";\n+static int Verbose, Quiet;\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static int use_curl = 1; // on by default; set later by parse_options\n+#else\n+static int use_curl = 0; // not available\n+#endif\n+\n+static const char * const imap_send_usage[] = { \"git imap-send < \", NULL };\n+\n+static struct option imap_send_options[] = {\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+\tOPT_BOOL(0, \"curl\", &use_curl, \"use libcurl to communicate with the IMAP server\"),\n+#endif\n+\tOPT__VERBOSE(&Verbose, \"be verbose\"),\n+\tOPT__QUIET(&Quiet, \"be quiet\"),\n+\tOPT_END()\n+};\n \n #undef DRV_OK\n #define DRV_OK 0\n@@ -38,8 +58,6 @@ static const char imap_send_usage[] = \"git imap-send < \";\n #define DRV_BOX_BAD -2\n #define DRV_STORE_BAD -3\n \n-static int Verbose, Quiet;\n-\n __attribute__((format (printf, 1, 2)))\n static void imap_info(const char *, ...);\n __attribute__((format (printf, 1, 2)))\n@@ -1338,22 +1356,145 @@ static void git_imap_config(void)\n \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n }\n \n-int main(int argc, char **argv)\n-{\n-\tstruct strbuf all_msgs = STRBUF_INIT;\n+static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n \tstruct strbuf msg = STRBUF_INIT;\n \tstruct imap_store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n-\tint total, n = 0;\n+\tint n = 0;\n+\n+\tctx = imap_open_store(server, server->folder);\n+\tif (!ctx) {\n+\t\tfprintf(stderr, \"failed to open store\\n\");\n+\t\treturn 1;\n+\t}\n+\tctx->name = server->folder;\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tif (!split_msg(all_msgs, &msg, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msg);\n+\t\tr = imap_store_msg(ctx, &msg);\n+\t\tif (r != DRV_OK)\n+\t\t\tbreak;\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\timap_close_store(ctx);\n+\n+\treturn 0;\n+}\n+\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static CURL *setup_curl(struct imap_server_conf *srvc)\n+{\n+\tCURL *curl;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf auth = STRBUF_INIT;\n+\n+\tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n+\t\tdie(\"curl_global_init failed\");\n+\n+\tcurl = curl_easy_init();\n+\n+\tif (!curl)\n+\t\tdie(\"curl_easy_init failed\");\n+\n+\tcurl_easy_setopt(curl, CURLOPT_USERNAME, server.user);\n+\tcurl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);\n+\n+\tstrbuf_addstr(&path, server.host);\n+\tif (!path.len || path.buf[path.len - 1] != '/')\n+\t\tstrbuf_addch(&path, '/');\n+\tstrbuf_addstr(&path, server.folder);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n+\tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n+\n+\tif (server.auth_method) {\n+\t\tstrbuf_addstr(&auth, \"AUTH=\");\n+\t\tstrbuf_addstr(&auth, server.auth_method);\n+\t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n+\t}\n+\n+\tif (server.use_ssl)\n+\t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n+\n+\tif (Verbose)\n+\t\tcurl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);\n+\n+\treturn curl;\n+}\n+\n+static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n+\tint ofs = 0;\n+\tint n = 0;\n+\tstruct buffer msgbuf = { STRBUF_INIT, 0 };\n+\tCURL *curl;\n+\tCURLcode res = CURLE_OK;\n+\n+\tcurl = setup_curl(server);\n+\tcurl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tint prev_len = msgbuf.buf.len;\n+\t\tif (!split_msg(all_msgs, &msgbuf.buf, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msgbuf.buf);\n+\t\tlf_to_crlf(&msgbuf.buf);\n+\n+\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));\n+\n+\t\tres = curl_easy_perform(curl);\n+\n+\t\tif(res != CURLE_OK) {\n+\t\t\tfprintf(stderr, \"curl_easy_perform() failed: %s\\n\",\n+\t\t\t\t\tcurl_easy_strerror(res));\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\tcurl_easy_cleanup(curl);\n+\tcurl_global_cleanup();\n+\n+\treturn 0;\n+}\n+#endif\n+\n+int main(int argc, char **argv)\n+{\n+\tstruct strbuf all_msgs = STRBUF_INIT;\n+\tint total;\n \tint nongit_ok;\n \n \tgit_extract_argv0_path(argv[0]);\n \n-\tgit_setup_gettext();\n+\targc = parse_options(argc, (const char **)argv, \"\", imap_send_options, imap_send_usage, 0);\n \n-\tif (argc != 1)\n-\t\tusage(imap_send_usage);\n+\tgit_setup_gettext();\n \n \tsetup_git_directory_gently(&nongit_ok);\n \tgit_imap_config();\n@@ -1391,29 +1532,13 @@ int main(int argc, char **argv)\n \t}\n \n \t/* write it to the imap server */\n-\tctx = imap_open_store(&server, server.folder);\n-\tif (!ctx) {\n-\t\tfprintf(stderr, \"failed to open store\\n\");\n-\t\treturn 1;\n-\t}\n \n-\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n-\twhile (1) {\n-\t\tunsigned percent = n * 100 / total;\n+\tif (server.tunnel)\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n \n-\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n-\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n-\t\t\tbreak;\n-\t\tif (server.use_html)\n-\t\t\twrap_in_html(&msg);\n-\t\tr = imap_store_msg(ctx, &msg);\n-\t\tif (r != DRV_OK)\n-\t\t\tbreak;\n-\t\tn++;\n+\tif (use_curl) {\n+\t\treturn curl_append_msgs_to_imap(&server, &all_msgs, total);\n+\t} else {\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n \t}\n-\tfprintf(stderr, \"\\n\");\n-\n-\timap_close_store(ctx);\n-\n-\treturn 0;\n }\n-- \n2.1.2.375.g218c475\n"},{"id":"250965","messageId":"5447C7A8.7030502@raz.or.at","threadId":"37420","inReplyTo":"543A9CAC.5030404@raz.or.at","subject":"Fwd: Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Bernhard Reiter","fromEmail":"ockham@raz.or.at","sentAt":"2014-10-22T15:05:12Z","receivedAt":"2014-10-22T15:05:12Z","isPatch":true,"sender":{"key":"ockham@raz.or.at","avatar":"https://avatars.githubusercontent.com/u/96308?v=4"},"body":"*ping*\nHope I didn't mess up formatting again...\nOr do I need to top-post, as the original thread is too old to keep posting to it?\n\nBernhard\n\n-------- Weitergeleitete Nachricht --------\nBetreff: Re: [PATCH] git-imap-send: use libcurl for implementation\nDatum: Sun, 12 Oct 2014 17:22:20 +0200\nVon: Bernhard Reiter \nAn: Junio C Hamano \nKopie (CC): git@vger.kernel.org, Jonathan Nieder , Jeff King , 434599@bugs.debian.org, René Scharfe , Tony Finch , Tanay Abhra , Dan Albert , Jeremy Huddleston , David Aguilar , Michael Haggerty , Oswald Buddenhagen \n\nSorry for not getting back to this any sooner, I've been pretty busy\nrecently with Other Projects(tm).\n\nAm 2014-08-27 um 19:20 schrieb Junio C Hamano:\n> Bernhard Reiter writes:\n> \n>> [...] For now,\n>> the old ones are wrapped in #ifdefs, and the new functions are enabled\n>> by make if curl's version is >= 7.35.0, from which version on curl's\n>> CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\n>> available.\n> \n> https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions\n> says that this was introduced as of 7.34.0, though.\n\nStrange, I thought I recalled having seen that in\nhttp://curl.haxx.se/libcurl/c/CURLOPT_LOGIN_OPTIONS.html but it clearly\nsays 7.34.0 there too. I've now changed all occurrences of 7.35.0 to\n7.34.0 (and the corresponding hex value in the Makefile).\n\n>> As I don't have access to that many IMAP servers, I haven't been able to\n>> test the new code with a wide variety of parameter combinations. I did\n>> test both secure and insecure (imaps:// and imap://) connections and\n>> values of \"PLAIN\" and \"LOGIN\" for the authMethod.\n> \n> Perhaps CC'ing those who have touched git-imap-send code over the\n> years and asking for their help testing might help?\n\nCC'ing them (going back about 2 years, which already makes the list\nquite long) and the people who have taken part in the initial discussion\non this feature in August. And the related Debian bug.\n\nPlease test this, folks!\n\n>> Signed-off-by: Bernhard Reiter \n>> ---\n>> I rebased the patch on the pu branch, hope that was the right thing to do.\n> \n> Usually I would appreciate a patch for a new feature not meant for\n> the maintenance tracks to be based on 'master', so that it can go to\n> the next release without having to wait other changes that may\n> conflict with it and that may not yet be ready.\n> \n> I will try to apply this one to 'pu', rebase it on 'master' to make\n> sure the result does not depend on the other topics in flight, and\n> then merge it back to 'pu'.\n\nOkay, I'll stick to master. I've rebased on master now that the first\ncouple related patches are there anyway.\n\n> [...]\n>>\n>> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\n>> index 7d991d9..9d244c4 100644\n>> --- a/Documentation/git-imap-send.txt\n>> +++ b/Documentation/git-imap-send.txt\n>> @@ -75,7 +75,8 @@ imap.preformattedHTML::\n>> \n>> imap.authMethod::\n>> \tSpecify authenticate method for authentication with IMAP server.\n>> -\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n>> +\tIf you compiled git with the NO_CURL option or if your curl version is\n>> +\t< 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set\n>> \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n> \n> Hmph, so there is no option that lets me say \"I know my libcurl is\n> new enough but I have some reason not to want to use the new code to\n> interact with my imap server\", at compile time or (more preferrably)\n> at runtime?\n\nAdded a runtime option, see below.\n\n>> diff --git a/INSTALL b/INSTALL\n>> index 6ec7a24..e2770a0 100644\n>> --- a/INSTALL\n>> +++ b/INSTALL\n>> @@ -108,18 +108,21 @@ Issues of note:\n>> \t so you might need to install additional packages other than Perl\n>> \t itself, e.g. Time::HiRes.\n>> \n>> -\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n>> -\t If you don't need it, use NO_OPENSSL.\n>> +\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL,\n>> +\t unless you're using curl >= 7.35.0, in which case that will be\n>> +\t used. If you don't need git-imap-send, you can use NO_OPENSSL.\n> \n> The last sentence makes it unclear which of the following is true:\n> \n> - I have sufficiently new libcurl. I cannot say NO_OPENSSL because\n> I do need git-imap-send.\n> \n> - I have sufficiently new libcurl, so \"openssl\" is not used by\n> git-imap send for me. I can say NO_OPENSSL.\n> \n> Perhaps\n> \n> - git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n> you are using libCurl older than 7.35.0. Otherwise you can use\n> NO_OPENSSL without losing git-imap-send.\n\nFixed.\n\n>> diff --git a/git.spec.in b/git.spec.in\n>> index d61d537..9535cc3 100644\n>> --- a/git.spec.in\n>> +++ b/git.spec.in\n>> @@ -8,7 +8,7 @@ License: \tGPL\n>> Group: \t\tDevelopment/Tools\n>> URL: \t\thttp://kernel.org/pub/software/scm/git/\n>> Source: \thttp://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz\n>> -BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n>> +BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n> \n> This is very iffy. It incompatible with the body of the patch where\n> you allow older curl library and because you depend on openssl-devel\n> you wouldn't lose imap-send.\n\nOkay, removed the version requirement.\n\n>> @@ -1391,29 +1518,13 @@ int main(int argc, char **argv)\n>> [...]\n> \n> Much more nicely done. It appears that you could already turn these\n> #ifndef/#else/#endif into a runtime conditional, allowing:\n> \n> - At compile-time, can be built with the four combinations\n> \n> (1) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=No\n> (2) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=Yes\n> (3) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=No\n> (4) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=Yes\n> \n> - The first two variants can support --with-curl/--without-curl and\n> choose between curl_append/append. When run --without-curl, it\n> may lose some auth-methods and for variant (1) SSL is not\n> supported.\n> \n> or am I mis-reading the patch?\n\nYou're reading it correctly; thanks for the hint about the runtime switch.\n\nI've now implemented it using parse_options, so the switch is called\n--[no-]curl. I hope parse_options isn't overkill, but as I wanted to\nkeep other options -- namely verbose and quiet switches, useful for\ndebugging curl -- I would've had to look through all argv[] items\nmanually, so I figured it made sense to us an already existing API for\nthat instead.\n\nBernhard\n\n>From 218c47580330d5ac38875c30e14dc3049d1ce3c5 Mon Sep 17 00:00:00 2001\nFrom: Bernhard Reiter \nDate: Wed, 13 Aug 2014 23:41:40 +0200\nSubject: [PATCH] git-imap-send: use libcurl for implementation\n\nUse libcurl's high-level API functions to implement git-imap-send\ninstead of the previous low-level OpenSSL-based functions.\n\nSince version 7.30.0, libcurl's API has been able to communicate with\nIMAP servers. Using those high-level functions instead of the current\nones would reduce imap-send.c by some 1200 lines of code. For now,\nthe old ones are wrapped in #ifdefs, and the new functions are enabled\nby make if curl's version is >= 7.34.0, from which version on curl's\nCURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\navailable. The low-level functions will still be used for tunneling\ninto the server for now.\n\nAs I don't have access to that many IMAP servers, I haven't been able to\ntest the new code with a wide variety of parameter combinations. I did\ntest both secure and insecure (imaps:// and imap://) connections and\nvalues of \"PLAIN\" and \"LOGIN\" for the authMethod.\n\nSigned-off-by: Bernhard Reiter \n---\n Documentation/git-imap-send.txt | 26 +++++-\n INSTALL | 15 ++--\n Makefile | 16 +++-\n imap-send.c | 189 +++++++++++++++++++++++++++++++++-------\n 4 files changed, 204 insertions(+), 42 deletions(-)\n\ndiff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\nindex 7d991d9..1c24e1f 100644\n--- a/Documentation/git-imap-send.txt\n+++ b/Documentation/git-imap-send.txt\n@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder\n SYNOPSIS\n --------\n [verse]\n-'git imap-send'\n+'git imap-send' [-v] [-q] --[no-]curl\n \n \n DESCRIPTION\n@@ -25,6 +25,26 @@ Typical usage is something like:\n \n git format-patch --signoff --stdout --attach origin | git imap-send\n \n+OPTIONS\n+-------\n+-v::\n+--verbose::\n+\tBe verbose.\n+\n+-q::\n+--quiet::\n+\tBe quiet.\n+\n+--curl::\n+\tUse libcurl to communicate with the IMAP server, unless tunneling\n+\tinto it. Only available if git was built with the\n+\tUSE_CURL_FOR_IMAP_SEND option set, in which case this is the\n+\tdefault behavior.\n+\n+--no-curl::\n+\tTalk to the IMAP server using git's own IMAP routines instead of\n+\tusing libcurl. Only available git was built with the\n+\tUSE_CURL_FOR_IMAP_SEND option set; implicitly assumed otherwise.\n \n CONFIGURATION\n -------------\n@@ -75,7 +95,9 @@ imap.preformattedHTML::\n \n imap.authMethod::\n \tSpecify authenticate method for authentication with IMAP server.\n-\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n+\tIf git was built with the NO_CURL option, or if your curl version is\n+\t< 7.34.0, or if you're running git-imap-send with the --no-curl\n+\toption, the only supported method is 'CRAM-MD5'. If this is not set\n \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n \n Examples\ndiff --git a/INSTALL b/INSTALL\nindex 6ec7a24..ffb071e 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -108,18 +108,21 @@ Issues of note:\n \t so you might need to install additional packages other than Perl\n \t itself, e.g. Time::HiRes.\n \n-\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n-\t If you don't need it, use NO_OPENSSL.\n+\t- git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n+\t you are using libcurl older than 7.34.0. Otherwise you can use\n+\t NO_OPENSSL without losing git-imap-send.\n \n \t By default, git uses OpenSSL for SHA1 but it will use its own\n \t library (inspired by Mozilla's) with either NO_OPENSSL or\n \t BLK_SHA1. Also included is a version optimized for PowerPC\n \t (PPC_SHA1).\n \n-\t- \"libcurl\" library is used by git-http-fetch and git-fetch. You\n-\t might also want the \"curl\" executable for debugging purposes.\n-\t If you do not use http:// or https:// repositories, you do not\n-\t have to have them (use NO_CURL).\n+\t- \"libcurl\" library is used by git-http-fetch, git-fetch, and, if\n+\t the curl version >= 7.34.0, for git-imap-send. You might also\n+\t want the \"curl\" executable for debugging purposes. If you do not\n+\t use http:// or https:// repositories, and do not want to put\n+\t patches into an IMAP mailbox, you do not have to have them\n+\t (use NO_CURL).\n \n \t- \"expat\" library; git-http-push uses it for remote lock\n \t management over DAV. Similar to \"curl\" above, this is optional\ndiff --git a/Makefile b/Makefile\nindex f34a2d4..69b2fbf 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -992,6 +992,9 @@ ifdef HAVE_ALLOCA_H\n \tBASIC_CFLAGS += -DHAVE_ALLOCA_H\n endif\n \n+IMAP_SEND_BUILDDEPS =\n+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\n ifdef NO_CURL\n \tBASIC_CFLAGS += -DNO_CURL\n \tREMOTE_CURL_PRIMARY =\n@@ -1026,6 +1029,15 @@ else\n \t\t\tPROGRAM_OBJS += http-push.o\n \t\tendif\n \tendif\n+\tcurl_check := $(shell (echo 072200; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)\n+\tifeq \"$(curl_check)\" \"072200\"\n+\t\tUSE_CURL_FOR_IMAP_SEND = YesPlease\n+\tendif\n+\tifdef USE_CURL_FOR_IMAP_SEND\n+\t\tBASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND\n+\t\tIMAP_SEND_BUILDDEPS = http.o\n+\t\tIMAP_SEND_LDFLAGS += $(CURL_LIBCURL)\n+\tendif\n \tifndef NO_EXPAT\n \t\tifdef EXPATDIR\n \t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n@@ -1892,9 +1904,9 @@ endif\n git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n \n-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\t\t$(LIBS) $(IMAP_SEND_LDFLAGS)\n \n git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\ndiff --git a/imap-send.c b/imap-send.c\nindex 70bcc7a..9cc80ae 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -26,11 +26,31 @@\n #include \"credential.h\"\n #include \"exec_cmd.h\"\n #include \"run-command.h\"\n+#include \"parse-options.h\"\n #ifdef NO_OPENSSL\n typedef void *SSL;\n #endif\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+#include \"http.h\"\n+#endif\n \n-static const char imap_send_usage[] = \"git imap-send < \";\n+static int Verbose, Quiet;\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static int use_curl = 1; // on by default; set later by parse_options\n+#else\n+static int use_curl = 0; // not available\n+#endif\n+\n+static const char * const imap_send_usage[] = { \"git imap-send < \", NULL };\n+\n+static struct option imap_send_options[] = {\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+\tOPT_BOOL(0, \"curl\", &use_curl, \"use libcurl to communicate with the IMAP server\"),\n+#endif\n+\tOPT__VERBOSE(&Verbose, \"be verbose\"),\n+\tOPT__QUIET(&Quiet, \"be quiet\"),\n+\tOPT_END()\n+};\n \n #undef DRV_OK\n #define DRV_OK 0\n@@ -38,8 +58,6 @@ static const char imap_send_usage[] = \"git imap-send < \";\n #define DRV_BOX_BAD -2\n #define DRV_STORE_BAD -3\n \n-static int Verbose, Quiet;\n-\n __attribute__((format (printf, 1, 2)))\n static void imap_info(const char *, ...);\n __attribute__((format (printf, 1, 2)))\n@@ -1338,22 +1356,145 @@ static void git_imap_config(void)\n \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n }\n \n-int main(int argc, char **argv)\n-{\n-\tstruct strbuf all_msgs = STRBUF_INIT;\n+static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n \tstruct strbuf msg = STRBUF_INIT;\n \tstruct imap_store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n-\tint total, n = 0;\n+\tint n = 0;\n+\n+\tctx = imap_open_store(server, server->folder);\n+\tif (!ctx) {\n+\t\tfprintf(stderr, \"failed to open store\\n\");\n+\t\treturn 1;\n+\t}\n+\tctx->name = server->folder;\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tif (!split_msg(all_msgs, &msg, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msg);\n+\t\tr = imap_store_msg(ctx, &msg);\n+\t\tif (r != DRV_OK)\n+\t\t\tbreak;\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\timap_close_store(ctx);\n+\n+\treturn 0;\n+}\n+\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static CURL *setup_curl(struct imap_server_conf *srvc)\n+{\n+\tCURL *curl;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf auth = STRBUF_INIT;\n+\n+\tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n+\t\tdie(\"curl_global_init failed\");\n+\n+\tcurl = curl_easy_init();\n+\n+\tif (!curl)\n+\t\tdie(\"curl_easy_init failed\");\n+\n+\tcurl_easy_setopt(curl, CURLOPT_USERNAME, server.user);\n+\tcurl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);\n+\n+\tstrbuf_addstr(&path, server.host);\n+\tif (!path.len || path.buf[path.len - 1] != '/')\n+\t\tstrbuf_addch(&path, '/');\n+\tstrbuf_addstr(&path, server.folder);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n+\tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n+\n+\tif (server.auth_method) {\n+\t\tstrbuf_addstr(&auth, \"AUTH=\");\n+\t\tstrbuf_addstr(&auth, server.auth_method);\n+\t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n+\t}\n+\n+\tif (server.use_ssl)\n+\t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n+\n+\tif (Verbose)\n+\t\tcurl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);\n+\n+\treturn curl;\n+}\n+\n+static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n+\tint ofs = 0;\n+\tint n = 0;\n+\tstruct buffer msgbuf = { STRBUF_INIT, 0 };\n+\tCURL *curl;\n+\tCURLcode res = CURLE_OK;\n+\n+\tcurl = setup_curl(server);\n+\tcurl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tint prev_len = msgbuf.buf.len;\n+\t\tif (!split_msg(all_msgs, &msgbuf.buf, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msgbuf.buf);\n+\t\tlf_to_crlf(&msgbuf.buf);\n+\n+\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));\n+\n+\t\tres = curl_easy_perform(curl);\n+\n+\t\tif(res != CURLE_OK) {\n+\t\t\tfprintf(stderr, \"curl_easy_perform() failed: %s\\n\",\n+\t\t\t\t\tcurl_easy_strerror(res));\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\tcurl_easy_cleanup(curl);\n+\tcurl_global_cleanup();\n+\n+\treturn 0;\n+}\n+#endif\n+\n+int main(int argc, char **argv)\n+{\n+\tstruct strbuf all_msgs = STRBUF_INIT;\n+\tint total;\n \tint nongit_ok;\n \n \tgit_extract_argv0_path(argv[0]);\n \n-\tgit_setup_gettext();\n+\targc = parse_options(argc, (const char **)argv, \"\", imap_send_options, imap_send_usage, 0);\n \n-\tif (argc != 1)\n-\t\tusage(imap_send_usage);\n+\tgit_setup_gettext();\n \n \tsetup_git_directory_gently(&nongit_ok);\n \tgit_imap_config();\n@@ -1391,29 +1532,13 @@ int main(int argc, char **argv)\n \t}\n \n \t/* write it to the imap server */\n-\tctx = imap_open_store(&server, server.folder);\n-\tif (!ctx) {\n-\t\tfprintf(stderr, \"failed to open store\\n\");\n-\t\treturn 1;\n-\t}\n \n-\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n-\twhile (1) {\n-\t\tunsigned percent = n * 100 / total;\n+\tif (server.tunnel)\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n \n-\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n-\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n-\t\t\tbreak;\n-\t\tif (server.use_html)\n-\t\t\twrap_in_html(&msg);\n-\t\tr = imap_store_msg(ctx, &msg);\n-\t\tif (r != DRV_OK)\n-\t\t\tbreak;\n-\t\tn++;\n+\tif (use_curl) {\n+\t\treturn curl_append_msgs_to_imap(&server, &all_msgs, total);\n+\t} else {\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n \t}\n-\tfprintf(stderr, \"\\n\");\n-\n-\timap_close_store(ctx);\n-\n-\treturn 0;\n }\n-- \n2.1.2.375.g218c475\n"},{"id":"250972","messageId":"xmqqbnp4hu8g.fsf@gitster.dls.corp.google.com","threadId":"37420","inReplyTo":"5447C7A8.7030502@raz.or.at","subject":"Re: Fwd: Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-22T17:47:27Z","receivedAt":"2014-10-22T17:47:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bernhard Reiter writes:\n\n> *ping*\n> Hope I didn't mess up formatting again...\n> Or do I need to top-post, as the original thread is too old to keep posting to it?\n\nPlease avoid top-posting on this list.\n\nIf you have some background material (e.g. summary of previous\ndiscussions, list of things that have been changed in response to\nprevious review, etc.), you have three options:\n\n * Use a cover letter, even for a one-patch topic. Use [PATCH 0/1]\n for the background material if you have tons of it and then make\n [PATCH 1/1] as a follow-up to that cover letter.\n\n See http://thread.gmane.org/gmane.comp.version-control.git/256386\n for how such an approach looks like.\n\n * Use a single message, [PATCH] (aka [PATCH 1/1]), and add your\n background material _after_ three-dash lines as comment.\n\n See http://thread.gmane.org/gmane.comp.version-control.git/258451\n for how such an approach looks like.\n\n * Use a single message, [PATCH] (aka [PATCH 1/1]), and add your\n background material _before_ a scissors line \"-- >8 --\",\n optionally followed by \"From:\" and/or \"Subject:\" (but in your\n case, you are sending your own patch as yourself with the same\n subject, so neither of them apply).\n\n See http://thread.gmane.org/gmane.comp.version-control.git/258470/focus=258474\n for how such an approach looks like.\n\n\nThanks.\n"},{"id":"251181","messageId":"545134D5.3030308@raz.or.at","threadId":"37420","inReplyTo":"xmqqoav5etnb.fsf@gitster.dls.corp.google.com","subject":"[PATCH] git-imap-send: use libcurl for implementation","fromName":"Bernhard Reiter","fromEmail":"ockham@raz.or.at","sentAt":"2014-10-29T18:41:25Z","receivedAt":"2014-10-29T18:41:25Z","isPatch":true,"sender":{"key":"ockham@raz.or.at","avatar":"https://avatars.githubusercontent.com/u/96308?v=4"},"body":"Resending this once more, as indicated by \nHope my formatting and posting style is now conformant. Sorry for the noise.\n\nAm 2014-08-27 um 19:20 schrieb Junio C Hamano:\n> Bernhard Reiter writes:\n> \n>> [...] For now,\n>> the old ones are wrapped in #ifdefs, and the new functions are enabled\n>> by make if curl's version is >= 7.35.0, from which version on curl's\n>> CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\n>> available.\n> \n> https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions\n> says that this was introduced as of 7.34.0, though.\n\nStrange, I thought I recalled having seen that in\nhttp://curl.haxx.se/libcurl/c/CURLOPT_LOGIN_OPTIONS.html but it clearly\nsays 7.34.0 there too. I've now changed all occurrences of 7.35.0 to\n7.34.0 (and the corresponding hex value in the Makefile).\n\n>> As I don't have access to that many IMAP servers, I haven't been able to\n>> test the new code with a wide variety of parameter combinations. I did\n>> test both secure and insecure (imaps:// and imap://) connections and\n>> values of \"PLAIN\" and \"LOGIN\" for the authMethod.\n> \n> Perhaps CC'ing those who have touched git-imap-send code over the\n> years and asking for their help testing might help?\n\nCC'ing them (going back about 2 years, which already makes the list\nquite long) and the people who have taken part in the initial discussion\non this feature in August. And the related Debian bug.\n\nPlease test this, folks!\n\n>> Signed-off-by: Bernhard Reiter \n>> ---\n>> I rebased the patch on the pu branch, hope that was the right thing to do.\n> \n> Usually I would appreciate a patch for a new feature not meant for\n> the maintenance tracks to be based on 'master', so that it can go to\n> the next release without having to wait other changes that may\n> conflict with it and that may not yet be ready.\n> \n> I will try to apply this one to 'pu', rebase it on 'master' to make\n> sure the result does not depend on the other topics in flight, and\n> then merge it back to 'pu'.\n\nOkay, I'll stick to master. I've rebased on master now that the first\ncouple related patches are there anyway.\n\n> [...]\n>>\n>> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\n>> index 7d991d9..9d244c4 100644\n>> --- a/Documentation/git-imap-send.txt\n>> +++ b/Documentation/git-imap-send.txt\n>> @@ -75,7 +75,8 @@ imap.preformattedHTML::\n>> \n>> imap.authMethod::\n>> \tSpecify authenticate method for authentication with IMAP server.\n>> -\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n>> +\tIf you compiled git with the NO_CURL option or if your curl version is\n>> +\t< 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set\n>> \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n> \n> Hmph, so there is no option that lets me say \"I know my libcurl is\n> new enough but I have some reason not to want to use the new code to\n> interact with my imap server\", at compile time or (more preferrably)\n> at runtime?\n\nAdded a runtime option, see below.\n\n>> diff --git a/INSTALL b/INSTALL\n>> index 6ec7a24..e2770a0 100644\n>> --- a/INSTALL\n>> +++ b/INSTALL\n>> @@ -108,18 +108,21 @@ Issues of note:\n>> \t so you might need to install additional packages other than Perl\n>> \t itself, e.g. Time::HiRes.\n>> \n>> -\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n>> -\t If you don't need it, use NO_OPENSSL.\n>> +\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL,\n>> +\t unless you're using curl >= 7.35.0, in which case that will be\n>> +\t used. If you don't need git-imap-send, you can use NO_OPENSSL.\n> \n> The last sentence makes it unclear which of the following is true:\n> \n> - I have sufficiently new libcurl. I cannot say NO_OPENSSL because\n> I do need git-imap-send.\n> \n> - I have sufficiently new libcurl, so \"openssl\" is not used by\n> git-imap send for me. I can say NO_OPENSSL.\n> \n> Perhaps\n> \n> - git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n> you are using libCurl older than 7.35.0. Otherwise you can use\n> NO_OPENSSL without losing git-imap-send.\n\nFixed.\n\n>> diff --git a/git.spec.in b/git.spec.in\n>> index d61d537..9535cc3 100644\n>> --- a/git.spec.in\n>> +++ b/git.spec.in\n>> @@ -8,7 +8,7 @@ License: \tGPL\n>> Group: \t\tDevelopment/Tools\n>> URL: \t\thttp://kernel.org/pub/software/scm/git/\n>> Source: \thttp://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz\n>> -BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n>> +BuildRequires:\tzlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc > 6.0.3}\n> \n> This is very iffy. It incompatible with the body of the patch where\n> you allow older curl library and because you depend on openssl-devel\n> you wouldn't lose imap-send.\n\nOkay, removed the version requirement.\n\n>> @@ -1391,29 +1518,13 @@ int main(int argc, char **argv)\n>> [...]\n> \n> Much more nicely done. It appears that you could already turn these\n> #ifndef/#else/#endif into a runtime conditional, allowing:\n> \n> - At compile-time, can be built with the four combinations\n> \n> (1) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=No\n> (2) USE_CURL_FOR_IMAP_SEND=Yes NO_OPENSSL=Yes\n> (3) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=No\n> (4) USE_CURL_FOR_IMAP_SEND=No NO_OPENSSL=Yes\n> \n> - The first two variants can support --with-curl/--without-curl and\n> choose between curl_append/append. When run --without-curl, it\n> may lose some auth-methods and for variant (1) SSL is not\n> supported.\n> \n> or am I mis-reading the patch?\n\nYou're reading it correctly; thanks for the hint about the runtime switch.\n\nI've now implemented it using parse_options, so the switch is called\n--[no-]curl. I hope parse_options isn't overkill, but as I wanted to\nkeep other options -- namely verbose and quiet switches, useful for\ndebugging curl -- I would've had to look through all argv[] items\nmanually, so I figured it made sense to us an already existing API for\nthat instead.\n\nBernhard\n\n-- >8 --\n\nUse libcurl's high-level API functions to implement git-imap-send\ninstead of the previous low-level OpenSSL-based functions.\n\nSince version 7.30.0, libcurl's API has been able to communicate with\nIMAP servers. Using those high-level functions instead of the current\nones would reduce imap-send.c by some 1200 lines of code. For now,\nthe old ones are wrapped in #ifdefs, and the new functions are enabled\nby make if curl's version is >= 7.34.0, from which version on curl's\nCURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\navailable. The low-level functions will still be used for tunneling\ninto the server for now.\n\nAs I don't have access to that many IMAP servers, I haven't been able to\ntest the new code with a wide variety of parameter combinations. I did\ntest both secure and insecure (imaps:// and imap://) connections and\nvalues of \"PLAIN\" and \"LOGIN\" for the authMethod.\n\nSigned-off-by: Bernhard Reiter \n---\n Documentation/git-imap-send.txt | 26 +++++-\n INSTALL | 15 ++--\n Makefile | 16 +++-\n imap-send.c | 189 +++++++++++++++++++++++++++++++++-------\n 4 files changed, 204 insertions(+), 42 deletions(-)\n\ndiff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\nindex 7d991d9..1c24e1f 100644\n--- a/Documentation/git-imap-send.txt\n+++ b/Documentation/git-imap-send.txt\n@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder\n SYNOPSIS\n --------\n [verse]\n-'git imap-send'\n+'git imap-send' [-v] [-q] --[no-]curl\n DESCRIPTION\n@@ -25,6 +25,26 @@ Typical usage is something like:\n git format-patch --signoff --stdout --attach origin | git imap-send\n +OPTIONS\n+-------\n+-v::\n+--verbose::\n+\tBe verbose.\n+\n+-q::\n+--quiet::\n+\tBe quiet.\n+\n+--curl::\n+\tUse libcurl to communicate with the IMAP server, unless tunneling\n+\tinto it. Only available if git was built with the\n+\tUSE_CURL_FOR_IMAP_SEND option set, in which case this is the\n+\tdefault behavior.\n+\n+--no-curl::\n+\tTalk to the IMAP server using git's own IMAP routines instead of\n+\tusing libcurl. Only available git was built with the\n+\tUSE_CURL_FOR_IMAP_SEND option set; implicitly assumed otherwise.\n CONFIGURATION\n -------------\n@@ -75,7 +95,9 @@ imap.preformattedHTML::\n imap.authMethod::\n \tSpecify authenticate method for authentication with IMAP server.\n-\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n+\tIf git was built with the NO_CURL option, or if your curl version is\n+\t< 7.34.0, or if you're running git-imap-send with the --no-curl\n+\toption, the only supported method is 'CRAM-MD5'. If this is not set\n \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n Examples\ndiff --git a/INSTALL b/INSTALL\nindex 6ec7a24..ffb071e 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -108,18 +108,21 @@ Issues of note:\n \t so you might need to install additional packages other than Perl\n \t itself, e.g. Time::HiRes.\n -\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n-\t If you don't need it, use NO_OPENSSL.\n+\t- git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n+\t you are using libcurl older than 7.34.0. Otherwise you can use\n+\t NO_OPENSSL without losing git-imap-send.\n \t By default, git uses OpenSSL for SHA1 but it will use its own\n \t library (inspired by Mozilla's) with either NO_OPENSSL or\n \t BLK_SHA1. Also included is a version optimized for PowerPC\n \t (PPC_SHA1).\n -\t- \"libcurl\" library is used by git-http-fetch and git-fetch. You\n-\t might also want the \"curl\" executable for debugging purposes.\n-\t If you do not use http:// or https:// repositories, you do not\n-\t have to have them (use NO_CURL).\n+\t- \"libcurl\" library is used by git-http-fetch, git-fetch, and, if\n+\t the curl version >= 7.34.0, for git-imap-send. You might also\n+\t want the \"curl\" executable for debugging purposes. If you do not\n+\t use http:// or https:// repositories, and do not want to put\n+\t patches into an IMAP mailbox, you do not have to have them\n+\t (use NO_CURL).\n \t- \"expat\" library; git-http-push uses it for remote lock\n \t management over DAV. Similar to \"curl\" above, this is optional\ndiff --git a/Makefile b/Makefile\nindex f34a2d4..69b2fbf 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -992,6 +992,9 @@ ifdef HAVE_ALLOCA_H\n \tBASIC_CFLAGS += -DHAVE_ALLOCA_H\n endif\n +IMAP_SEND_BUILDDEPS =\n+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\n ifdef NO_CURL\n \tBASIC_CFLAGS += -DNO_CURL\n \tREMOTE_CURL_PRIMARY =\n@@ -1026,6 +1029,15 @@ else\n \t\t\tPROGRAM_OBJS += http-push.o\n \t\tendif\n \tendif\n+\tcurl_check := $(shell (echo 072200; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)\n+\tifeq \"$(curl_check)\" \"072200\"\n+\t\tUSE_CURL_FOR_IMAP_SEND = YesPlease\n+\tendif\n+\tifdef USE_CURL_FOR_IMAP_SEND\n+\t\tBASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND\n+\t\tIMAP_SEND_BUILDDEPS = http.o\n+\t\tIMAP_SEND_LDFLAGS += $(CURL_LIBCURL)\n+\tendif\n \tifndef NO_EXPAT\n \t\tifdef EXPATDIR\n \t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n@@ -1892,9 +1904,9 @@ endif\n git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n -git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\t\t$(LIBS) $(IMAP_SEND_LDFLAGS)\n git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\ndiff --git a/imap-send.c b/imap-send.c\nindex 70bcc7a..9cc80ae 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -26,11 +26,31 @@\n #include \"credential.h\"\n #include \"exec_cmd.h\"\n #include \"run-command.h\"\n+#include \"parse-options.h\"\n #ifdef NO_OPENSSL\n typedef void *SSL;\n #endif\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+#include \"http.h\"\n+#endif\n -static const char imap_send_usage[] = \"git imap-send < \";\n+static int Verbose, Quiet;\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static int use_curl = 1; // on by default; set later by parse_options\n+#else\n+static int use_curl = 0; // not available\n+#endif\n+\n+static const char * const imap_send_usage[] = { \"git imap-send < \", NULL };\n+\n+static struct option imap_send_options[] = {\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+\tOPT_BOOL(0, \"curl\", &use_curl, \"use libcurl to communicate with the IMAP server\"),\n+#endif\n+\tOPT__VERBOSE(&Verbose, \"be verbose\"),\n+\tOPT__QUIET(&Quiet, \"be quiet\"),\n+\tOPT_END()\n+};\n #undef DRV_OK\n #define DRV_OK 0\n@@ -38,8 +58,6 @@ static const char imap_send_usage[] = \"git imap-send < \";\n #define DRV_BOX_BAD -2\n #define DRV_STORE_BAD -3\n -static int Verbose, Quiet;\n-\n __attribute__((format (printf, 1, 2)))\n static void imap_info(const char *, ...);\n __attribute__((format (printf, 1, 2)))\n@@ -1338,22 +1356,145 @@ static void git_imap_config(void)\n \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n }\n -int main(int argc, char **argv)\n-{\n-\tstruct strbuf all_msgs = STRBUF_INIT;\n+static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n \tstruct strbuf msg = STRBUF_INIT;\n \tstruct imap_store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n-\tint total, n = 0;\n+\tint n = 0;\n+\n+\tctx = imap_open_store(server, server->folder);\n+\tif (!ctx) {\n+\t\tfprintf(stderr, \"failed to open store\\n\");\n+\t\treturn 1;\n+\t}\n+\tctx->name = server->folder;\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tif (!split_msg(all_msgs, &msg, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msg);\n+\t\tr = imap_store_msg(ctx, &msg);\n+\t\tif (r != DRV_OK)\n+\t\t\tbreak;\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\timap_close_store(ctx);\n+\n+\treturn 0;\n+}\n+\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static CURL *setup_curl(struct imap_server_conf *srvc)\n+{\n+\tCURL *curl;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf auth = STRBUF_INIT;\n+\n+\tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n+\t\tdie(\"curl_global_init failed\");\n+\n+\tcurl = curl_easy_init();\n+\n+\tif (!curl)\n+\t\tdie(\"curl_easy_init failed\");\n+\n+\tcurl_easy_setopt(curl, CURLOPT_USERNAME, server.user);\n+\tcurl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);\n+\n+\tstrbuf_addstr(&path, server.host);\n+\tif (!path.len || path.buf[path.len - 1] != '/')\n+\t\tstrbuf_addch(&path, '/');\n+\tstrbuf_addstr(&path, server.folder);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n+\tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n+\n+\tif (server.auth_method) {\n+\t\tstrbuf_addstr(&auth, \"AUTH=\");\n+\t\tstrbuf_addstr(&auth, server.auth_method);\n+\t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n+\t}\n+\n+\tif (server.use_ssl)\n+\t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n+\n+\tif (Verbose)\n+\t\tcurl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);\n+\n+\treturn curl;\n+}\n+\n+static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n+\tint ofs = 0;\n+\tint n = 0;\n+\tstruct buffer msgbuf = { STRBUF_INIT, 0 };\n+\tCURL *curl;\n+\tCURLcode res = CURLE_OK;\n+\n+\tcurl = setup_curl(server);\n+\tcurl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tint prev_len = msgbuf.buf.len;\n+\t\tif (!split_msg(all_msgs, &msgbuf.buf, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msgbuf.buf);\n+\t\tlf_to_crlf(&msgbuf.buf);\n+\n+\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));\n+\n+\t\tres = curl_easy_perform(curl);\n+\n+\t\tif(res != CURLE_OK) {\n+\t\t\tfprintf(stderr, \"curl_easy_perform() failed: %s\\n\",\n+\t\t\t\t\tcurl_easy_strerror(res));\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\tcurl_easy_cleanup(curl);\n+\tcurl_global_cleanup();\n+\n+\treturn 0;\n+}\n+#endif\n+\n+int main(int argc, char **argv)\n+{\n+\tstruct strbuf all_msgs = STRBUF_INIT;\n+\tint total;\n \tint nongit_ok;\n \tgit_extract_argv0_path(argv[0]);\n -\tgit_setup_gettext();\n+\targc = parse_options(argc, (const char **)argv, \"\", imap_send_options, imap_send_usage, 0);\n -\tif (argc != 1)\n-\t\tusage(imap_send_usage);\n+\tgit_setup_gettext();\n \tsetup_git_directory_gently(&nongit_ok);\n \tgit_imap_config();\n@@ -1391,29 +1532,13 @@ int main(int argc, char **argv)\n \t}\n \t/* write it to the imap server */\n-\tctx = imap_open_store(&server, server.folder);\n-\tif (!ctx) {\n-\t\tfprintf(stderr, \"failed to open store\\n\");\n-\t\treturn 1;\n-\t}\n -\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n-\twhile (1) {\n-\t\tunsigned percent = n * 100 / total;\n+\tif (server.tunnel)\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n -\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n-\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n-\t\t\tbreak;\n-\t\tif (server.use_html)\n-\t\t\twrap_in_html(&msg);\n-\t\tr = imap_store_msg(ctx, &msg);\n-\t\tif (r != DRV_OK)\n-\t\t\tbreak;\n-\t\tn++;\n+\tif (use_curl) {\n+\t\treturn curl_append_msgs_to_imap(&server, &all_msgs, total);\n+\t} else {\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n \t}\n-\tfprintf(stderr, \"\\n\");\n-\n-\timap_close_store(ctx);\n-\n-\treturn 0;\n }\n-- \n2.1.2.375.g218c475\n"},{"id":"251199","messageId":"xmqqzjcewq6p.fsf@gitster.dls.corp.google.com","threadId":"37420","inReplyTo":"545134D5.3030308@raz.or.at","subject":"Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-29T20:56:14Z","receivedAt":"2014-10-29T20:56:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bernhard Reiter writes:\n\n> Resending this once more, as indicated by \n> Hope my formatting and posting style is now conformant. Sorry for the noise.\n\nThanks.\n\nThe patch does not apply for me (please send a trial message to\nyourself and then try to apply it out of your mailbox to avoid such\nan incident in the future), unfortunately. I'll comment on the\npatch from just code inspection without applying it, though.\n\n> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\n> index 7d991d9..1c24e1f 100644\n> --- a/Documentation/git-imap-send.txt\n> +++ b/Documentation/git-imap-send.txt\n> @@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder\n> SYNOPSIS\n> --------\n> [verse]\n> -'git imap-send'\n> +'git imap-send' [-v] [-q] --[no-]curl\n> DESCRIPTION\n> @@ ...\n\nThe hunk header says that the original and the updated both starts\nat line #9 and they both should have 7 lines, but I count only 5\nlines. Where did you lose two lines of the post-context?\n\n> diff --git a/Makefile b/Makefile\n> index f34a2d4..69b2fbf 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -992,6 +992,9 @@ ifdef HAVE_ALLOCA_H\n> \tBASIC_CFLAGS += -DHAVE_ALLOCA_H\n> endif\n> +IMAP_SEND_BUILDDEPS =\n> +IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n> +\n\nHere the patch looks funny as well. I guess \"IMAP_SEND_BUILDDEPS =\"\nis an added line prefixed with \"+\", but where does the SP before the\nplus sign come from?\n\nI won't point out other patch breakages in the remainder of this\nmessage.\n\n> diff --git a/imap-send.c b/imap-send.c\n> index 70bcc7a..9cc80ae 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -26,11 +26,31 @@\n> #include \"credential.h\"\n> #include \"exec_cmd.h\"\n> #include \"run-command.h\"\n> +#include \"parse-options.h\"\n> #ifdef NO_OPENSSL\n> typedef void *SSL;\n> #endif\n> +#ifdef USE_CURL_FOR_IMAP_SEND\n> +#include \"http.h\"\n> +#endif\n> -static const char imap_send_usage[] = \"git imap-send < \";\n> +static int Verbose, Quiet;\n\nYikes. I know this is not a new problem, but please do not name\nvariables Capitalized. Also what would it mean to set both Verbose\nand Quiet at the same time?\n\nI think we would want to use OPT__VERBOSITY with a single variable\n\"verbosity\" (see hits from \"git grep OPT__VERBOSITY\" for examples).\nSuch a change should probably come before this change to use libcurl\nas a separate preparatory clean-up patch.\n\n> +#ifdef USE_CURL_FOR_IMAP_SEND\n> +static int use_curl = 1; // on by default; set later by parse_options\n> +#else\n> +static int use_curl = 0; // not available\n> +#endif\n\nPlease don't use // comments. Besides, I think this should be\ninitialized to -1 to mean \"unspecified by the user\" in both cases\n(i.e. no need for #ifdef/#endif).\n\n> +static struct option imap_send_options[] = {\n> +#ifdef USE_CURL_FOR_IMAP_SEND\n> +\tOPT_BOOL(0, \"curl\", &use_curl, \"use libcurl to communicate with the IMAP server\"),\n> +#endif\n\nAnd lose the ifdef here (i.e. take \"--use-curl\" even on a build that\ndoes not support it).\n\n> +int main(int argc, char **argv)\n> +{\n> +\tstruct strbuf all_msgs = STRBUF_INIT;\n> +\tint total;\n> \tint nongit_ok;\n> \tgit_extract_argv0_path(argv[0]);\n> -\tgit_setup_gettext();\n> +\targc = parse_options(argc, (const char **)argv, \"\", imap_send_options, imap_send_usage, 0);\n> -\tif (argc != 1)\n> -\t\tusage(imap_send_usage);\n> +\tgit_setup_gettext();\n> \tsetup_git_directory_gently(&nongit_ok);\n> \tgit_imap_config();\n\nUsually we read config and then parse options, so that people can\nset configuration variables for their usual use pattern and override\nwhat is configured from the command line option as needed.\n\nFor example, you may want to add\n\n\t[imap]\n \tuseCurl = true\n\nin the configuration file and run \"git imap-send\" without \"--curl\"\noption on the command line almost always, but in some specific\noccasion, \"git imap-send --no-curl\" to countermand the\nconfiguration.\n\nWas there a specific reason why you had to add parse_options()\nbefore git_imap_config()?\n\nWith \"use_curl\" initialized to \"-1 (unspecified)\", the final code\nstructure may look more like this:\n\n\t\t...\n git_imap_config();\n parse_options();\n\n #ifndef USE_CURL_FOR_IMAP_SEND\n if (use_curl < 0)\n use_curl = 0;\n if (use_curl) {\n warning(\"--use-curl not supported in this build\");\n use_curl = 0;\n }\n #else\n if (use_curl < 0) /* default to enable libcurl */\n use_curl = 1;\n #endif \n\nalthough I think it is also OK to make this feature strictly optional\nby defaulting to \"use_curl = 0\" in the #else part above. Initializing\nthe static variable to 0 would make the result even shorter if we\nwere to go that route, i.e.\n\n\tstatic int use_curl; /* strictly opt in */\n ...\n\tmain()\n {\n\t\t...\n\t\tgit_imap_config();\n\t parse_options();\n\t#ifndef USE_CURL_FOR_IMAP_SEND\n \tif (use_curl) {\n\t\t\twarning(...);\n\t\t\tuse_curl = 0;\n\t\t}\n\t#endif\n"},{"id":"251607","messageId":"545F6586.109@web.de","threadId":"37420","inReplyTo":"53FD0CD1.7030801@raz.or.at","subject":"Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-11-09T13:00:54Z","receivedAt":"2014-11-09T13:00:54Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-08-27 00.40, Bernhard Reiter wrote:\n> Use libcurl's high-level API functions to implement git-imap-send\n> instead of the previous low-level OpenSSL-based functions.\n> \nThis doesn't seem to fully work under Debian 7:\n/home/tb/projects/git/git.pu/imap-send.c:1546: undefined reference to `curl_append_msgs_to_imap'\n\nSome more info:\n curl-config --vernum\n071a00\n\n (echo 072200; curl-config --vernum) 2>/dev/null | sort -r \n072200\n071a00\n\n(echo 072200; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p\n071a00\n"},{"id":"251614","messageId":"545F8079.7010806@raz.or.at","threadId":"37420","inReplyTo":"545F6586.109@web.de","subject":"Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Bernhard Reiter","fromEmail":"ockham@raz.or.at","sentAt":"2014-11-09T14:55:53Z","receivedAt":"2014-11-09T14:55:53Z","isPatch":true,"sender":{"key":"ockham@raz.or.at","avatar":"https://avatars.githubusercontent.com/u/96308?v=4"},"body":"Am 2014-11-09 um 14:00 schrieb Torsten Bögershausen:\n> On 2014-08-27 00.40, Bernhard Reiter wrote:\n>> Use libcurl's high-level API functions to implement git-imap-send\n>> instead of the previous low-level OpenSSL-based functions.\n>>\n> This doesn't seem to fully work under Debian 7:\n> /home/tb/projects/git/git.pu/imap-send.c:1546: undefined reference to `curl_append_msgs_to_imap'\n\nThx for the notice. I forgot to guard that with an #ifdef.\n\nThe new patch below includes that, and the fix sent by Ramsay; hopefully the squashed/edited commit message is fine.\n\nBernhard\n\n-- >8 --\n\nUse libcurl's high-level API functions to implement git-imap-send\ninstead of the previous low-level OpenSSL-based functions.\n\nSince version 7.30.0, libcurl's API has been able to communicate with\nIMAP servers. Using those high-level functions instead of the current\nones would reduce imap-send.c by some 1200 lines of code. For now,\nthe old ones are wrapped in #ifdefs, and the new functions are enabled\nby make if curl's version is >= 7.34.0, from which version on curl's\nCURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\navailable. The low-level functions will still be used for tunneling\ninto the server for now.\n\nAs I don't have access to that many IMAP servers, I haven't been able to\ntest the new code with a wide variety of parameter combinations. I did\ntest both secure and insecure (imaps:// and imap://) connections and\nvalues of \"PLAIN\" and \"LOGIN\" for the authMethod.\n\nIn order to suppress a sparse warning about \"using sizeof on a\nfunction\", we use the same solution used in commit 9371322a6\n(\"sparse: suppress some \"using sizeof on a function\" warnings\",\n06-10-2013) which solved exactly this problem for the other commands\nusing libcurl.\n\nSigned-off-by: Bernhard Reiter \nSigned-off-by: Ramsay Jones \n---\n Documentation/git-imap-send.txt | 15 +++-\n INSTALL | 15 ++--\n Makefile | 18 +++-\n imap-send.c | 176 ++++++++++++++++++++++++++++++++++------\n 4 files changed, 187 insertions(+), 37 deletions(-)\n\ndiff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\nindex 0897131..77aacf1 100644\n--- a/Documentation/git-imap-send.txt\n+++ b/Documentation/git-imap-send.txt\n@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder\n SYNOPSIS\n --------\n [verse]\n-'git imap-send' [-v] [-q]\n+'git imap-send' [-v] [-q] [--[no-]curl]\n \n \n DESCRIPTION\n@@ -37,6 +37,15 @@ OPTIONS\n --quiet::\n \tBe quiet.\n \n+--curl::\n+\tUse libcurl to communicate with the IMAP server, unless tunneling\n+\tinto it. Ignored if Git was built without the USE_CURL_FOR_IMAP_SEND\n+\toption set.\n+\n+--no-curl::\n+\tTalk to the IMAP server using git's own IMAP routines instead of\n+\tusing libcurl.\n+\n \n CONFIGURATION\n -------------\n@@ -87,7 +96,9 @@ imap.preformattedHTML::\n \n imap.authMethod::\n \tSpecify authenticate method for authentication with IMAP server.\n-\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n+\tIf Git was built with the NO_CURL option, or if your curl version is older\n+\tthan 7.34.0, or if you're running git-imap-send with the `--no-curl`\n+\toption, the only supported method is 'CRAM-MD5'. If this is not set\n \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n \n Examples\ndiff --git a/INSTALL b/INSTALL\nindex 6ec7a24..ffb071e 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -108,18 +108,21 @@ Issues of note:\n \t so you might need to install additional packages other than Perl\n \t itself, e.g. Time::HiRes.\n \n-\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n-\t If you don't need it, use NO_OPENSSL.\n+\t- git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n+\t you are using libcurl older than 7.34.0. Otherwise you can use\n+\t NO_OPENSSL without losing git-imap-send.\n \n \t By default, git uses OpenSSL for SHA1 but it will use its own\n \t library (inspired by Mozilla's) with either NO_OPENSSL or\n \t BLK_SHA1. Also included is a version optimized for PowerPC\n \t (PPC_SHA1).\n \n-\t- \"libcurl\" library is used by git-http-fetch and git-fetch. You\n-\t might also want the \"curl\" executable for debugging purposes.\n-\t If you do not use http:// or https:// repositories, you do not\n-\t have to have them (use NO_CURL).\n+\t- \"libcurl\" library is used by git-http-fetch, git-fetch, and, if\n+\t the curl version >= 7.34.0, for git-imap-send. You might also\n+\t want the \"curl\" executable for debugging purposes. If you do not\n+\t use http:// or https:// repositories, and do not want to put\n+\t patches into an IMAP mailbox, you do not have to have them\n+\t (use NO_CURL).\n \n \t- \"expat\" library; git-http-push uses it for remote lock\n \t management over DAV. Similar to \"curl\" above, this is optional\ndiff --git a/Makefile b/Makefile\nindex 827006b..14d5ac1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -995,6 +995,9 @@ ifdef HAVE_ALLOCA_H\n \tBASIC_CFLAGS += -DHAVE_ALLOCA_H\n endif\n \n+IMAP_SEND_BUILDDEPS =\n+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\n ifdef NO_CURL\n \tBASIC_CFLAGS += -DNO_CURL\n \tREMOTE_CURL_PRIMARY =\n@@ -1029,6 +1032,15 @@ else\n \t\t\tPROGRAM_OBJS += http-push.o\n \t\tendif\n \tendif\n+\tcurl_check := $(shell (echo 072200; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)\n+\tifeq \"$(curl_check)\" \"072200\"\n+\t\tUSE_CURL_FOR_IMAP_SEND = YesPlease\n+\tendif\n+\tifdef USE_CURL_FOR_IMAP_SEND\n+\t\tBASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND\n+\t\tIMAP_SEND_BUILDDEPS = http.o\n+\t\tIMAP_SEND_LDFLAGS += $(CURL_LIBCURL)\n+\tendif\n \tifndef NO_EXPAT\n \t\tifdef EXPATDIR\n \t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n@@ -1874,7 +1886,7 @@ gettext.sp gettext.s gettext.o: GIT-PREFIX\n gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n \t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n \n-http-push.sp http.sp http-walker.sp remote-curl.sp: SPARSE_FLAGS += \\\n+http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS += \\\n \t-DCURL_DISABLE_TYPECHECK\n \n ifdef NO_EXPAT\n@@ -1895,9 +1907,9 @@ endif\n git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n \n-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\t\t$(LIBS) $(IMAP_SEND_LDFLAGS)\n \n git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\ndiff --git a/imap-send.c b/imap-send.c\nindex 7f9d30e..08271d9 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -30,13 +30,18 @@\n #ifdef NO_OPENSSL\n typedef void *SSL;\n #endif\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+#include \"http.h\"\n+#endif\n \n static int verbosity;\n+static int use_curl; /* strictly opt in */\n \n-static const char * const imap_send_usage[] = { \"git imap-send [-v] [-q] < \", NULL };\n+static const char * const imap_send_usage[] = { \"git imap-send [-v] [-q] [--[no-]curl] < \", NULL };\n \n static struct option imap_send_options[] = {\n \tOPT__VERBOSITY(&verbosity),\n+\tOPT_BOOL(0, \"curl\", &use_curl, \"use libcurl to communicate with the IMAP server\"),\n \tOPT_END()\n };\n \n@@ -1344,14 +1349,141 @@ static void git_imap_config(void)\n \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n }\n \n-int main(int argc, char **argv)\n+static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total)\n {\n-\tstruct strbuf all_msgs = STRBUF_INIT;\n \tstruct strbuf msg = STRBUF_INIT;\n \tstruct imap_store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n-\tint total, n = 0;\n+\tint n = 0;\n+\n+\tctx = imap_open_store(server, server->folder);\n+\tif (!ctx) {\n+\t\tfprintf(stderr, \"failed to open store\\n\");\n+\t\treturn 1;\n+\t}\n+\tctx->name = server->folder;\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tif (!split_msg(all_msgs, &msg, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msg);\n+\t\tr = imap_store_msg(ctx, &msg);\n+\t\tif (r != DRV_OK)\n+\t\t\tbreak;\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\timap_close_store(ctx);\n+\n+\treturn 0;\n+}\n+\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static CURL *setup_curl(struct imap_server_conf *srvc)\n+{\n+\tCURL *curl;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf auth = STRBUF_INIT;\n+\n+\tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n+\t\tdie(\"curl_global_init failed\");\n+\n+\tcurl = curl_easy_init();\n+\n+\tif (!curl)\n+\t\tdie(\"curl_easy_init failed\");\n+\n+\tcurl_easy_setopt(curl, CURLOPT_USERNAME, server.user);\n+\tcurl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);\n+\n+\tstrbuf_addstr(&path, server.host);\n+\tif (!path.len || path.buf[path.len - 1] != '/')\n+\t\tstrbuf_addch(&path, '/');\n+\tstrbuf_addstr(&path, server.folder);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n+\tstrbuf_release(&path);\n+\tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n+\n+\tif (server.auth_method) {\n+\t\tstrbuf_addstr(&auth, \"AUTH=\");\n+\t\tstrbuf_addstr(&auth, server.auth_method);\n+\t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n+\t}\n+\tstrbuf_release(&auth);\n+\n+\tif (server.use_ssl)\n+\t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n+\n+\tif (0 < verbosity)\n+\t\tcurl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);\n+\n+\treturn curl;\n+}\n+\n+static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n+\tint ofs = 0;\n+\tint n = 0;\n+\tstruct buffer msgbuf = { STRBUF_INIT, 0 };\n+\tCURL *curl;\n+\tCURLcode res = CURLE_OK;\n+\n+\tcurl = setup_curl(server);\n+\tcurl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tint prev_len = msgbuf.buf.len;\n+\t\tif (!split_msg(all_msgs, &msgbuf.buf, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msgbuf.buf);\n+\t\tlf_to_crlf(&msgbuf.buf);\n+\n+\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));\n+\n+\t\tres = curl_easy_perform(curl);\n+\n+\t\tif(res != CURLE_OK) {\n+\t\t\tfprintf(stderr, \"curl_easy_perform() failed: %s\\n\",\n+\t\t\t\t\tcurl_easy_strerror(res));\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\tcurl_easy_cleanup(curl);\n+\tcurl_global_cleanup();\n+\n+\treturn 0;\n+}\n+#endif\n+\n+int main(int argc, char **argv)\n+{\n+\tstruct strbuf all_msgs = STRBUF_INIT;\n+\tint total;\n \tint nongit_ok;\n \n \tgit_extract_argv0_path(argv[0]);\n@@ -1366,6 +1498,13 @@ int main(int argc, char **argv)\n \tif (argc)\n \t\tusage_with_options(imap_send_usage, imap_send_options);\n \n+#ifndef USE_CURL_FOR_IMAP_SEND\n+\tif (use_curl) {\n+\t\twarning(\"--use-curl not supported in this build\");\n+\t\tuse_curl = 0;\n+\t}\n+#endif\n+\n \tif (!server.port)\n \t\tserver.port = server.use_ssl ? 993 : 143;\n \n@@ -1399,29 +1538,14 @@ int main(int argc, char **argv)\n \t}\n \n \t/* write it to the imap server */\n-\tctx = imap_open_store(&server, server.folder);\n-\tif (!ctx) {\n-\t\tfprintf(stderr, \"failed to open store\\n\");\n-\t\treturn 1;\n-\t}\n-\n-\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n-\twhile (1) {\n-\t\tunsigned percent = n * 100 / total;\n \n-\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n-\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n-\t\t\tbreak;\n-\t\tif (server.use_html)\n-\t\t\twrap_in_html(&msg);\n-\t\tr = imap_store_msg(ctx, &msg);\n-\t\tif (r != DRV_OK)\n-\t\t\tbreak;\n-\t\tn++;\n-\t}\n-\tfprintf(stderr, \"\\n\");\n+\tif (server.tunnel)\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n \n-\timap_close_store(ctx);\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+\tif (use_curl)\n+\t\treturn curl_append_msgs_to_imap(&server, &all_msgs, total);\n+#endif\n \n-\treturn 0;\n+\treturn append_msgs_to_imap(&server, &all_msgs, total);\n }\n-- \n2.2.0.rc0.4.g1aeb70f.dirty\n"},{"id":"251623","messageId":"545FA3FA.10308@ramsay1.demon.co.uk","threadId":"37420","inReplyTo":"545F8079.7010806@raz.or.at","subject":"Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2014-11-09T17:27:22Z","receivedAt":"2014-11-09T17:27:22Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"On 09/11/14 14:55, Bernhard Reiter wrote:\n> Am 2014-11-09 um 14:00 schrieb Torsten Bögershausen:\n>> On 2014-08-27 00.40, Bernhard Reiter wrote:\n>>> Use libcurl's high-level API functions to implement git-imap-send\n>>> instead of the previous low-level OpenSSL-based functions.\n>>>\n>> This doesn't seem to fully work under Debian 7:\n>> /home/tb/projects/git/git.pu/imap-send.c:1546: undefined reference to `curl_append_msgs_to_imap'\n> \n> Thx for the notice. I forgot to guard that with an #ifdef.\n> \n> The new patch below includes that, and the fix sent by Ramsay; hopefully the squashed/edited commit message is fine.\n\nSorry for not responding earlier, I've been somewhat busy for the last\ncouple of days. :(\n\n> Bernhard\n> \n> -- >8 --\n> \n> Use libcurl's high-level API functions to implement git-imap-send\n> instead of the previous low-level OpenSSL-based functions.\n> \n> Since version 7.30.0, libcurl's API has been able to communicate with\n> IMAP servers. Using those high-level functions instead of the current\n> ones would reduce imap-send.c by some 1200 lines of code. For now,\n> the old ones are wrapped in #ifdefs, and the new functions are enabled\n> by make if curl's version is >= 7.34.0, from which version on curl's\n> CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\n> available. The low-level functions will still be used for tunneling\n> into the server for now.\n> \n> As I don't have access to that many IMAP servers, I haven't been able to\n> test the new code with a wide variety of parameter combinations. I did\n> test both secure and insecure (imaps:// and imap://) connections and\n> values of \"PLAIN\" and \"LOGIN\" for the authMethod.\n> \n> In order to suppress a sparse warning about \"using sizeof on a\n> function\", we use the same solution used in commit 9371322a6\n> (\"sparse: suppress some \"using sizeof on a function\" warnings\",\n> 06-10-2013) which solved exactly this problem for the other commands\n> using libcurl.\n\nAlthough it doesn't hurt, I don't think this 'problem' deserves\nso many (or any) inches in the commit message. ;-)\n\n> \n> Signed-off-by: Bernhard Reiter \n> Signed-off-by: Ramsay Jones \n\nSince I didn't actually review this patch, or make any significant\ncontribution to the code (it's not even enough to be copyright-able!),\nthen this 'Signed-off-by:' should not be included. At *most* you might\nwant to put a 'Helped-by:' _prior_ to your sign-off.\n\nThanks.\n\nATB,\nRamsay Jones\n\n\n> ---\n> Documentation/git-imap-send.txt | 15 +++-\n> INSTALL | 15 ++--\n> Makefile | 18 +++-\n> imap-send.c | 176 ++++++++++++++++++++++++++++++++++------\n> 4 files changed, 187 insertions(+), 37 deletions(-)\n> \n> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\n> index 0897131..77aacf1 100644\n> --- a/Documentation/git-imap-send.txt\n> +++ b/Documentation/git-imap-send.txt\n> @@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder\n> SYNOPSIS\n> --------\n> [verse]\n> -'git imap-send' [-v] [-q]\n> +'git imap-send' [-v] [-q] [--[no-]curl]\n> \n> \n> DESCRIPTION\n> @@ -37,6 +37,15 @@ OPTIONS\n> --quiet::\n> \tBe quiet.\n> \n> +--curl::\n> +\tUse libcurl to communicate with the IMAP server, unless tunneling\n> +\tinto it. Ignored if Git was built without the USE_CURL_FOR_IMAP_SEND\n> +\toption set.\n> +\n> +--no-curl::\n> +\tTalk to the IMAP server using git's own IMAP routines instead of\n> +\tusing libcurl.\n> +\n> \n> CONFIGURATION\n> -------------\n> @@ -87,7 +96,9 @@ imap.preformattedHTML::\n> \n> imap.authMethod::\n> \tSpecify authenticate method for authentication with IMAP server.\n> -\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n> +\tIf Git was built with the NO_CURL option, or if your curl version is older\n> +\tthan 7.34.0, or if you're running git-imap-send with the `--no-curl`\n> +\toption, the only supported method is 'CRAM-MD5'. If this is not set\n> \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n> \n> Examples\n> diff --git a/INSTALL b/INSTALL\n> index 6ec7a24..ffb071e 100644\n> --- a/INSTALL\n> +++ b/INSTALL\n> @@ -108,18 +108,21 @@ Issues of note:\n> \t so you might need to install additional packages other than Perl\n> \t itself, e.g. Time::HiRes.\n> \n> -\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n> -\t If you don't need it, use NO_OPENSSL.\n> +\t- git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n> +\t you are using libcurl older than 7.34.0. Otherwise you can use\n> +\t NO_OPENSSL without losing git-imap-send.\n> \n> \t By default, git uses OpenSSL for SHA1 but it will use its own\n> \t library (inspired by Mozilla's) with either NO_OPENSSL or\n> \t BLK_SHA1. Also included is a version optimized for PowerPC\n> \t (PPC_SHA1).\n> \n> -\t- \"libcurl\" library is used by git-http-fetch and git-fetch. You\n> -\t might also want the \"curl\" executable for debugging purposes.\n> -\t If you do not use http:// or https:// repositories, you do not\n> -\t have to have them (use NO_CURL).\n> +\t- \"libcurl\" library is used by git-http-fetch, git-fetch, and, if\n> +\t the curl version >= 7.34.0, for git-imap-send. You might also\n> +\t want the \"curl\" executable for debugging purposes. If you do not\n> +\t use http:// or https:// repositories, and do not want to put\n> +\t patches into an IMAP mailbox, you do not have to have them\n> +\t (use NO_CURL).\n> \n> \t- \"expat\" library; git-http-push uses it for remote lock\n> \t management over DAV. Similar to \"curl\" above, this is optional\n> diff --git a/Makefile b/Makefile\n> index 827006b..14d5ac1 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -995,6 +995,9 @@ ifdef HAVE_ALLOCA_H\n> \tBASIC_CFLAGS += -DHAVE_ALLOCA_H\n> endif\n> \n> +IMAP_SEND_BUILDDEPS =\n> +IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n> +\n> ifdef NO_CURL\n> \tBASIC_CFLAGS += -DNO_CURL\n> \tREMOTE_CURL_PRIMARY =\n> @@ -1029,6 +1032,15 @@ else\n> \t\t\tPROGRAM_OBJS += http-push.o\n> \t\tendif\n> \tendif\n> +\tcurl_check := $(shell (echo 072200; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)\n> +\tifeq \"$(curl_check)\" \"072200\"\n> +\t\tUSE_CURL_FOR_IMAP_SEND = YesPlease\n> +\tendif\n> +\tifdef USE_CURL_FOR_IMAP_SEND\n> +\t\tBASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND\n> +\t\tIMAP_SEND_BUILDDEPS = http.o\n> +\t\tIMAP_SEND_LDFLAGS += $(CURL_LIBCURL)\n> +\tendif\n> \tifndef NO_EXPAT\n> \t\tifdef EXPATDIR\n> \t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n> @@ -1874,7 +1886,7 @@ gettext.sp gettext.s gettext.o: GIT-PREFIX\n> gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \\\n> \t-DGIT_LOCALE_PATH='\"$(localedir_SQ)\"'\n> \n> -http-push.sp http.sp http-walker.sp remote-curl.sp: SPARSE_FLAGS += \\\n> +http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS += \\\n> \t-DCURL_DISABLE_TYPECHECK\n> \n> ifdef NO_EXPAT\n> @@ -1895,9 +1907,9 @@ endif\n> git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n> \n> -git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n> +git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n> -\t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n> +\t\t$(LIBS) $(IMAP_SEND_LDFLAGS)\n> \n> git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n> \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n> diff --git a/imap-send.c b/imap-send.c\n> index 7f9d30e..08271d9 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -30,13 +30,18 @@\n> #ifdef NO_OPENSSL\n> typedef void *SSL;\n> #endif\n> +#ifdef USE_CURL_FOR_IMAP_SEND\n> +#include \"http.h\"\n> +#endif\n> \n> static int verbosity;\n> +static int use_curl; /* strictly opt in */\n> \n> -static const char * const imap_send_usage[] = { \"git imap-send [-v] [-q] < \", NULL };\n> +static const char * const imap_send_usage[] = { \"git imap-send [-v] [-q] [--[no-]curl] < \", NULL };\n> \n> static struct option imap_send_options[] = {\n> \tOPT__VERBOSITY(&verbosity),\n> +\tOPT_BOOL(0, \"curl\", &use_curl, \"use libcurl to communicate with the IMAP server\"),\n> \tOPT_END()\n> };\n> \n> @@ -1344,14 +1349,141 @@ static void git_imap_config(void)\n> \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n> }\n> \n> -int main(int argc, char **argv)\n> +static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total)\n> {\n> -\tstruct strbuf all_msgs = STRBUF_INIT;\n> \tstruct strbuf msg = STRBUF_INIT;\n> \tstruct imap_store *ctx = NULL;\n> \tint ofs = 0;\n> \tint r;\n> -\tint total, n = 0;\n> +\tint n = 0;\n> +\n> +\tctx = imap_open_store(server, server->folder);\n> +\tif (!ctx) {\n> +\t\tfprintf(stderr, \"failed to open store\\n\");\n> +\t\treturn 1;\n> +\t}\n> +\tctx->name = server->folder;\n> +\n> +\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n> +\twhile (1) {\n> +\t\tunsigned percent = n * 100 / total;\n> +\n> +\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n> +\n> +\t\tif (!split_msg(all_msgs, &msg, &ofs))\n> +\t\t\tbreak;\n> +\t\tif (server->use_html)\n> +\t\t\twrap_in_html(&msg);\n> +\t\tr = imap_store_msg(ctx, &msg);\n> +\t\tif (r != DRV_OK)\n> +\t\t\tbreak;\n> +\t\tn++;\n> +\t}\n> +\tfprintf(stderr, \"\\n\");\n> +\n> +\timap_close_store(ctx);\n> +\n> +\treturn 0;\n> +}\n> +\n> +#ifdef USE_CURL_FOR_IMAP_SEND\n> +static CURL *setup_curl(struct imap_server_conf *srvc)\n> +{\n> +\tCURL *curl;\n> +\tstruct strbuf path = STRBUF_INIT;\n> +\tstruct strbuf auth = STRBUF_INIT;\n> +\n> +\tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n> +\t\tdie(\"curl_global_init failed\");\n> +\n> +\tcurl = curl_easy_init();\n> +\n> +\tif (!curl)\n> +\t\tdie(\"curl_easy_init failed\");\n> +\n> +\tcurl_easy_setopt(curl, CURLOPT_USERNAME, server.user);\n> +\tcurl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);\n> +\n> +\tstrbuf_addstr(&path, server.host);\n> +\tif (!path.len || path.buf[path.len - 1] != '/')\n> +\t\tstrbuf_addch(&path, '/');\n> +\tstrbuf_addstr(&path, server.folder);\n> +\n> +\tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n> +\tstrbuf_release(&path);\n> +\tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n> +\n> +\tif (server.auth_method) {\n> +\t\tstrbuf_addstr(&auth, \"AUTH=\");\n> +\t\tstrbuf_addstr(&auth, server.auth_method);\n> +\t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n> +\t}\n> +\tstrbuf_release(&auth);\n> +\n> +\tif (server.use_ssl)\n> +\t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);\n> +\n> +\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);\n> +\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);\n> +\n> +\tcurl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);\n> +\n> +\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n> +\n> +\tif (0 < verbosity)\n> +\t\tcurl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);\n> +\n> +\treturn curl;\n> +}\n> +\n> +static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n> +\tint ofs = 0;\n> +\tint n = 0;\n> +\tstruct buffer msgbuf = { STRBUF_INIT, 0 };\n> +\tCURL *curl;\n> +\tCURLcode res = CURLE_OK;\n> +\n> +\tcurl = setup_curl(server);\n> +\tcurl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);\n> +\n> +\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n> +\twhile (1) {\n> +\t\tunsigned percent = n * 100 / total;\n> +\n> +\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n> +\n> +\t\tint prev_len = msgbuf.buf.len;\n> +\t\tif (!split_msg(all_msgs, &msgbuf.buf, &ofs))\n> +\t\t\tbreak;\n> +\t\tif (server->use_html)\n> +\t\t\twrap_in_html(&msgbuf.buf);\n> +\t\tlf_to_crlf(&msgbuf.buf);\n> +\n> +\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));\n> +\n> +\t\tres = curl_easy_perform(curl);\n> +\n> +\t\tif(res != CURLE_OK) {\n> +\t\t\tfprintf(stderr, \"curl_easy_perform() failed: %s\\n\",\n> +\t\t\t\t\tcurl_easy_strerror(res));\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tn++;\n> +\t}\n> +\tfprintf(stderr, \"\\n\");\n> +\n> +\tcurl_easy_cleanup(curl);\n> +\tcurl_global_cleanup();\n> +\n> +\treturn 0;\n> +}\n> +#endif\n> +\n> +int main(int argc, char **argv)\n> +{\n> +\tstruct strbuf all_msgs = STRBUF_INIT;\n> +\tint total;\n> \tint nongit_ok;\n> \n> \tgit_extract_argv0_path(argv[0]);\n> @@ -1366,6 +1498,13 @@ int main(int argc, char **argv)\n> \tif (argc)\n> \t\tusage_with_options(imap_send_usage, imap_send_options);\n> \n> +#ifndef USE_CURL_FOR_IMAP_SEND\n> +\tif (use_curl) {\n> +\t\twarning(\"--use-curl not supported in this build\");\n> +\t\tuse_curl = 0;\n> +\t}\n> +#endif\n> +\n> \tif (!server.port)\n> \t\tserver.port = server.use_ssl ? 993 : 143;\n> \n> @@ -1399,29 +1538,14 @@ int main(int argc, char **argv)\n> \t}\n> \n> \t/* write it to the imap server */\n> -\tctx = imap_open_store(&server, server.folder);\n> -\tif (!ctx) {\n> -\t\tfprintf(stderr, \"failed to open store\\n\");\n> -\t\treturn 1;\n> -\t}\n> -\n> -\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n> -\twhile (1) {\n> -\t\tunsigned percent = n * 100 / total;\n> \n> -\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n> -\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n> -\t\t\tbreak;\n> -\t\tif (server.use_html)\n> -\t\t\twrap_in_html(&msg);\n> -\t\tr = imap_store_msg(ctx, &msg);\n> -\t\tif (r != DRV_OK)\n> -\t\t\tbreak;\n> -\t\tn++;\n> -\t}\n> -\tfprintf(stderr, \"\\n\");\n> +\tif (server.tunnel)\n> +\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n> \n> -\timap_close_store(ctx);\n> +#ifdef USE_CURL_FOR_IMAP_SEND\n> +\tif (use_curl)\n> +\t\treturn curl_append_msgs_to_imap(&server, &all_msgs, total);\n> +#endif\n> \n> -\treturn 0;\n> +\treturn append_msgs_to_imap(&server, &all_msgs, total);\n> }\n> \n"},{"id":"251627","messageId":"xmqqy4rkb74v.fsf@gitster.dls.corp.google.com","threadId":"37420","inReplyTo":"545FA3FA.10308@ramsay1.demon.co.uk","subject":"Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-09T17:45:04Z","receivedAt":"2014-11-09T17:45:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n>> In order to suppress a sparse warning about \"using sizeof on a\n>> function\", we use the same solution used in commit 9371322a6\n>> (\"sparse: suppress some \"using sizeof on a function\" warnings\",\n>> 06-10-2013) which solved exactly this problem for the other commands\n>> using libcurl.\n>\n> Although it doesn't hurt, I don't think this 'problem' deserves\n> so many (or any) inches in the commit message. ;-)\n\nUse smaller line pitch, then? ;-) I am sure myself 6 months down the\nroad as a \"git show\" reader of this patch would appreciate these\nfive lines when I scratch my head looking at the Makefile changes.\n\n>> Signed-off-by: Bernhard Reiter \n>> Signed-off-by: Ramsay Jones \n>\n> Since I didn't actually review this patch, or make any significant\n> contribution to the code (it's not even enough to be copyright-able!),\n> then this 'Signed-off-by:' should not be included. At *most* you might\n> want to put a 'Helped-by:' _prior_ to your sign-off.\n\nYes, that is the appropriate thing to do, I would think.\n\nThanks.\n"},{"id":"251683","messageId":"xmqqfvdr9dob.fsf@gitster.dls.corp.google.com","threadId":"37420","inReplyTo":"545F8079.7010806@raz.or.at","subject":"Re: [PATCH] git-imap-send: use libcurl for implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-10T17:19:00Z","receivedAt":"2014-11-10T17:19:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bernhard Reiter writes:\n\n> Am 2014-11-09 um 14:00 schrieb Torsten Bögershausen:\n>> On 2014-08-27 00.40, Bernhard Reiter wrote:\n>>> Use libcurl's high-level API functions to implement git-imap-send\n>>> instead of the previous low-level OpenSSL-based functions.\n>>>\n>> This doesn't seem to fully work under Debian 7:\n>> /home/tb/projects/git/git.pu/imap-send.c:1546: undefined reference\n>> to `curl_append_msgs_to_imap'\n>\n> Thx for the notice. I forgot to guard that with an #ifdef.\n>\n> The new patch below includes that, and the fix sent by Ramsay;\n> hopefully the squashed/edited commit message is fine.\n\nQueued with a small fix-ups, including\n\n - line-fold a couple of overlong lines;\n\n - avoid decl-after-stmt of \"int prev_len\";\n\n - reduce the scope of \"struct struct auth\" down to only the block\n it is used;\n\n - the footer of the log message now reads \"helped-by ramsay\", your\n sign-off and then mine.\n\nThanks.\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 08271d9..4dfe4c2 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1349,7 +1349,8 @@ static void git_imap_config(void)\n \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n }\n \n-static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total)\n+static int append_msgs_to_imap(struct imap_server_conf *server,\n+\t\t\t struct strbuf* all_msgs, int total)\n {\n \tstruct strbuf msg = STRBUF_INIT;\n \tstruct imap_store *ctx = NULL;\n@@ -1391,7 +1392,6 @@ static CURL *setup_curl(struct imap_server_conf *srvc)\n {\n \tCURL *curl;\n \tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf auth = STRBUF_INIT;\n \n \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n \t\tdie(\"curl_global_init failed\");\n@@ -1414,11 +1414,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc)\n \tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n \n \tif (server.auth_method) {\n+\t\tstruct strbuf auth = STRBUF_INIT;\n \t\tstrbuf_addstr(&auth, \"AUTH=\");\n \t\tstrbuf_addstr(&auth, server.auth_method);\n \t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n+\t\tstrbuf_release(&auth);\n \t}\n-\tstrbuf_release(&auth);\n \n \tif (server.use_ssl)\n \t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);\n@@ -1436,7 +1437,8 @@ static CURL *setup_curl(struct imap_server_conf *srvc)\n \treturn curl;\n }\n \n-static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n+static int curl_append_msgs_to_imap(struct imap_server_conf *server,\n+\t\t\t\t struct strbuf* all_msgs, int total) {\n \tint ofs = 0;\n \tint n = 0;\n \tstruct buffer msgbuf = { STRBUF_INIT, 0 };\n@@ -1449,17 +1451,19 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strb\n \tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n \twhile (1) {\n \t\tunsigned percent = n * 100 / total;\n+\t\tint prev_len;\n \n \t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n \n-\t\tint prev_len = msgbuf.buf.len;\n+\t\tprev_len = msgbuf.buf.len;\n \t\tif (!split_msg(all_msgs, &msgbuf.buf, &ofs))\n \t\t\tbreak;\n \t\tif (server->use_html)\n \t\t\twrap_in_html(&msgbuf.buf);\n \t\tlf_to_crlf(&msgbuf.buf);\n \n-\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));\n+\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,\n+\t\t\t\t (curl_off_t)(msgbuf.buf.len-prev_len));\n \n \t\tres = curl_easy_perform(curl);\n \n"}]}