Re: [PATCH] imap-send: use libcurl for implementation
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:51
Bernhard Reiter [off-list ref] writes:
quoted hunk
@@ -25,7 +25,6 @@ Typical usage is something like: git format-patch --signoff --stdout --attach origin | git imap-send - OPTIONS
Why?
quoted hunk
@@ -37,6 +36,17 @@ OPTIONS --quiet:: Be quiet. +--curl:: + Use libcurl to communicate with the IMAP server, unless tunneling + into it. Only available if git was built with the + USE_CURL_FOR_IMAP_SEND option set, in which case this is the + default behavior. + +--no-curl:: + Talk to the IMAP server using git's own IMAP routines instead of + using libcurl. Only available git was built with the + USE_CURL_FOR_IMAP_SEND option set; implicitly assumed otherwise. +
I think we tend to spell "Git" not "git" when we refer to the software suite as a whole. More importantly, the description on these two items are no longer in line with the implementation, aren't they? We accept these options but warn and a build without libcurl ignores --curl with a warning, and --curl is not default in any build.
quoted hunk
@@ -87,7 +97,9 @@ imap.preformattedHTML:: imap.authMethod:: Specify authenticate method for authentication with IMAP server. - Current supported method is 'CRAM-MD5' only. If this is not set + If git was built with the NO_CURL option, or if your curl version is + < 7.34.0, or if you're running git-imap-send with the --no-curl
s/< /older than /; Also quote --no-curl inside bq-pair, i.e. `--no-curl`, as that is something the user will type as-is.
quoted hunk
+ option, the only supported method is 'CRAM-MD5'. If this is not set then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Examplesdiff --git a/INSTALL b/INSTALL index 6ec7a24..ffb071e 100644 --- a/INSTALL +++ b/INSTALL@@ -108,18 +108,21 @@ Issues of note: so you might need to install additional packages other than Perl itself, e.g. Time::HiRes. - - "openssl" library is used by git-imap-send to use IMAP over SSL. - If you don't need it, use NO_OPENSSL. + - git-imap-send needs the OpenSSL library to talk IMAP over SSL if + you are using libcurl older than 7.34.0. Otherwise you can use + NO_OPENSSL without losing git-imap-send.
OK.
+ - "libcurl" library is used by git-http-fetch, git-fetch, and, if + the curl version >= 7.34.0, for git-imap-send. You might also + want the "curl" executable for debugging purposes. If you do not + use http:// or https:// repositories, and do not want to put + patches into an IMAP mailbox, you do not have to have them + (use NO_CURL).
OK.
quoted hunk
diff --git a/imap-send.c b/imap-send.c index 7f9d30e..01ce175 100644 --- a/imap-send.c +++ b/imap-send.c@@ -30,13 +30,18 @@ #ifdef NO_OPENSSL typedef void *SSL; #endif +#ifdef USE_CURL_FOR_IMAP_SEND +#include "http.h" +#endif static int verbosity; +static int use_curl; /* strictly opt in */ -static const char * const imap_send_usage[] = { "git imap-send [-v] [-q] < <mbox>", NULL }; +static const char * const imap_send_usage[] = { "git imap-send [-v] [-q] [--[no-]curl] < <mbox>", NULL }; static struct option imap_send_options[] = { OPT__VERBOSITY(&verbosity), + OPT_BOOL(0, "curl", &use_curl, "use libcurl to communicate with the IMAP server"), OPT_END() };@@ -1344,14 +1349,138 @@ static void git_imap_config(void) git_config_get_string("imap.authmethod", &server.auth_method); } -int main(int argc, char **argv) -{ - struct strbuf all_msgs = STRBUF_INIT; +static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {
The opening brace sits on its own line by itself, so
+#ifdef USE_CURL_FOR_IMAP_SEND
+static CURL *setup_curl(struct imap_server_conf *srvc)
+{
+ CURL *curl;
+ struct strbuf path = STRBUF_INIT;
+ struct strbuf auth = STRBUF_INIT;
+
+ if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
+ die("curl_global_init failed");
+
+ curl = curl_easy_init();
+
+ if (!curl)
+ die("curl_easy_init failed");
+
+ curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
+ curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
+
+ strbuf_addstr(&path, server.host);
+ if (!path.len || path.buf[path.len - 1] != '/')
+ strbuf_addch(&path, '/');
+ strbuf_addstr(&path, server.folder);
+
+ curl_easy_setopt(curl, CURLOPT_URL, path.buf);
+ curl_easy_setopt(curl, CURLOPT_PORT, server.port);
+
+ if (server.auth_method) {
+ strbuf_addstr(&auth, "AUTH=");
+ strbuf_addstr(&auth, server.auth_method);
+ curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);
+ }Are path.buf and auth.buf leaked here, or does CURL *curl take possession of them by curl_easy_setopt() and not freeing them ourselves is the right thing to do? Assuming that is the case, perhaps we would want to use strbuf_detach() on &path and &auth to make it clear that is what is going on?
quoted hunk
+int main(int argc, char **argv) +{ + struct strbuf all_msgs = STRBUF_INIT; + int total; int nongit_ok; git_extract_argv0_path(argv[0]);@@ -1360,12 +1489,18 @@ int main(int argc, char **argv) setup_git_directory_gently(&nongit_ok); git_imap_config(); - argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);
Why? Thanks.