From: Junio C Hamano <gitster@pobox.com>
To: Toon Claes <toon@iotcl.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 3/4] remote-curl: optionally show progress for HTTP get
Date: Wed, 08 May 2024 15:29:20 -0700 [thread overview]
Message-ID: <xmqq5xvox7i7.fsf@gitster.g> (raw)
In-Reply-To: <20240508124453.600871-4-toon@iotcl.com> (Toon Claes's message of "Wed, 8 May 2024 14:44:52 +0200")
Toon Claes <toon@iotcl.com> writes:
> git-remote-curl supports the `option progress` basically since it's
"it's" -> "its".
> inception. But this option had no effect for regular HTTP(S) downloads.
>
> Add progress indicator when downloading files through curl HTTP GET.
>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> remote-curl.c | 8 +++++++-
> t/t5557-http-get.sh | 15 +++++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index 0b6d7815fd..9fc7c3580c 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -1293,6 +1293,7 @@ static void parse_get(const char *arg)
> {
> struct strbuf url = STRBUF_INIT;
> struct strbuf path = STRBUF_INIT;
> + struct http_get_options http_options = {0};
> const char *space;
>
> space = strchr(arg, ' ');
> @@ -1303,7 +1304,12 @@ static void parse_get(const char *arg)
> strbuf_add(&url, arg, space - arg);
> strbuf_addstr(&path, space + 1);
>
> - if (http_get_file(url.buf, path.buf, NULL))
> + http_options.initial_request = 1;
> +
> + if (options.progress)
> + http_options.progress = 1;
> +
> + if (http_get_file(url.buf, path.buf, &http_options))
> die(_("failed to download file at URL '%s'"), url.buf);
>
> strbuf_release(&url);
> diff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh
> index 76a4bbd16a..92a138caaf 100755
> --- a/t/t5557-http-get.sh
> +++ b/t/t5557-http-get.sh
> @@ -36,4 +36,19 @@ test_expect_success 'get by URL: 200' '
> test_cmp "$HTTPD_DOCUMENT_ROOT_PATH/exists.txt" file2
> '
>
> +test_expect_success 'get by URL with progress' '
> + echo hello >"$HTTPD_DOCUMENT_ROOT_PATH/hello.txt" &&
> +
> + url="$HTTPD_URL/hello.txt" &&
> + cat >input <<-EOF &&
> + capabilities
> + option progress true
> + get $url file3
> +
> + EOF
> +
> + git remote-http $url <input 2>err &&
> + test_grep "^Downloading via HTTP: 100%" err
> +'
Are we sure that by the time we finish the transfer we know the
total? Does this rely on the fact that somehow we are in
control of the server and can force it to use content-length:
(instead of chunked encoding without any explicit indication of the
total length)?
Also, I am curious if we know how often the progress indicator is
updated by cURL library with the API used in [2/4] (and if we can
control the frequency if we wanted to). Note that this is me just
being curious, not a request to make it tweakable. The findings may
help polish the [2/4], though.
Thanks.
> +
> test_done
next prev parent reply other threads:[~2024-05-08 22:29 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
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 [this message]
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=xmqq5xvox7i7.fsf@gitster.g \
--to=gitster@pobox.com \
--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).