git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 3/6] imap-send: replace auto-probe libcurl with hard dependency

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 2, 2023, 19:33 UTC
Message-ID
<xmqqk00zuakx.fsf@gitster.g>
In-Reply-To
<patch-v2-3.6-354b6a65a78-20230202T093706Z-avarab@gmail.com>
Show 7 quoted lines
>  imap.authMethod::
>  	Specify authenticate method for authentication with IMAP server.
> -	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`
> +	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.
OK.
Show 11 quoted lines
> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
> index f7b18515141..202e3e59094 100644
> --- a/Documentation/git-imap-send.txt
> +++ b/Documentation/git-imap-send.txt
> @@ -44,8 +44,7 @@ OPTIONS
>  
>  --no-curl::
>  	Talk to the IMAP server using git's own IMAP routines instead of
> -	using libcurl.  Ignored if Git was built with the NO_OPENSSL option
> -	set.
> +	using libcurl.

Hmph, let's read on to resolve "when built with NO_OPENSSL, giving --no-curl now errors out or do something else? do we need to? why?", which was my knee-jerk reaction.

Show 14 quoted lines
> diff --git a/INSTALL b/INSTALL
> index d5694f8c470..d9538bbcb45 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -129,13 +129,13 @@ Issues of note:
>  	  itself, e.g. Digest::MD5, File::Spec, File::Temp, Net::Domain,
>  	  Net::SMTP, and Time::HiRes.
>  
> -	- 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.
> +	- git-imap-send needs libcurl 7.34.0 or newer, in addition
> +	  OpenSSL is needed if using the "imap.tunnel" open to tunnel
> +	  over SSL. Define NO_OPENSSL to omit the OpenSSL prerequisite.

"if using the imap.tunnel to open a tunnel over ssl"? because I think there are some grammo there.

"to omit the OpenSSL prerequisite" -> "if you do not need it"? because the original sounds like losing prereq without any penalty.

Show 6 quoted lines
>  	- "libcurl" library is used for fetching and pushing
>  	  repositories over http:// or https://, as well as by
> -	  git-imap-send if the curl version is >= 7.34.0. If you do
> +	  git-imap-send. If you do
>  	  not need that functionality, use NO_CURL to build without
>  	  it.
OK.
Show 11 quoted lines
> diff --git a/Makefile b/Makefile
> index 45bd6ac9c3e..b08a855198c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -773,7 +773,9 @@ PROGRAMS += $(EXTRA_PROGRAMS)
>  
>  PROGRAM_OBJS += daemon.o
>  PROGRAM_OBJS += http-backend.o
> +ifndef NO_CURL
>  PROGRAM_OBJS += imap-send.o
> +endif
Nice.
Show 5 quoted lines
> @@ -1592,6 +1593,7 @@ ifdef NO_CURL
>  	REMOTE_CURL_ALIASES =
>  	REMOTE_CURL_NAMES =
>  	EXCLUDED_PROGRAMS += git-http-fetch git-http-push
> +	EXCLUDED_PROGRAMS += git-imap-send
OK.
Show 5 quoted lines
> @@ -1617,19 +1619,9 @@ else
>  	REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
>  	PROGRAM_OBJS += http-fetch.o
>  	PROGRAMS += $(REMOTE_CURL_NAMES)
> +	IMAP_SEND_LDFLAGS += $(CURL_LIBCURL)

OK. That is a natural consequence of losing USE_CURL_FOR_IMAP_SEND conditional, which is good.

Show 6 quoted lines
> @@ -2786,7 +2778,7 @@ 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 $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
> +git-imap-send$X: imap-send.o http.o GIT-LDFLAGS $(GITLIBS)

And this too is a natural consequence of http.o that serves as a linkage between us and libcURL being always used. Good.

Show 7 quoted lines
> diff --git a/imap-send.c b/imap-send.c
> index b7902babd4c..26f8f01e97a 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -30,20 +30,10 @@
> ...
> +static int use_curl = 1;
OK.
Show 11 quoted lines
>  int cmd_main(int argc, const char **argv)
>  {
> @@ -1531,12 +1519,7 @@ int cmd_main(int argc, const char **argv)
>  	if (argc)
>  		usage_with_options(imap_send_usage, imap_send_options);
>  
> -#ifndef USE_CURL_FOR_IMAP_SEND
> -	if (use_curl) {
> -		warning("--curl not supported in this build");
> -		use_curl = 0;
> -	}
Naturally ;-)
Show 5 quoted lines
> -#elif defined(NO_OPENSSL)
> +#if defined(NO_OPENSSL)
>  	if (!use_curl) {
>  		warning("--no-curl not supported in this build");
>  		use_curl = 1;

In the original, this part reached iff we had USE_CURL_FOR_IMAP_SEND, so "if we are using curl and do not have openssl, then --no-curl is rejected and we forced use of curl" was how the original behaved here.

Here, we always link with curl, so the updated code is doing exactly the same thing. Good.

But then the documentation change above that puzzled me was there not because it was needed to match updated behaviour (the behaviour stayed the same). So was it meant as a documentation improvement?

Show 8 quoted lines
> @@ -1580,10 +1563,8 @@ int cmd_main(int argc, const char **argv)
>  	if (server.tunnel)
>  		return append_msgs_to_imap(&server, &all_msgs, total);
>  
> -#ifdef USE_CURL_FOR_IMAP_SEND
>  	if (use_curl)
>  		return curl_append_msgs_to_imap(&server, &all_msgs, total);
> -#endif
Naturally.
>  	return append_msgs_to_imap(&server, &all_msgs, total);
>  }
Looking good.  Thanks.
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 14 of 37 in “Makefile: not use mismatched curl_config to check version”
  1. 1/2 Makefile: not use mismatched curl_config to check versionJiang Xin, Feb 1, 2023
  2. 2/2 imap-send: not define USE_CURL_FOR_IMAP_SEND in MakefileJiang Xin, Feb 1, 2023
  3. imap-send: replace auto-probe libcurl with hard dependencyÆvar Arnfjörð Bjarmason, Feb 1, 2023
  4. Junio C HamanoFeb 1, 2023
  5. Jeff KingFeb 1, 2023
  6. Ævar Arnfjörð BjarmasonFeb 2, 2023
  7. Ævar Arnfjörð BjarmasonFeb 1, 2023
  8. Junio C HamanoFeb 2, 2023
  9. 0/6 imap-send: replace auto-probe libcurl with hard dependencyÆvar Arnfjörð Bjarmason, Feb 2, 2023
  10. 1/6 imap-send: note "auth_method", not "host" on auth method failureÆvar Arnfjörð Bjarmason, Feb 2, 2023
  11. Junio C HamanoFeb 2, 2023
  12. 2/6 imap-send doc: the imap.sslVerify is used with imap.tunnelÆvar Arnfjörð Bjarmason, Feb 2, 2023
  13. 3/6 imap-send: replace auto-probe libcurl with hard dependencyÆvar Arnfjörð Bjarmason, Feb 2, 2023
  14. Junio C HamanoFeb 2, 2023
  15. 4/6 imap-send: make --curl no-optionalÆvar Arnfjörð Bjarmason, Feb 2, 2023
  16. Junio C HamanoFeb 2, 2023
  17. Ævar Arnfjörð BjarmasonFeb 3, 2023
  18. Junio C HamanoFeb 4, 2023
  19. 6/6 imap-send: correctly report "host" when using "tunnel"Ævar Arnfjörð Bjarmason, Feb 2, 2023
  20. Junio C HamanoFeb 2, 2023
  21. Jeff KingFeb 3, 2023
  22. Ævar Arnfjörð BjarmasonFeb 3, 2023
  23. Jeff KingFeb 4, 2023
  24. Ævar Arnfjörð BjarmasonFeb 5, 2023
  25. Jeff KingFeb 7, 2023
  26. Ævar Arnfjörð BjarmasonFeb 7, 2023
  27. Junio C HamanoFeb 7, 2023
  28. Ævar Arnfjörð BjarmasonFeb 7, 2023
  29. Jeff KingFeb 7, 2023
  30. Jeff KingFeb 7, 2023
  31. Junio C HamanoFeb 7, 2023
  32. Ævar Arnfjörð BjarmasonFeb 8, 2023
  33. Jeff KingFeb 17, 2023
  34. Junio C HamanoFeb 6, 2023
  35. 5/6 imap-send: remove old --no-curl codepathÆvar Arnfjörð Bjarmason, Feb 2, 2023
  36. Junio C HamanoFeb 2, 2023
  37. Junio C HamanoFeb 1, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.