Re: Stable GnuPG interface, git should use GPGME
Am Freitag 10 März 2017 15:23:27 schrieb Ævar Arnfjörð Bjarmason: > On Fri, Mar 10, 2017 at 11:00 AM, Bernhard E. Reiter wrote > > please consider using libgpgme interfacing to GnuPG, because the gpg > > command-line interface is not considered an official API to GnuPG by the > > GnuPG-devs and thus potentially unstable. > > == Usability problem with `gpg2` vs `gpg` > > > > My use case today was signing and git by default found the `gpg` binary > > by default and the command failed. I've mentioned this as one example for a possible advantage using libgpgme when interfacing with GnuPG. > > The reason is that I have `gpg2` > > installed and most applications use it right away. So git failed signing > > because the .gnupg configuration of the user was not ready for the old > > `gpg` which is still installed on Debian GNU/Linux for purposes of the > > operating system. If git would have used libgpgme, gpgme would have > > choosen the most uptodate version of `gpg` available (or configured) > > without me intervening via gpg.program. Now because of this problem you > > could adding a check for `gpg2` and fallback to `gpg`, but even better > > would be to move to libgpgme. >:) > > I'm on Debian but haven't had these issues. What's your gpg & gpg2 > --version & Debian release? And what in particular failed? If you use options in your configuration that only gpg2 understands, gpg(1) will barf. For example the following lines in ~/.gnupg/gpg.conf debug-level basic log-file socket:///home/bern/.gnupg/log-socket will lead to LANG=C gpg -K gpg: /powerhome/bern/.gnupg/gpg.conf:102: argument not expected gpg: /powerhome/bern/.gnupg/gpg.conf:103: invalid option where gpg2 works as expected. As a number of application already uses gpg2 (via libgpgme or not), this may go unnoticed for a while. So when I've started to sign with git on this machine I ran into the problem (current Jessie default versions): dpkg -s gnupg | grep ^Version #Version: 1.4.18-7+deb8u3 dpkg -s gnupg2 | grep ^Version #Version: 2.0.26-6+deb8u1 Workarounds are: * Use a different config for gpg2 and gpg, e.g. ~/.gnupg/gpg.conf-2 (https://www.gnupg.org/documentation/manuals/gnupg-2.0/Invoking-GPG.html ) * or set gpg.program for git to gpg2. > And what git version was this? I see we've had a couple of workarounds > for gpg2, in particular Linus's v2.8.4-1-gb624a3e67f, but if you have > v2.10.0 or later that won't fix whatever issue you had. dpkg -s git | grep ^Version Version: 1:2.1.4-2.1+deb8u2 (I've checked the most current master source to see that git still calls gpg.program, otherwise followed advise on https://git-scm.com/community to send reports and questions to the list.) > Using the library sounds good, but a shorter-term immediate fix would > be to figure out what bug you encountered in our use of the > command-line version, and see if we've fixed that already or not. > Regardless of what we do with a gpg library in the future some distros > might want to backport such a small patch if we can come up with it. I guess a good simple approach would be to try "gpg2" first and then fall back to "gpg" or "gpgv" in case only these version are available. (Here is a report that puts forward using gpgv in some situations https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=852684 ) As there are other subtle potential issues with directly calling a gpg binary, using libgpgme by default probably has other advantages as well. And if there are important functions missing the GnuPG-devs would like to hear about them. Regards, Bernhard -- www.intevation.de/~bernhard +49 541 33 508 3-3 Intevation GmbH, Osnabrück, DE; Amtsgericht Osnabrück, HRB 18998 Geschäftsführer Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner signature.asc Description: This is a digitally signed message part.
Re: [PATCH] git-imap-send: use libcurl for implementation
Am 2014-11-09 um 14:00 schrieb Torsten Bögershausen: On 2014-08-27 00.40, Bernhard Reiter wrote: Use libcurl's high-level API functions to implement git-imap-send instead of the previous low-level OpenSSL-based functions. This doesn't seem to fully work under Debian 7: /home/tb/projects/git/git.pu/imap-send.c:1546: undefined reference to `curl_append_msgs_to_imap' Thx for the notice. I forgot to guard that with an #ifdef. The new patch below includes that, and the fix sent by Ramsay; hopefully the squashed/edited commit message is fine. Bernhard -- 8 -- Use libcurl's high-level API functions to implement git-imap-send instead of the previous low-level OpenSSL-based functions. Since version 7.30.0, libcurl's API has been able to communicate with IMAP servers. Using those high-level functions instead of the current ones would reduce imap-send.c by some 1200 lines of code. For now, the old ones are wrapped in #ifdefs, and the new functions are enabled by make if curl's version is = 7.34.0, from which version on curl's CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been available. The low-level functions will still be used for tunneling into the server for now. As I don't have access to that many IMAP servers, I haven't been able to test the new code with a wide variety of parameter combinations. I did test both secure and insecure (imaps:// and imap://) connections and values of PLAIN and LOGIN for the authMethod. In order to suppress a sparse warning about using sizeof on a function, we use the same solution used in commit 9371322a6 (sparse: suppress some using sizeof on a function warnings, 06-10-2013) which solved exactly this problem for the other commands using libcurl. Signed-off-by: Bernhard Reiter ock...@raz.or.at Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- Documentation/git-imap-send.txt | 15 +++- INSTALL | 15 ++-- Makefile| 18 +++- imap-send.c | 176 ++-- 4 files changed, 187 insertions(+), 37 deletions(-) diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 0897131..77aacf1 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder SYNOPSIS [verse] -'git imap-send' [-v] [-q] +'git imap-send' [-v] [-q] [--[no-]curl] DESCRIPTION @@ -37,6 +37,15 @@ OPTIONS --quiet:: Be quiet. +--curl:: + Use libcurl to communicate with the IMAP server, unless tunneling + into it. Ignored if Git was built without the USE_CURL_FOR_IMAP_SEND + option set. + +--no-curl:: + Talk to the IMAP server using git's own IMAP routines instead of + using libcurl. + CONFIGURATION - @@ -87,7 +96,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 older + than 7.34.0, or if you're running git-imap-send with the `--no-curl` + option, the only supported method is 'CRAM-MD5'. If this is not set then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Examples diff --git a/INSTALL b/INSTALL index 6ec7a24..ffb071e 100644 --- a/INSTALL +++ b/INSTALL @@ -108,18 +108,21 @@ Issues of note: so you might need to install additional packages other than Perl itself, e.g. Time::HiRes. - - openssl library is used by git-imap-send to use IMAP over SSL. - If you don't need it, use NO_OPENSSL. + - git-imap-send needs the OpenSSL library to talk IMAP over SSL if + you are using libcurl older than 7.34.0. Otherwise you can use + NO_OPENSSL without losing git-imap-send. By default, git uses OpenSSL for SHA1 but it will use its own library (inspired by Mozilla's) with either NO_OPENSSL or BLK_SHA1. Also included is a version optimized for PowerPC (PPC_SHA1). - - libcurl library is used by git-http-fetch and git-fetch. You - might also want the curl executable for debugging purposes. - If you do not use http:// or https:// repositories, you do not - have to have them (use NO_CURL). + - libcurl library is used by git-http-fetch, git-fetch, and, if + the curl version = 7.34.0, for git-imap-send. You might also + want the curl executable for debugging purposes. If you do not + use http:// or https:// repositories, and do not want to put + patches into an IMAP mailbox, you do not have to have them + (use NO_CURL). - expat library; git-http-push uses it for remote lock management over DAV. Similar to curl above, this is optional diff
Re: [PATCH] imap-send: use libcurl for implementation
Zitat von Junio C Hamano gits...@pobox.com: Bernhard Reiter ock...@raz.or.at writes: @@ -25,7 +25,6 @@ Typical usage is something like: git format-patch --signoff --stdout --attach origin | git imap-send - OPTIONS Why? By mistake when rebasing. Sorry, fixed. (Same for previous -1360,12 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. Fixed. @@ -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. Fixed. [...] 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 Fixed (I hope there wasn't more...) +#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? I looked it up at http://curl.haxx.se/libcurl/c/curl_easy_setopt.html -- char* parameters are actually string copied by curl_easy_setopt(), so I'm now strbuf_release()ing path and auth after use. Bernhard -- 8 -- Use libcurl's high-level API functions to implement git-imap-send instead of the previous low-level OpenSSL-based functions. Since version 7.30.0, libcurl's API has been able to communicate with IMAP servers. Using those high-level functions instead of the current ones would reduce imap-send.c by some 1200 lines of code. For now, the old ones are wrapped in #ifdefs, and the new functions are enabled by make if curl's version is = 7.34.0, from which version on curl's CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been available. The low-level functions will still be used for tunneling into the server for now. As I don't have access to that many IMAP servers, I haven't
[PATCH] imap-send: use libcurl for implementation
Use libcurl's high-level API functions to implement git-imap-send instead of the previous low-level OpenSSL-based functions. Since version 7.30.0, libcurl's API has been able to communicate with IMAP servers. Using those high-level functions instead of the current ones would reduce imap-send.c by some 1200 lines of code. For now, the old ones are wrapped in #ifdefs, and the new functions are enabled by make if curl's version is = 7.34.0, from which version on curl's CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been available. The low-level functions will still be used for tunneling into the server for now. As I don't have access to that many IMAP servers, I haven't been able to test the new code with a wide variety of parameter combinations. I did test both secure and insecure (imaps:// and imap://) connections and values of PLAIN and LOGIN for the authMethod. Signed-off-by: Bernhard Reiter ock...@raz.or.at --- Fixed according to xmqqzjcewq6p@gitster.dls.corp.google.com, and based upon master with f1a35295c2b66d2501f034d864afb2c5d8bb0e08 cherry-picked. Documentation/git-imap-send.txt | 18 - INSTALL | 15 ++-- Makefile| 16 +++- imap-send.c | 174 +--- 4 files changed, 184 insertions(+), 39 deletions(-) diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 0897131..e70ee56 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder SYNOPSIS [verse] -'git imap-send' [-v] [-q] +'git imap-send' [-v] [-q] [--[no-]curl] DESCRIPTION @@ -25,7 +25,6 @@ Typical usage is something like: git format-patch --signoff --stdout --attach origin | git imap-send - OPTIONS --- @@ -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. + CONFIGURATION - @@ -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 + option, the only supported method is 'CRAM-MD5'. If this is not set then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Examples diff --git a/INSTALL b/INSTALL index 6ec7a24..ffb071e 100644 --- a/INSTALL +++ b/INSTALL @@ -108,18 +108,21 @@ Issues of note: so you might need to install additional packages other than Perl itself, e.g. Time::HiRes. - - openssl library is used by git-imap-send to use IMAP over SSL. - If you don't need it, use NO_OPENSSL. + - git-imap-send needs the OpenSSL library to talk IMAP over SSL if + you are using libcurl older than 7.34.0. Otherwise you can use + NO_OPENSSL without losing git-imap-send. By default, git uses OpenSSL for SHA1 but it will use its own library (inspired by Mozilla's) with either NO_OPENSSL or BLK_SHA1. Also included is a version optimized for PowerPC (PPC_SHA1). - - libcurl library is used by git-http-fetch and git-fetch. You - might also want the curl executable for debugging purposes. - If you do not use http:// or https:// repositories, you do not - have to have them (use NO_CURL). + - libcurl library is used by git-http-fetch, git-fetch, and, if + the curl version = 7.34.0, for git-imap-send. You might also + want the curl executable for debugging purposes. If you do not + use http:// or https:// repositories, and do not want to put + patches into an IMAP mailbox, you do not have to have them + (use NO_CURL). - expat library; git-http-push uses it for remote lock management over DAV. Similar to curl above, this is optional diff --git a/Makefile b/Makefile index 827006b..fb954a9 100644 --- a/Makefile +++ b/Makefile @@ -995,6 +995,9 @@ ifdef HAVE_ALLOCA_H BASIC_CFLAGS += -DHAVE_ALLOCA_H endif +IMAP_SEND_BUILDDEPS = +IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) + ifdef NO_CURL BASIC_CFLAGS += -DNO_CURL REMOTE_CURL_PRIMARY = @@ -1029,6 +1032,15 @@ else PROGRAM_OBJS += http-push.o
imap-send: Use parse options API to determine verbosity
Signed-off-by: Bernhard Reiter ock...@raz.or.at --- As requested per xmqqzjcewq6p@gitster.dls.corp.google.com. Documentation/git-imap-send.txt | 14 +- imap-send.c | 25 +++-- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index c7c0d21..0897131 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder SYNOPSIS [verse] -'git imap-send' +'git imap-send' [-v] [-q] DESCRIPTION @@ -26,6 +26,18 @@ Typical usage is something like: git format-patch --signoff --stdout --attach origin | git imap-send +OPTIONS +--- + +-v:: +--verbose:: + Be verbose. + +-q:: +--quiet:: + Be quiet. + + CONFIGURATION - diff --git a/imap-send.c b/imap-send.c index 70bcc7a..7f40960 100644 --- a/imap-send.c +++ b/imap-send.c @@ -26,11 +26,19 @@ #include credential.h #include exec_cmd.h #include run-command.h +#include parse-options.h #ifdef NO_OPENSSL typedef void *SSL; #endif -static const char imap_send_usage[] = git imap-send mbox; +static int verbosity; + +static const char * const imap_send_usage[] = { git imap-send [-v] [-q] mbox, NULL }; + +static struct option imap_send_options[] = { + OPT__VERBOSITY(verbosity), + OPT_END() +}; #undef DRV_OK #define DRV_OK 0 @@ -38,8 +46,6 @@ static const char imap_send_usage[] = git imap-send mbox; #define DRV_BOX_BAD -2 #define DRV_STORE_BAD -3 -static int Verbose, Quiet; - __attribute__((format (printf, 1, 2))) static void imap_info(const char *, ...); __attribute__((format (printf, 1, 2))) @@ -418,7 +424,7 @@ static int buffer_gets(struct imap_buffer *b, char **s) if (b-buf[b-offset + 1] == '\n') { b-buf[b-offset] = 0; /* terminate the string */ b-offset += 2; /* next line */ - if (Verbose) + if (verbosity = 0) puts(*s); return 0; } @@ -433,7 +439,7 @@ static void imap_info(const char *msg, ...) { va_list va; - if (!Quiet) { + if (verbosity = 0) { va_start(va, msg); vprintf(msg, va); va_end(va); @@ -445,7 +451,7 @@ static void imap_warn(const char *msg, ...) { va_list va; - if (Quiet 2) { + if (verbosity 2) { va_start(va, msg); vfprintf(stderr, msg, va); va_end(va); @@ -522,7 +528,7 @@ static struct imap_cmd *issue_imap_cmd(struct imap_store *ctx, cmd-tag, cmd-cmd, cmd-cb.dlen, CAP(LITERALPLUS) ? + : ); - if (Verbose) { + if (verbosity = 0) { if (imap-num_in_progress) printf((%d in progress) , imap-num_in_progress); if (!starts_with(cmd-cmd, LOGIN)) @@ -1352,12 +1358,11 @@ int main(int argc, char **argv) git_setup_gettext(); - if (argc != 1) - usage(imap_send_usage); - setup_git_directory_gently(nongit_ok); git_imap_config(); + argc = parse_options(argc, (const char **)argv, , imap_send_options, imap_send_usage, 0); + if (!server.port) server.port = server.use_ssl ? 993 : 143; -- 2.1.2.557.g06ecad4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] imap-send: Use parse options API to determine verbosity
Signed-off-by: Bernhard Reiter ock...@raz.or.at --- In reply to xmqqk339ibal@gitster.dls.corp.google.com. Thanks for bearing with me. I should've given the corresponding verbosity values more thought myself in the first place. Documentation/git-imap-send.txt | 14 +- imap-send.c | 25 +++-- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index c7c0d21..0897131 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder SYNOPSIS [verse] -'git imap-send' +'git imap-send' [-v] [-q] DESCRIPTION @@ -26,6 +26,18 @@ Typical usage is something like: git format-patch --signoff --stdout --attach origin | git imap-send +OPTIONS +--- + +-v:: +--verbose:: + Be verbose. + +-q:: +--quiet:: + Be quiet. + + CONFIGURATION - diff --git a/imap-send.c b/imap-send.c index 70bcc7a..6efaae8 100644 --- a/imap-send.c +++ b/imap-send.c @@ -26,11 +26,19 @@ #include credential.h #include exec_cmd.h #include run-command.h +#include parse-options.h #ifdef NO_OPENSSL typedef void *SSL; #endif -static const char imap_send_usage[] = git imap-send mbox; +static int verbosity; + +static const char * const imap_send_usage[] = { git imap-send [-v] [-q] mbox, NULL }; + +static struct option imap_send_options[] = { + OPT__VERBOSITY(verbosity), + OPT_END() +}; #undef DRV_OK #define DRV_OK 0 @@ -38,8 +46,6 @@ static const char imap_send_usage[] = git imap-send mbox; #define DRV_BOX_BAD -2 #define DRV_STORE_BAD -3 -static int Verbose, Quiet; - __attribute__((format (printf, 1, 2))) static void imap_info(const char *, ...); __attribute__((format (printf, 1, 2))) @@ -418,7 +424,7 @@ static int buffer_gets(struct imap_buffer *b, char **s) if (b-buf[b-offset + 1] == '\n') { b-buf[b-offset] = 0; /* terminate the string */ b-offset += 2; /* next line */ - if (Verbose) + if (0 verbosity) puts(*s); return 0; } @@ -433,7 +439,7 @@ static void imap_info(const char *msg, ...) { va_list va; - if (!Quiet) { + if (verbosity = 0) { va_start(va, msg); vprintf(msg, va); va_end(va); @@ -445,7 +451,7 @@ static void imap_warn(const char *msg, ...) { va_list va; - if (Quiet 2) { + if (-2 verbosity) { va_start(va, msg); vfprintf(stderr, msg, va); va_end(va); @@ -522,7 +528,7 @@ static struct imap_cmd *issue_imap_cmd(struct imap_store *ctx, cmd-tag, cmd-cmd, cmd-cb.dlen, CAP(LITERALPLUS) ? + : ); - if (Verbose) { + if (0 verbosity) { if (imap-num_in_progress) printf((%d in progress) , imap-num_in_progress); if (!starts_with(cmd-cmd, LOGIN)) @@ -1352,12 +1358,11 @@ int main(int argc, char **argv) git_setup_gettext(); - if (argc != 1) - usage(imap_send_usage); - setup_git_directory_gently(nongit_ok); git_imap_config(); + argc = parse_options(argc, (const char **)argv, , imap_send_options, imap_send_usage, 0); + if (!server.port) server.port = server.use_ssl ? 993 : 143; -- 2.1.2.556.g44ebd84 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-imap-send: use libcurl for implementation
Resending this once more, as indicated by xmqqbnp4hu8g@gitster.dls.corp.google.com Hope my formatting and posting style is now conformant. Sorry for the noise. Am 2014-08-27 um 19:20 schrieb Junio C Hamano: Bernhard Reiter ock...@raz.or.at writes: [...] For now, the old ones are wrapped in #ifdefs, and the new functions are enabled by make if curl's version is = 7.35.0, from which version on curl's CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been available. https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions says that this was introduced as of 7.34.0, though. Strange, I thought I recalled having seen that in http://curl.haxx.se/libcurl/c/CURLOPT_LOGIN_OPTIONS.html but it clearly says 7.34.0 there too. I've now changed all occurrences of 7.35.0 to 7.34.0 (and the corresponding hex value in the Makefile). As I don't have access to that many IMAP servers, I haven't been able to test the new code with a wide variety of parameter combinations. I did test both secure and insecure (imaps:// and imap://) connections and values of PLAIN and LOGIN for the authMethod. Perhaps CC'ing those who have touched git-imap-send code over the years and asking for their help testing might help? CC'ing them (going back about 2 years, which already makes the list quite long) and the people who have taken part in the initial discussion on this feature in August. And the related Debian bug. Please test this, folks! Signed-off-by: Bernhard Reiter ock...@raz.or.at --- I rebased the patch on the pu branch, hope that was the right thing to do. Usually I would appreciate a patch for a new feature not meant for the maintenance tracks to be based on 'master', so that it can go to the next release without having to wait other changes that may conflict with it and that may not yet be ready. I will try to apply this one to 'pu', rebase it on 'master' to make sure the result does not depend on the other topics in flight, and then merge it back to 'pu'. Okay, I'll stick to master. I've rebased on master now that the first couple related patches are there anyway. [...] diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 7d991d9..9d244c4 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -75,7 +75,8 @@ imap.preformattedHTML:: imap.authMethod:: Specify authenticate method for authentication with IMAP server. -Current supported method is 'CRAM-MD5' only. If this is not set +If you compiled git with the NO_CURL option or if your curl version is + 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Hmph, so there is no option that lets me say I know my libcurl is new enough but I have some reason not to want to use the new code to interact with my imap server, at compile time or (more preferrably) at runtime? Added a runtime option, see below. diff --git a/INSTALL b/INSTALL index 6ec7a24..e2770a0 100644 --- a/INSTALL +++ b/INSTALL @@ -108,18 +108,21 @@ Issues of note: so you might need to install additional packages other than Perl itself, e.g. Time::HiRes. -- openssl library is used by git-imap-send to use IMAP over SSL. - If you don't need it, use NO_OPENSSL. +- openssl library is used by git-imap-send to use IMAP over SSL, + unless you're using curl = 7.35.0, in which case that will be + used. If you don't need git-imap-send, you can use NO_OPENSSL. The last sentence makes it unclear which of the following is true: - I have sufficiently new libcurl. I cannot say NO_OPENSSL because I do need git-imap-send. - I have sufficiently new libcurl, so openssl is not used by git-imap send for me. I can say NO_OPENSSL. Perhaps - git-imap-send needs the OpenSSL library to talk IMAP over SSL if you are using libCurl older than 7.35.0. Otherwise you can use NO_OPENSSL without losing git-imap-send. Fixed. diff --git a/git.spec.in b/git.spec.in index d61d537..9535cc3 100644 --- a/git.spec.in +++ b/git.spec.in @@ -8,7 +8,7 @@ License: GPL Group: Development/Tools URL:http://kernel.org/pub/software/scm/git/ Source: http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz -BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3} +BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel = 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3} This is very iffy. It incompatible with the body of the patch where you allow older curl library and because you depend on openssl-devel you wouldn't lose imap-send. Okay, removed the version requirement. @@ -1391,29 +1518,13 @@ int main(int argc, char **argv) [...] Much more
Fwd: Re: [PATCH] git-imap-send: use libcurl for implementation
*ping* Hope I didn't mess up formatting again... Or do I need to top-post, as the original thread is too old to keep posting to it? Bernhard Weitergeleitete Nachricht Betreff: Re: [PATCH] git-imap-send: use libcurl for implementation Datum: Sun, 12 Oct 2014 17:22:20 +0200 Von: Bernhard Reiter ock...@raz.or.at An: Junio C Hamano gits...@pobox.com Kopie (CC): git@vger.kernel.org, Jonathan Nieder jrnie...@gmail.com, Jeff King p...@peff.net, 434...@bugs.debian.org, René Scharfe l@web.de, Tony Finch d...@dotat.at, Tanay Abhra tanay...@gmail.com, Dan Albert danalb...@google.com, Jeremy Huddleston jerem...@apple.com, David Aguilar dav...@gmail.com, Michael Haggerty mhag...@alum.mit.edu, Oswald Buddenhagen o...@kde.org Sorry for not getting back to this any sooner, I've been pretty busy recently with Other Projects(tm). Am 2014-08-27 um 19:20 schrieb Junio C Hamano: Bernhard Reiter ock...@raz.or.at writes: [...] For now, the old ones are wrapped in #ifdefs, and the new functions are enabled by make if curl's version is = 7.35.0, from which version on curl's CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been available. https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions says that this was introduced as of 7.34.0, though. Strange, I thought I recalled having seen that in http://curl.haxx.se/libcurl/c/CURLOPT_LOGIN_OPTIONS.html but it clearly says 7.34.0 there too. I've now changed all occurrences of 7.35.0 to 7.34.0 (and the corresponding hex value in the Makefile). As I don't have access to that many IMAP servers, I haven't been able to test the new code with a wide variety of parameter combinations. I did test both secure and insecure (imaps:// and imap://) connections and values of PLAIN and LOGIN for the authMethod. Perhaps CC'ing those who have touched git-imap-send code over the years and asking for their help testing might help? CC'ing them (going back about 2 years, which already makes the list quite long) and the people who have taken part in the initial discussion on this feature in August. And the related Debian bug. Please test this, folks! Signed-off-by: Bernhard Reiter ock...@raz.or.at --- I rebased the patch on the pu branch, hope that was the right thing to do. Usually I would appreciate a patch for a new feature not meant for the maintenance tracks to be based on 'master', so that it can go to the next release without having to wait other changes that may conflict with it and that may not yet be ready. I will try to apply this one to 'pu', rebase it on 'master' to make sure the result does not depend on the other topics in flight, and then merge it back to 'pu'. Okay, I'll stick to master. I've rebased on master now that the first couple related patches are there anyway. [...] diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 7d991d9..9d244c4 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -75,7 +75,8 @@ imap.preformattedHTML:: imap.authMethod:: Specify authenticate method for authentication with IMAP server. -Current supported method is 'CRAM-MD5' only. If this is not set +If you compiled git with the NO_CURL option or if your curl version is + 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Hmph, so there is no option that lets me say I know my libcurl is new enough but I have some reason not to want to use the new code to interact with my imap server, at compile time or (more preferrably) at runtime? Added a runtime option, see below. diff --git a/INSTALL b/INSTALL index 6ec7a24..e2770a0 100644 --- a/INSTALL +++ b/INSTALL @@ -108,18 +108,21 @@ Issues of note: so you might need to install additional packages other than Perl itself, e.g. Time::HiRes. -- openssl library is used by git-imap-send to use IMAP over SSL. - If you don't need it, use NO_OPENSSL. +- openssl library is used by git-imap-send to use IMAP over SSL, + unless you're using curl = 7.35.0, in which case that will be + used. If you don't need git-imap-send, you can use NO_OPENSSL. The last sentence makes it unclear which of the following is true: - I have sufficiently new libcurl. I cannot say NO_OPENSSL because I do need git-imap-send. - I have sufficiently new libcurl, so openssl is not used by git-imap send for me. I can say NO_OPENSSL. Perhaps - git-imap-send needs the OpenSSL library to talk IMAP over SSL if you are using libCurl older than 7.35.0. Otherwise you can use NO_OPENSSL without losing git-imap-send. Fixed. diff --git a/git.spec.in b/git.spec.in index d61d537..9535cc3 100644 --- a/git.spec.in +++ b/git.spec.in @@ -8,7 +8,7 @@ License: GPL Group: Development/Tools URL
Re: [PATCH] git-imap-send: use libcurl for implementation
Sorry for not getting back to this any sooner, I've been pretty busy recently with Other Projects(tm). Am 2014-08-27 um 19:20 schrieb Junio C Hamano: Bernhard Reiter ock...@raz.or.at writes: [...] For now, the old ones are wrapped in #ifdefs, and the new functions are enabled by make if curl's version is = 7.35.0, from which version on curl's CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been available. https://github.com/bagder/curl/blob/master/docs/libcurl/symbols-in-versions says that this was introduced as of 7.34.0, though. Strange, I thought I recalled having seen that in http://curl.haxx.se/libcurl/c/CURLOPT_LOGIN_OPTIONS.html but it clearly says 7.34.0 there too. I've now changed all occurrences of 7.35.0 to 7.34.0 (and the corresponding hex value in the Makefile). As I don't have access to that many IMAP servers, I haven't been able to test the new code with a wide variety of parameter combinations. I did test both secure and insecure (imaps:// and imap://) connections and values of PLAIN and LOGIN for the authMethod. Perhaps CC'ing those who have touched git-imap-send code over the years and asking for their help testing might help? CC'ing them (going back about 2 years, which already makes the list quite long) and the people who have taken part in the initial discussion on this feature in August. And the related Debian bug. Please test this, folks! Signed-off-by: Bernhard Reiter ock...@raz.or.at --- I rebased the patch on the pu branch, hope that was the right thing to do. Usually I would appreciate a patch for a new feature not meant for the maintenance tracks to be based on 'master', so that it can go to the next release without having to wait other changes that may conflict with it and that may not yet be ready. I will try to apply this one to 'pu', rebase it on 'master' to make sure the result does not depend on the other topics in flight, and then merge it back to 'pu'. Okay, I'll stick to master. I've rebased on master now that the first couple related patches are there anyway. [...] diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 7d991d9..9d244c4 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -75,7 +75,8 @@ imap.preformattedHTML:: imap.authMethod:: Specify authenticate method for authentication with IMAP server. -Current supported method is 'CRAM-MD5' only. If this is not set +If you compiled git with the NO_CURL option or if your curl version is + 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Hmph, so there is no option that lets me say I know my libcurl is new enough but I have some reason not to want to use the new code to interact with my imap server, at compile time or (more preferrably) at runtime? Added a runtime option, see below. diff --git a/INSTALL b/INSTALL index 6ec7a24..e2770a0 100644 --- a/INSTALL +++ b/INSTALL @@ -108,18 +108,21 @@ Issues of note: so you might need to install additional packages other than Perl itself, e.g. Time::HiRes. -- openssl library is used by git-imap-send to use IMAP over SSL. - If you don't need it, use NO_OPENSSL. +- openssl library is used by git-imap-send to use IMAP over SSL, + unless you're using curl = 7.35.0, in which case that will be + used. If you don't need git-imap-send, you can use NO_OPENSSL. The last sentence makes it unclear which of the following is true: - I have sufficiently new libcurl. I cannot say NO_OPENSSL because I do need git-imap-send. - I have sufficiently new libcurl, so openssl is not used by git-imap send for me. I can say NO_OPENSSL. Perhaps - git-imap-send needs the OpenSSL library to talk IMAP over SSL if you are using libCurl older than 7.35.0. Otherwise you can use NO_OPENSSL without losing git-imap-send. Fixed. diff --git a/git.spec.in b/git.spec.in index d61d537..9535cc3 100644 --- a/git.spec.in +++ b/git.spec.in @@ -8,7 +8,7 @@ License: GPL Group: Development/Tools URL:http://kernel.org/pub/software/scm/git/ Source: http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz -BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3} +BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel = 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3} This is very iffy. It incompatible with the body of the patch where you allow older curl library and because you depend on openssl-devel you wouldn't lose imap-send. Okay, removed the version requirement. @@ -1391,29 +1518,13 @@ int main(int argc, char **argv) [...] Much more nicely done. It appears that you could already turn
[PATCH] git-imap-send: use libcurl for implementation
Use libcurl's high-level API functions to implement git-imap-send instead of the previous low-level OpenSSL-based functions. Since version 7.30.0, libcurl's API has been able to communicate with IMAP servers. Using those high-level functions instead of the current ones would reduce imap-send.c by some 1200 lines of code. For now, the old ones are wrapped in #ifdefs, and the new functions are enabled by make if curl's version is = 7.35.0, from which version on curl's CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been available. As I don't have access to that many IMAP servers, I haven't been able to test the new code with a wide variety of parameter combinations. I did test both secure and insecure (imaps:// and imap://) connections and values of PLAIN and LOGIN for the authMethod. Signed-off-by: Bernhard Reiter ock...@raz.or.at --- I rebased the patch on the pu branch, hope that was the right thing to do. Bernhard Documentation/git-imap-send.txt | 3 +- INSTALL | 15 ++-- Makefile| 16 +++- git.spec.in | 5 +- imap-send.c | 165 +--- 5 files changed, 167 insertions(+), 37 deletions(-) diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 7d991d9..9d244c4 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -75,7 +75,8 @@ imap.preformattedHTML:: imap.authMethod:: Specify authenticate method for authentication with IMAP server. - Current supported method is 'CRAM-MD5' only. If this is not set + If you compiled git with the NO_CURL option or if your curl version is +7.35.0, the only supported method is 'CRAM-MD5'. If this is not set then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Examples diff --git a/INSTALL b/INSTALL index 6ec7a24..e2770a0 100644 --- a/INSTALL +++ b/INSTALL @@ -108,18 +108,21 @@ Issues of note: so you might need to install additional packages other than Perl itself, e.g. Time::HiRes. - - openssl library is used by git-imap-send to use IMAP over SSL. - If you don't need it, use NO_OPENSSL. + - openssl library is used by git-imap-send to use IMAP over SSL, + unless you're using curl = 7.35.0, in which case that will be + used. If you don't need git-imap-send, you can use NO_OPENSSL. By default, git uses OpenSSL for SHA1 but it will use its own library (inspired by Mozilla's) with either NO_OPENSSL or BLK_SHA1. Also included is a version optimized for PowerPC (PPC_SHA1). - - libcurl library is used by git-http-fetch and git-fetch. You - might also want the curl executable for debugging purposes. - If you do not use http:// or https:// repositories, you do not - have to have them (use NO_CURL). + - libcurl library is used by git-http-fetch, git-fetch, and, if + the curl version = 7.35.0, for git-imap-send. You might also + want the curl executable for debugging purposes. If you do not + use http:// or https:// repositories, and do not want to put + patches into an IMAP mailbox, you do not have to have them + (use NO_CURL). - expat library; git-http-push uses it for remote lock management over DAV. Similar to curl above, this is optional diff --git a/Makefile b/Makefile index 237bc05..c08963c 100644 --- a/Makefile +++ b/Makefile @@ -1133,6 +1133,9 @@ ifdef HAVE_ALLOCA_H BASIC_CFLAGS += -DHAVE_ALLOCA_H endif +IMAP_SEND_BUILDDEPS = +IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) + ifdef NO_CURL BASIC_CFLAGS += -DNO_CURL REMOTE_CURL_PRIMARY = @@ -1167,6 +1170,15 @@ else PROGRAM_OBJS += http-push.o endif endif + curl_check := $(shell (echo 072300; curl-config --vernum) 2/dev/null | sort -r | sed -ne 2p) + ifeq $(curl_check) 072300 + USE_CURL_FOR_IMAP_SEND = YesPlease + endif + ifdef USE_CURL_FOR_IMAP_SEND + BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND + IMAP_SEND_BUILDDEPS = http.o + IMAP_SEND_LDFLAGS += $(CURL_LIBCURL) + endif ifndef NO_EXPAT ifdef EXPATDIR BASIC_CFLAGS += -I$(EXPATDIR)/include @@ -2084,9 +2096,9 @@ endif git-%$X: %.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) -git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS) +git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) + $(LIBS) $(IMAP_SEND_LDFLAGS) git-http-fetch$X: http.o
Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
| 16 ++- git.spec.in | 5 ++- imap-send.c | 96 + 5 files changed, 125 insertions(+), 10 deletions(-) diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 7d991d9..9d244c4 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -75,7 +75,8 @@ imap.preformattedHTML:: imap.authMethod:: Specify authenticate method for authentication with IMAP server. - Current supported method is 'CRAM-MD5' only. If this is not set + If you compiled git with the NO_CURL option or if your curl version is +7.35.0, the only supported method is 'CRAM-MD5'. If this is not set then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Examples diff --git a/INSTALL b/INSTALL index 6ec7a24..e2770a0 100644 --- a/INSTALL +++ b/INSTALL @@ -108,18 +108,21 @@ Issues of note: so you might need to install additional packages other than Perl itself, e.g. Time::HiRes. - - openssl library is used by git-imap-send to use IMAP over SSL. - If you don't need it, use NO_OPENSSL. + - openssl library is used by git-imap-send to use IMAP over SSL, + unless you're using curl = 7.35.0, in which case that will be + used. If you don't need git-imap-send, you can use NO_OPENSSL. By default, git uses OpenSSL for SHA1 but it will use its own library (inspired by Mozilla's) with either NO_OPENSSL or BLK_SHA1. Also included is a version optimized for PowerPC (PPC_SHA1). - - libcurl library is used by git-http-fetch and git-fetch. You - might also want the curl executable for debugging purposes. - If you do not use http:// or https:// repositories, you do not - have to have them (use NO_CURL). + - libcurl library is used by git-http-fetch, git-fetch, and, if + the curl version = 7.35.0, for git-imap-send. You might also + want the curl executable for debugging purposes. If you do not + use http:// or https:// repositories, and do not want to put + patches into an IMAP mailbox, you do not have to have them + (use NO_CURL). - expat library; git-http-push uses it for remote lock management over DAV. Similar to curl above, this is optional diff --git a/Makefile b/Makefile index 237bc05..c08963c 100644 --- a/Makefile +++ b/Makefile @@ -1133,6 +1133,9 @@ ifdef HAVE_ALLOCA_H BASIC_CFLAGS += -DHAVE_ALLOCA_H endif +IMAP_SEND_BUILDDEPS = +IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) + ifdef NO_CURL BASIC_CFLAGS += -DNO_CURL REMOTE_CURL_PRIMARY = @@ -1167,6 +1170,15 @@ else PROGRAM_OBJS += http-push.o endif endif + curl_check := $(shell (echo 072300; curl-config --vernum) 2/dev/null | sort -r | sed -ne 2p) + ifeq $(curl_check) 072300 + USE_CURL_FOR_IMAP_SEND = YesPlease + endif + ifdef USE_CURL_FOR_IMAP_SEND + BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND + IMAP_SEND_BUILDDEPS = http.o + IMAP_SEND_LDFLAGS += $(CURL_LIBCURL) + endif ifndef NO_EXPAT ifdef EXPATDIR BASIC_CFLAGS += -I$(EXPATDIR)/include @@ -2084,9 +2096,9 @@ endif git-%$X: %.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) -git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS) +git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) + $(LIBS) $(IMAP_SEND_LDFLAGS) git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ diff --git a/git.spec.in b/git.spec.in index d61d537..9535cc3 100644 --- a/git.spec.in +++ b/git.spec.in @@ -8,7 +8,7 @@ License:GPL Group: Development/Tools URL: http://kernel.org/pub/software/scm/git/ Source:http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz -BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3} +BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel = 7.35.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3} BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) Requires: perl-Git = %{version}-%{release} @@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT # No files for you! %changelog +* Mon Aug 11 2014 Bernhard Reiter ock...@raz.or.at +- Require version = 7.35.0 of curl-devel for IMAP functions. + * Sun Sep 18 2011 Jakub
Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
Am 2014-08-17 um 20:42 schrieb Jeff King: [...] I'm not sure I understand this comment. Even if SSL is not in use, wouldn't we be passing a regular pipe to curl, which would break? Yeah, we can't do that, and thus would have to keep the handwritten IMAP implementation just for the tunnel case (allowing to drop only the OpenSSL specific stuff), see my other email: http://www.mail-archive.com/git@vger.kernel.org/msg56791.html (the relevant part is pretty far down at the bottom). I'd really love it if we could make this work with tunnels and eventually get rid of the hand-written imap code entirely. I agree with Jonathan that we probably need to keep it around a bit for people on older curl, but dropping it is a good goal in the long run. That code was forked from the isync project, but mangled enough that we could not take bug fixes from upstream. As not many people use imap-send, I suspect it is largely unmaintained and the source of many lurking bugs[1]. Replacing it with curl's maintained implementation is probably a good step. I'll work on this as soon as I find some time, but as that will include changes to run-command.c (and possibly other files?), I'd like to cover that in a commit of its own. Do you guys think the current patch [1] is good enough for official submission already? If so, do I need some sort of official review? Documentation/SubmittingPatches says I'm only supposed to direct it to Junio after the list reaches consensus, so I'm wondering how to get there... :-) Bernhard -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] imap-send.c: imap_folder - imap_server_conf.folder
Rename the imap_folder variable to folder and make it a member of struct imap_server_conf. Signed-off-by: Bernhard Reiter ock...@raz.or.at --- As discussed in http://www.mail-archive.com/git@vger.kernel.org/msg57019.html Bernhard imap-send.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/imap-send.c b/imap-send.c index fb01a9c..05a02b5 100644 --- a/imap-send.c +++ b/imap-send.c @@ -69,6 +69,7 @@ struct imap_server_conf { char *tunnel; char *host; int port; + char *folder; char *user; char *pass; int use_ssl; @@ -82,6 +83,7 @@ static struct imap_server_conf server = { NULL, /* tunnel */ NULL, /* host */ 0, /* port */ + NULL, /* folder */ NULL, /* user */ NULL, /* pass */ 0, /* use_ssl */ @@ -1323,8 +1325,6 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs) return 1; } -static char *imap_folder; - static int git_imap_config(const char *key, const char *val, void *cb) { if (!skip_prefix(key, imap., key)) @@ -1339,7 +1339,7 @@ static int git_imap_config(const char *key, const char *val, void *cb) return config_error_nonbool(key); if (!strcmp(folder, key)) { - imap_folder = xstrdup(val); + server.folder = xstrdup(val); } else if (!strcmp(host, key)) { if (starts_with(val, imap:)) val += 5; @@ -1387,7 +1387,7 @@ int main(int argc, char **argv) if (!server.port) server.port = server.use_ssl ? 993 : 143; - if (!imap_folder) { + if (!server.folder) { fprintf(stderr, no imap store specified\n); return 1; } @@ -1424,7 +1424,7 @@ int main(int argc, char **argv) } fprintf(stderr, sending %d message%s\n, total, (total != 1) ? s : ); - ctx-name = imap_folder; + ctx-name = server.folder; while (1) { unsigned percent = n * 100 / total; -- 2.1.0.3.g63c96dd -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-imap-send: simplify tunnel construction
Am 2014-08-18 um 19:00 schrieb Junio C Hamano: Bernhard Reiter ock...@raz.or.at writes: Signed-off-by: Bernhard Reiter ock...@raz.or.at --- imap-send.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Oy. Where is the patch? Please avoid multipart/mixed on this list. Thanks. D'oh. Sorry about that. Strangely, that's what I'm getting from a message created with git-imap-send. Maybe Thunderbird is messing it up afterwards. Anyway: diff --git a/imap-send.c b/imap-send.c index 524fbab..fb01a9c 100644 --- a/imap-send.c +++ b/imap-send.c @@ -961,17 +961,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc) /* open connection to IMAP server */ if (srvc-tunnel) { - const char *argv[] = { srvc-tunnel, NULL }; struct child_process tunnel = {NULL}; imap_info(Starting tunnel '%s'... , srvc-tunnel); - tunnel.argv = argv; + argv_array_push(tunnel.args, srvc-tunnel); tunnel.use_shell = 1; tunnel.in = -1; tunnel.out = -1; if (start_command(tunnel)) - die(cannot start proxy %s, argv[0]); + die(cannot start proxy %s, srvc-tunnel); imap-buf.sock.fd[0] = tunnel.out; imap-buf.sock.fd[1] = tunnel.in; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
Am 2014-08-17 um 10:30 schrieb Jeff King: On Tue, Aug 12, 2014 at 06:59:17PM -0700, Jonathan Nieder wrote: + curl_socket_t sockfd = tunnel.out; // what about tunnel.in ? Hmm. curl expects to get a socket it can send(), recv(), setsockopt(), etc on instead of a pair of fds to read() and write(). I wonder if we could teach run_command to optionally use socketpair() instead of pipe(). That sounds like a good idea to me. I'm not sure if that would cause problems on Windows, though. Apparently socketpair is not available there. Googling socketpair windows yields, among a lot of other useful resources, the following relatively actively maintained ~150 LOC, BSD-3-clause-licensed, implementation: https://github.com/ncm/selectable-socketpair That license is GPL compatible, so should we consider including that implementation with git? That's the kind of stuff that goes to compat/win32, right? One thing to consider: seems like socketpair() gives AF_LOCAL sockets, so I've asked [1] on the curl ML if that would work or if libcurl needs an AF_INET one. I wonder why someone would want to use SSL through a tunnel, though. Currently it's impossible to get to the SSL codepath when a tunnel is active (it's in the 'else' block an 'if (srvc-tunnel)'). If that property is preserved, then we should be safe. I'm not sure I understand this comment. Even if SSL is not in use, wouldn't we be passing a regular pipe to curl, which would break? Yeah, we can't do that, and thus would have to keep the handwritten IMAP implementation just for the tunnel case (allowing to drop only the OpenSSL specific stuff), see my other email: http://www.mail-archive.com/git@vger.kernel.org/msg56791.html (the relevant part is pretty far down at the bottom). Bernhard [1] http://curl.haxx.se/mail/lib-2014-08/0131.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
Use libcurl's high-level API functions to implement git-imap-send instead of the previous low-level OpenSSL-based functions. Since version 7.30.0, libcurl's API has been able to communicate with IMAP servers. Using those high-level functions instead of the current ones would reduce imap-send.c by some 1200 lines of code. For now, the old ones are wrapped in #ifdefs, and the new functions are enabled by make if curl's version is = 7.35.0, from which version on curl's CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been available. As I don't have access to that many IMAP servers, I haven't been able to test the new code with a wide variety of parameter combinations. I did test both secure and insecure (imaps:// and imap://) connections and values of PLAIN and LOGIN for the authMethod. Signed-off-by: Bernhard Reiter ock...@raz.or.at --- Am 2014-08-13 um 03:59 schrieb Jonathan Nieder: Bernhard Reiter wrote: [...] Wow! This sounds lovely. Thanks for working on this. Well thanks for the friendly welcome and the helpful comments! I'm attaching a patch where I've applied the fixes you suggested, plus: * I added the lf_to_crlf conversion to the curl codepath as communication with another IMAP server I tried was broken without it. * I added STARTTLS. (That's just the curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL); line) * I tested (and fixed) authentication, i.e. the auth_method stuff. As the corresponding CURLOPT_LOGIN_OPTIONS flag has only been available starting with curl 7.35.0, I've bumped the required version to that. (Apparently it was possible to achieve the same effect with a different option in between versions 7.31.0 and 7.34.0 [1], but I haven't found yet how. Is it worth the effort?) * I made that file scope imap_folder a member of struct imap_server_conf (named folder), which makes some things easier. @@ -1417,31 +269,89 @@ int main(int argc, char **argv) return 1; } +curl_global_init(CURL_GLOBAL_ALL); http.c seems to make the same mistake, Patch at http://permalink.gmane.org/gmane.comp.version-control.git/255221 [...] +if (server.tunnel) { +const char *argv[] = { server.tunnel, NULL }; +struct child_process tunnel = {NULL}; (not about this patch) Could use the child_proccess's internal argv_array: struct child_process tunnel = {NULL}; argv_array_push(tunnel.args, server.tunnel); Patch at http://permalink.gmane.org/gmane.comp.version-control.git/255220 (The patch attached to this mail depends on that one.) No comments on those patches yet, though. (about this patch) Would there be a way to make this part reuse the existing code? The only difference I see is that *srvc has been renamed to server, which doesn't seem to be related to the change of transport API from OpenSSL to libcurl. [...] +curl_socket_t sockfd = tunnel.out; // what about tunnel.in ? Hmm. curl expects to get a socket it can send(), recv(), setsockopt(), etc on instead of a pair of fds to read() and write(). I wonder why someone would want to use SSL through a tunnel, though. Currently it's impossible to get to the SSL codepath when a tunnel is active (it's in the 'else' block an 'if (srvc-tunnel)'). If that property is preserved, then we should be safe. Now this turns out to be the one major annoyance left, because we only have those two fds (actually pipes, right?), and not a socket that we could pass to curl, so we can't use it to talk to the IMAP server. So if the tunnel parameter is set, we're stuck with the old hand-written IMAP handling routines, even with USE_CURL_FOR_IMAP set, meaning I can't wrap as much in #ifdef...#endif blocks as I'd like. :-( BTW, due to two of the blocks that I do add I get a compiler warning about the curl handle remaining possibly unitialized :-/ I've removed the curl specific socket handling routines, as we can't use them anyway for now. I've asked about passing two pipes instead of a socket to curl on their ML [1] as this has even been discussed before [2], but unfortunately, there doesn't seem to be a solution as of yet. I've also asked on SO [3], but no answers yet. To summarize: [...] * As soon as you're ready to roll this out to a wider audience of testers, let me know, and we can try to get it into shape for Junio's next branch (and hence Debian experimental). Is this one good enough already? Bernhard [1] http://sourceforge.net/p/curl/bugs/1372/ [2] http://curl.haxx.se/mail/lib-2014-08/0102.html [3] http://curl.haxx.se/mail/lib-2011-05/0102.html [4] http://stackoverflow.com/questions/25306264/connect-in-and-out-pipes-to-network-socket Documentation/git-imap-send.txt | 3 +- INSTALL | 15 +++--- Makefile| 16 +- git.spec.in | 5 +- imap-send.c | 109 +--- 5 files
[PATCH] git-imap-send: simplify tunnel construction
Signed-off-by: Bernhard Reiter ock...@raz.or.at --- imap-send.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/imap-send.c b/imap-send.c index 524fbab..fb01a9c 100644 --- a/imap-send.c +++ b/imap-send.c @@ -961,17 +961,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc) /* open connection to IMAP server */ if (srvc-tunnel) { - const char *argv[] = { srvc-tunnel, NULL }; struct child_process tunnel = {NULL}; imap_info(Starting tunnel '%s'... , srvc-tunnel); - tunnel.argv = argv; + argv_array_push(tunnel.args, srvc-tunnel); tunnel.use_shell = 1; tunnel.in = -1; tunnel.out = -1; if (start_command(tunnel)) - die(cannot start proxy %s, argv[0]); + die(cannot start proxy %s, srvc-tunnel); imap-buf.sock.fd[0] = tunnel.out; imap-buf.sock.fd[1] = tunnel.in;
[PATCH] http.c: die if curl_*_init fails
Signed-off-by: Bernhard Reiter ock...@raz.or.at --- http.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/http.c b/http.c index c8cd50d..afe4fc5 100644 --- a/http.c +++ b/http.c @@ -300,6 +300,9 @@ static CURL *get_curl_handle(void) { CURL *result = curl_easy_init(); + if (!result) + die(curl_easy_init failed); + if (!curl_ssl_verify) { curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 0); curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 0); @@ -399,7 +402,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) git_config(urlmatch_config_entry, config); free(normalized_url); - curl_global_init(CURL_GLOBAL_ALL); + if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) + die(curl_global_init failed); http_proactive_auth = proactive_auth;
[PATCH/RFC] git-imap-send: use libcurl for implementation
Use libcurl's high-level API functions to implement git-imap-send instead of the previous low-level OpenSSL-based functions. Signed-off-by: Bernhard Reiter ock...@raz.or.at --- Since version 7.30.0, libcurl's API has been able to communicate with IMAP servers. Using those high-level functions instead of the current ones reduces imap-send.c by some 1200 lines of code. As I don't have access to that many IMAP servers, I haven't been able to test a variety of parameter combinations. I did test both secure and insecure (imaps:// and imap://) connections -- this e-mail was generated that way -- but could e.g. neither test the authMethod nor the tunnel parameter. As git-imap-send is one of the two instances OpenSSL is currently used by git -- the other one being SHA1 -- it might be worthwhile considering dropping it altogether as there's already a SHA1 library built into git available as an alternative. Kind regards Bernhard PS: Please CC! INSTALL | 14 +- Makefile|4 +- git.spec.in |5 +- imap-send.c | 1288 +-- 4 files changed, 111 insertions(+), 1200 deletions(-) diff --git a/INSTALL b/INSTALL index 6ec7a24..2cd3a42 100644 --- a/INSTALL +++ b/INSTALL @@ -108,18 +108,16 @@ 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. - - By default, git uses OpenSSL for SHA1 but it will use its own + - By default, git uses OpenSSL for SHA1 but it will use its own library (inspired by Mozilla's) with either NO_OPENSSL or BLK_SHA1. Also included is a version optimized for PowerPC (PPC_SHA1). - - libcurl library is used by git-http-fetch and git-fetch. You - might also want the curl executable for debugging purposes. - If you do not use http:// or https:// repositories, you do not - have to have them (use NO_CURL). + - libcurl library is used by git-http-fetch, git-fetch, and + git-impap-send. You might also want the curl executable for + debugging purposes. If you do not use http:// or https:// + repositories, and do not want to put patches into an IMAP + mailbox, you do not have to have them (use NO_CURL). - expat library; git-http-push uses it for remote lock management over DAV. Similar to curl above, this is optional diff --git a/Makefile b/Makefile index 2320de5..7805603 100644 --- a/Makefile +++ b/Makefile @@ -2067,9 +2067,9 @@ endif git-%$X: %.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) -git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS) +git-imap-send$X: imap-send.o http.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) + $(LIBS) $(CURL_LIBCURL) git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ diff --git a/git.spec.in b/git.spec.in index d61d537..7d9230f 100644 --- a/git.spec.in +++ b/git.spec.in @@ -8,7 +8,7 @@ License: GPL Group: Development/Tools URL: http://kernel.org/pub/software/scm/git/ Source: http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz -BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3} +BuildRequires: zlib-devel = 1.2, openssl-devel, curl-devel = 7.30.0, expat-devel, gettext %{!?_without_docs:, xmlto, asciidoc 6.0.3} BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) Requires: perl-Git = %{version}-%{release} @@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT # No files for you! %changelog +* Mon Aug 11 2014 Bernhard Reiter ock...@raz.or.at +- Require version = 7.30.0 of curl-devel for IMAP functions. + * Sun Sep 18 2011 Jakub Narebski jna...@gmail.com - Add gitweb manpages to 'gitweb' subpackage diff --git a/imap-send.c b/imap-send.c index 524fbab..0c4583f 100644 --- a/imap-send.c +++ b/imap-send.c @@ -22,47 +22,13 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#include cache.h -#include credential.h +#include http.h #include exec_cmd.h #include run-command.h -#ifdef NO_OPENSSL -typedef void *SSL; -#endif -static const char imap_send_usage[] = git imap-send mbox; - -#undef DRV_OK -#define DRV_OK 0 -#define DRV_MSG_BAD -1 -#define DRV_BOX_BAD -2 -#define DRV_STORE_BAD -3 - -static int Verbose, Quiet; - -__attribute__((format (printf, 1, 2))) -static void imap_info(const char *, ...); -__attribute__((format (printf, 1, 2))) -static void imap_warn(const char *, ...); - -static char *next_arg(char **); - -__attribute__((format (printf, 3, 4))) -static int nfsnprintf(char *buf, int blen, const char *fmt, ...); +#include curl/curl.h