Bernhard Reiter <ockham@xxxxxxxxx> writes:
> @@ -25,7 +25,6 @@ Typical usage is something like:
>
> git format-patch --signoff --stdout --attach origin | git imap-send
>
> -
> OPTIONS
Why?
> @@ -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.
> @@ -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.
> + option, the only supported method is 'CRAM-MD5'. If this is not set
> then 'git imap-send' uses the basic IMAP plaintext LOGIN command.
>
> Examples
> diff --git a/INSTALL b/INSTALL
> index 6ec7a24..ffb071e 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -108,18 +108,21 @@ Issues of note:
> so you might need to install additional packages other than Perl
> itself, e.g. Time::HiRes.
>
> - - "openssl" library is used by git-imap-send to use IMAP over SSL.
> - If you don't need it, use NO_OPENSSL.
> + - git-imap-send needs the OpenSSL library to talk IMAP over SSL if
> + you are using libcurl older than 7.34.0. Otherwise you can use
> + NO_OPENSSL without losing git-imap-send.
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.
> 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?
> +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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html