Re: [PATCH 3/4] remote-curl: optionally show progress for HTTP get

From: Junio C Hamano <gitster@pobox.com>
Date: May 8, 2024, 22:29 UTC
Message-ID: <xmqq5xvox7i7.fsf@gitster.g>
In-Reply-To: <20240508124453.600871-4-toon@iotcl.com>

Toon Claes <toon@iotcl.com> writes:

> git-remote-curl supports the `option progress` basically since it's

"it's" -> "its".

Show 58 quoted lines

> 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