From: Jeff King <peff@peff.net>
To: Toon Claes <toon@iotcl.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/4] http: add the ability to log progress
Date: Thu, 9 May 2024 12:52:12 -0400 [thread overview]
Message-ID: <20240509165212.GC1708095@coredump.intra.peff.net> (raw)
In-Reply-To: <20240508124453.600871-3-toon@iotcl.com>
On Wed, May 08, 2024 at 02:44:51PM +0200, Toon Claes wrote:
> @@ -2017,6 +2021,21 @@ static void http_opt_request_remainder(CURL *curl, off_t pos)
> #define HTTP_REQUEST_STRBUF 0
> #define HTTP_REQUEST_FILE 1
>
> +static int http_progress_callback(void *clientp, curl_off_t dltotal,
> + curl_off_t dlnow, curl_off_t ultotal,
> + curl_off_t ulnow)
> +{
> + struct progress *progress = clientp;
> +
> + if (progress) {
> + progress_set_total(progress, dltotal);
> + display_progress(progress, dlnow);
> + display_throughput(progress, dlnow);
> + }
> +
> + return 0;
> +}
A very long time ago I implemented something similar (for a
proto-bundle-uri feature), and I found out the hard way that both curl
and our progress code may rely on SIGALRM (curl may use it to put a
timeout on blocking calls that don't support select, like DNS
resolution). Making things even more confusing, it's platform dependent,
since on modern platforms it spawns a separate resolving thread to avoid
blocking the main one.
So it seems to work OK for me on Linux without anything further, but we
may want this to be extra careful:
-- >8 --
Date: Thu, 10 Nov 2011 01:29:55 -0500
Subject: [PATCH] http: turn off curl signals
Curl sets and clears the handler for SIGALRM, which makes it
incompatible with git's progress code. However, we can ask
curl not to do this.
Signed-off-by: Jeff King <peff@peff.net>
---
http.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/http.c b/http.c
index 752c879c1f..e7986e1353 100644
--- a/http.c
+++ b/http.c
@@ -1230,6 +1230,8 @@ static CURL *get_curl_handle(void)
set_curl_keepalive(result);
+ curl_easy_setopt(result, CURLOPT_NOSIGNAL, 1);
+
return result;
}
-- 8< --
In the other part of the thread I suggested the possibility of having
remote-https shuttle machine-readable progress back to the caller. That
would also solve this issue, because the actual display_progress() calls
would happen in the parent.
-Peff
next prev parent reply other threads:[~2024-05-09 16:52 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 12:44 [PATCH 0/4] bundle-uri: show progress when downloading from bundle URIs Toon Claes
2024-05-08 12:44 ` [PATCH 1/4] progress: add function to set total Toon Claes
2024-05-08 12:44 ` [PATCH 2/4] http: add the ability to log progress Toon Claes
2024-05-08 16:52 ` Eric Sunshine
2024-05-09 16:34 ` Jeff King
2024-05-09 16:51 ` Junio C Hamano
2024-05-09 17:04 ` Jeff King
2024-05-09 16:52 ` Jeff King [this message]
2024-05-08 12:44 ` [PATCH 3/4] remote-curl: optionally show progress for HTTP get Toon Claes
2024-05-08 22:29 ` Junio C Hamano
2024-05-08 12:44 ` [PATCH 4/4] bundle-uri: enable git-remote-https progress Toon Claes
2024-05-09 16:46 ` Jeff King
2025-02-14 11:26 ` Toon Claes
2025-02-21 7:36 ` Jeff King
2024-05-08 23:49 ` [PATCH 0/4] bundle-uri: show progress when downloading from bundle URIs Junio C Hamano
2025-02-19 14:30 ` [PATCH v2 0/7] Show " Toon Claes
2025-02-19 14:30 ` [PATCH v2 1/7] progress: add function to set total Toon Claes
2025-02-21 7:43 ` Jeff King
2025-02-19 14:30 ` [PATCH v2 2/7] progress: allow pure-throughput progress meters Toon Claes
2025-02-19 14:30 ` [PATCH v2 3/7] http: turn off curl signals Toon Claes
2025-02-19 14:30 ` [PATCH v2 4/7] http: add the ability to log progress Toon Claes
2025-02-19 14:30 ` [PATCH v2 5/7] remote-curl: optionally show progress for HTTP get Toon Claes
2025-02-19 14:30 ` [PATCH v2 6/7] bundle-uri: enable git-remote-https progress Toon Claes
2025-02-19 14:30 ` [PATCH v2 7/7] http: silence stderr when progress is enabled Toon Claes
2025-02-21 7:48 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240509165212.GC1708095@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=toon@iotcl.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).