{"thread":{"id":"61434","subject":"[PATCH 0/4] bundle-uri: show progress when downloading from bundle URIs","startedAt":"2024-05-08T12:45:07Z","lastAt":"2025-02-21T07:48:56Z","messageCount":25,"participants":["Toon Claes","Eric Sunshine","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"494314","messageId":"20240508124453.600871-1-toon@iotcl.com","threadId":"61434","inReplyTo":null,"subject":"[PATCH 0/4] bundle-uri: show progress when downloading from bundle URIs","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-05-08T12:44:49Z","receivedAt":"2024-05-08T12:45:07Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When a user clones a repository, they see what's happening in the\nmessages like \"Enumerating objects\" and \"Receiving objects\". But when\na user clones a repository that uses bundle URIs they see:\n\n Cloning into 'repo.git'\n\nAnd then they have to wait until all bundles are downloaded before they\nsee any other message. When the bundles are large, this can take a lot\nof time and the user might consider the process hangs and they kill it.\n\nThis patch series introduces progress displayed to the user while\nbundles are downloaded. The full output of a clone using bundle URIs\nwill look something like:\n\n Cloning into 'repo.git'...\n Downloading via HTTP: 21% (351812809/1620086598), 315.34 MiB | 49.84 MiB/s\n Downloading via HTTP: 77% (1247493865/1620086598), 1.15 GiB | 34.31 MiB/s\n Downloading via HTTP: 100% (1620086598/1620086598), 1.50 GiB | 37.97 MiB/s, done.\n remote: Enumerating objects: 1322255, done.\n remote: Counting objects: 100% (611708/611708), done.\n remote: Total 1322255 (delta 611708), reused 611708 (delta 611708), pack-reused 710547\n Receiving objects: 100% (1322255/1322255), 539.66 MiB | 31.57 MiB/s, done.\n etc...\n\nToon Claes (4):\n progress: add function to set total\n http: add the ability to log progress\n remote-curl: optionally show progress for HTTP get\n bundle-uri: enable git-remote-https progress\n\n bundle-uri.c | 4 +++-\n http.c | 32 ++++++++++++++++++++++++++++++++\n http.h | 5 +++++\n progress.c | 6 ++++++\n progress.h | 1 +\n remote-curl.c | 8 +++++++-\n t/helper/test-progress.c | 5 +++++\n t/t0500-progress-display.sh | 24 ++++++++++++++++++++++++\n t/t5557-http-get.sh | 15 +++++++++++++++\n 9 files changed, 98 insertions(+), 2 deletions(-)\n\n--\n2.44.0\n"},{"id":"494315","messageId":"20240508124453.600871-2-toon@iotcl.com","threadId":"61434","inReplyTo":"20240508124453.600871-1-toon@iotcl.com","subject":"[PATCH 1/4] progress: add function to set total","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-05-08T12:44:50Z","receivedAt":"2024-05-08T12:45:09Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"We're about to add the use of progress through curl. Although, curl\ndoesn't know the total at the start of the download, but might receive\nthis information in the Content-Length header when the download starts.\n\nTo allow users set the total size after calling start_progress(), add a\nfunction progress_set_total().\n\nSigned-off-by: Toon Claes \n---\n progress.c | 6 ++++++\n progress.h | 1 +\n t/helper/test-progress.c | 5 +++++\n t/t0500-progress-display.sh | 24 ++++++++++++++++++++++++\n 4 files changed, 36 insertions(+)\n\ndiff --git a/progress.c b/progress.c\nindex c83cb60bf1..494b26eb20 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -271,6 +271,12 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n \treturn progress;\n }\n \n+void progress_set_total(struct progress *progress, uint64_t total)\n+{\n+\tif (progress)\n+\t\tprogress->total = total;\n+}\n+\n static int get_default_delay(void)\n {\n \tstatic int delay_in_secs = -1;\ndiff --git a/progress.h b/progress.h\nindex 3a945637c8..52f75ab1bf 100644\n--- a/progress.h\n+++ b/progress.h\n@@ -14,6 +14,7 @@ void progress_test_force_update(void);\n \n void display_throughput(struct progress *progress, uint64_t total);\n void display_progress(struct progress *progress, uint64_t n);\n+void progress_set_total(struct progress *progress, uint64_t total);\n struct progress *start_progress(const char *title, uint64_t total);\n struct progress *start_sparse_progress(const char *title, uint64_t total);\n struct progress *start_delayed_progress(const char *title, uint64_t total);\ndiff --git a/t/helper/test-progress.c b/t/helper/test-progress.c\nindex 66acb6a06c..622b1f738d 100644\n--- a/t/helper/test-progress.c\n+++ b/t/helper/test-progress.c\n@@ -70,6 +70,11 @@ int cmd__progress(int argc, const char **argv)\n \t\t\tif (*end != '\\0')\n \t\t\t\tdie(\"invalid input: '%s'\\n\", line.buf);\n \t\t\tdisplay_progress(progress, item_count);\n+\t\t} else if (skip_prefix(line.buf, \"total \", (const char **) &end)) {\n+\t\t\tuint64_t total = strtoull(end, &end, 10);\n+\t\t\tif (*end != '\\0')\n+\t\t\t\tdie(\"invalid input: '%s'\\n\", line.buf);\n+\t\t\tprogress_set_total(progress, total);\n \t\t} else if (skip_prefix(line.buf, \"throughput \",\n \t\t\t\t (const char **) &end)) {\n \t\t\tuint64_t byte_count, test_ms;\ndiff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh\nindex 1eb3a8306b..82a3b834a6 100755\n--- a/t/t0500-progress-display.sh\n+++ b/t/t0500-progress-display.sh\n@@ -56,6 +56,30 @@ test_expect_success 'progress display with total' '\n \ttest_cmp expect out\n '\n \n+test_expect_success 'progress display modify total' '\n+\tcat >expect <<-\\EOF &&\n+\tWorking hard: 1\n+\tWorking hard: 66% (2/3)\n+\tWorking hard: 100% (3/3)\n+\tWorking hard: 100% (3/3), done.\n+\tEOF\n+\n+\tcat >in <<-\\EOF &&\n+\tstart 0\n+\tupdate\n+\tprogress 1\n+\tupdate\n+\ttotal 3\n+\tprogress 2\n+\tprogress 3\n+\tstop\n+\tEOF\n+\ttest-tool progress stderr &&\n+\n+\tshow_cr out &&\n+\ttest_cmp expect out\n+'\n+\n test_expect_success 'progress display breaks long lines #1' '\n \tsed -e \"s/Z$//\" >expect <<\\EOF &&\n Working hard.......2.........3.........4.........5.........6: 0% (100/100000)\n-- \n2.44.0.651.g21306a098c\n\n"},{"id":"494316","messageId":"20240508124453.600871-3-toon@iotcl.com","threadId":"61434","inReplyTo":"20240508124453.600871-1-toon@iotcl.com","subject":"[PATCH 2/4] http: add the ability to log progress","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-05-08T12:44:51Z","receivedAt":"2024-05-08T12:45:11Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Add an option `progress` to `struct http_get_options` to allow the\ncaller to enable download progress using the progress.c API.\n\nSigned-off-by: Toon Claes \n---\n http.c | 32 ++++++++++++++++++++++++++++++++\n http.h | 5 +++++\n 2 files changed, 37 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex 3d80bd6116..15c5d53712 100644\n--- a/http.c\n+++ b/http.c\n@@ -10,6 +10,7 @@\n #include \"credential.h\"\n #include \"version.h\"\n #include \"pkt-line.h\"\n+#include \"progress.h\"\n #include \"gettext.h\"\n #include \"trace.h\"\n #include \"transport.h\"\n@@ -1457,6 +1458,9 @@ struct active_request_slot *get_active_slot(void)\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);\n \tcurl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 1L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, NULL);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, NULL);\n \n \t/*\n \t * Default following to off unless \"ALWAYS\" is configured; this gives\n@@ -2017,6 +2021,21 @@ static void http_opt_request_remainder(CURL *curl, off_t pos)\n #define HTTP_REQUEST_STRBUF\t0\n #define HTTP_REQUEST_FILE\t1\n \n+static int http_progress_callback(void *clientp, curl_off_t dltotal,\n+\t\t\t\t curl_off_t dlnow, curl_off_t ultotal,\n+\t\t\t\t curl_off_t ulnow)\n+{\n+\tstruct progress *progress = clientp;\n+\n+\tif (progress) {\n+\t\tprogress_set_total(progress, dltotal);\n+\t\tdisplay_progress(progress, dlnow);\n+\t\tdisplay_throughput(progress, dlnow);\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int http_request(const char *url,\n \t\t\tvoid *result, int target,\n \t\t\tconst struct http_get_options *options)\n@@ -2025,6 +2044,7 @@ static int http_request(const char *url,\n \tstruct slot_results results;\n \tstruct curl_slist *headers = http_copy_default_headers();\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct progress *progress = NULL;\n \tconst char *accept_language;\n \tint ret;\n \n@@ -2061,6 +2081,13 @@ static int http_request(const char *url,\n \tif (options && options->initial_request &&\n \t http_follow_config == HTTP_FOLLOW_INITIAL)\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n+\tif (options && options->progress) {\n+\t\tprogress = start_progress(_(\"Downloading via HTTP\"), 0);\n+\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 0L);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, progress);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, &http_progress_callback);\n+\t}\n \n \theaders = curl_slist_append(headers, buf.buf);\n \n@@ -2079,6 +2106,11 @@ static int http_request(const char *url,\n \n \tret = run_one_slot(slot, &results);\n \n+\tif (progress) {\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, NULL);\n+\t\tstop_progress(&progress);\n+\t}\n+\n \tif (options && options->content_type) {\n \t\tstruct strbuf raw = STRBUF_INIT;\n \t\tcurlinfo_strbuf(slot->curl, CURLINFO_CONTENT_TYPE, &raw);\ndiff --git a/http.h b/http.h\nindex 3af19a8bf5..37ecddec17 100644\n--- a/http.h\n+++ b/http.h\n@@ -146,6 +146,11 @@ struct http_get_options {\n \t * request has completed.\n \t */\n \tstruct string_list *extra_headers;\n+\n+\t/*\n+\t * If not zero, display the progress.\n+\t */\n+\tint progress;\n };\n \n /* Return values for http_get_*() */\n-- \n2.44.0.651.g21306a098c\n\n"},{"id":"494317","messageId":"20240508124453.600871-4-toon@iotcl.com","threadId":"61434","inReplyTo":"20240508124453.600871-1-toon@iotcl.com","subject":"[PATCH 3/4] remote-curl: optionally show progress for HTTP get","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-05-08T12:44:52Z","receivedAt":"2024-05-08T12:45:14Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"git-remote-curl supports the `option progress` basically since it's\ninception. But this option had no effect for regular HTTP(S) downloads.\n\nAdd progress indicator when downloading files through curl HTTP GET.\n\nSigned-off-by: Toon Claes \n---\n remote-curl.c | 8 +++++++-\n t/t5557-http-get.sh | 15 +++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 0b6d7815fd..9fc7c3580c 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1293,6 +1293,7 @@ static void parse_get(const char *arg)\n {\n \tstruct strbuf url = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n+\tstruct http_get_options http_options = {0};\n \tconst char *space;\n \n \tspace = strchr(arg, ' ');\n@@ -1303,7 +1304,12 @@ static void parse_get(const char *arg)\n \tstrbuf_add(&url, arg, space - arg);\n \tstrbuf_addstr(&path, space + 1);\n \n-\tif (http_get_file(url.buf, path.buf, NULL))\n+\thttp_options.initial_request = 1;\n+\n+\tif (options.progress)\n+\t\thttp_options.progress = 1;\n+\n+\tif (http_get_file(url.buf, path.buf, &http_options))\n \t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n \n \tstrbuf_release(&url);\ndiff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\nindex 76a4bbd16a..92a138caaf 100755\n--- a/t/t5557-http-get.sh\n+++ b/t/t5557-http-get.sh\n@@ -36,4 +36,19 @@ test_expect_success 'get by URL: 200' '\n \ttest_cmp \"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" file2\n '\n \n+test_expect_success 'get by URL with progress' '\n+\techo hello >\"$HTTPD_DOCUMENT_ROOT_PATH/hello.txt\" &&\n+\n+\turl=\"$HTTPD_URL/hello.txt\" &&\n+\tcat >input <<-EOF &&\n+\tcapabilities\n+\toption progress true\n+\tget $url file3\n+\n+\tEOF\n+\n+\tgit remote-http $url err &&\n+ test_grep \"^Downloading via HTTP: 100%\" err\n+'\n+\n test_done\n-- \n2.44.0.651.g21306a098c\n\n"},{"id":"494318","messageId":"20240508124453.600871-5-toon@iotcl.com","threadId":"61434","inReplyTo":"20240508124453.600871-1-toon@iotcl.com","subject":"[PATCH 4/4] bundle-uri: enable git-remote-https progress","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-05-08T12:44:53Z","receivedAt":"2024-05-08T12:45:16Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When using bundle URIs large files might get downloaded during clone.\nDuring that time, there's no feedback to the user what is happening.\n\nEnable HTTP download progress for bundle URIs to inform the user.\n\nSigned-off-by: Toon Claes \n---\n bundle-uri.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex ca32050a78..462f00f668 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -293,7 +293,6 @@ static int download_https_uri_to_file(const char *file, const char *uri)\n \tint found_get = 0;\n \n \tstrvec_pushl(&cp.args, \"git-remote-https\", uri, NULL);\n-\tcp.err = -1;\n \tcp.in = -1;\n \tcp.out = -1;\n \n@@ -328,6 +327,9 @@ static int download_https_uri_to_file(const char *file, const char *uri)\n \t\tgoto cleanup;\n \t}\n \n+\tfprintf(child_in, \"option progress true\\n\");\n+\tfflush(child_in);\n+\n \tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n \n cleanup:\n-- \n2.44.0.651.g21306a098c\n\n"},{"id":"494328","messageId":"CAPig+cRS1SE5RNoqMp4F=rdjy7ueukN82q9qRaLdQWjjDJDvXQ@mail.gmail.com","threadId":"61434","inReplyTo":"20240508124453.600871-3-toon@iotcl.com","subject":"Re: [PATCH 2/4] http: add the ability to log progress","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-05-08T16:52:57Z","receivedAt":"2024-05-08T16:53:09Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 8, 2024 at 8:45 AM Toon Claes wrote:\n> Add an option `progress` to `struct http_get_options` to allow the\n> caller to enable download progress using the progress.c API.\n>\n> Signed-off-by: Toon Claes \n> ---\n> diff --git a/http.c b/http.c\n> @@ -2061,6 +2081,13 @@ static int http_request(const char *url,\n> + if (options && options->progress) {\n> + progress = start_progress(_(\"Downloading via HTTP\"), 0);\n> +\n> + curl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 0L);\n> + curl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, progress);\n> + curl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, &http_progress_callback);\n> + }\n> @@ -2079,6 +2106,11 @@ static int http_request(const char *url,\n> + if (progress) {\n> + curl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, NULL);\n> + stop_progress(&progress);\n> + }\n\nThe changes thus far in the series all seem very straightforward. Nicely done.\n\nCan you explain to this reviewer why you only reset\nCURLOPT_XFERINFODATA here, but not CURLOPT_NOPROGRESS and\nCURLOPT_XFERINFOFUNCTION?\n"},{"id":"494334","messageId":"xmqq5xvox7i7.fsf@gitster.g","threadId":"61434","inReplyTo":"20240508124453.600871-4-toon@iotcl.com","subject":"Re: [PATCH 3/4] remote-curl: optionally show progress for HTTP get","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-08T22:29:20Z","receivedAt":"2024-05-08T22:29:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> git-remote-curl supports the `option progress` basically since it's\n\n\"it's\" -> \"its\".\n\n> inception. But this option had no effect for regular HTTP(S) downloads.\n>\n> Add progress indicator when downloading files through curl HTTP GET.\n>\n> Signed-off-by: Toon Claes \n> ---\n> remote-curl.c | 8 +++++++-\n> t/t5557-http-get.sh | 15 +++++++++++++++\n> 2 files changed, 22 insertions(+), 1 deletion(-)\n>\n> diff --git a/remote-curl.c b/remote-curl.c\n> index 0b6d7815fd..9fc7c3580c 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -1293,6 +1293,7 @@ static void parse_get(const char *arg)\n> {\n> \tstruct strbuf url = STRBUF_INIT;\n> \tstruct strbuf path = STRBUF_INIT;\n> +\tstruct http_get_options http_options = {0};\n> \tconst char *space;\n> \n> \tspace = strchr(arg, ' ');\n> @@ -1303,7 +1304,12 @@ static void parse_get(const char *arg)\n> \tstrbuf_add(&url, arg, space - arg);\n> \tstrbuf_addstr(&path, space + 1);\n> \n> -\tif (http_get_file(url.buf, path.buf, NULL))\n> +\thttp_options.initial_request = 1;\n> +\n> +\tif (options.progress)\n> +\t\thttp_options.progress = 1;\n> +\n> +\tif (http_get_file(url.buf, path.buf, &http_options))\n> \t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n> \n> \tstrbuf_release(&url);\n> diff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\n> index 76a4bbd16a..92a138caaf 100755\n> --- a/t/t5557-http-get.sh\n> +++ b/t/t5557-http-get.sh\n> @@ -36,4 +36,19 @@ test_expect_success 'get by URL: 200' '\n> \ttest_cmp \"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" file2\n> '\n> \n> +test_expect_success 'get by URL with progress' '\n> +\techo hello >\"$HTTPD_DOCUMENT_ROOT_PATH/hello.txt\" &&\n> +\n> +\turl=\"$HTTPD_URL/hello.txt\" &&\n> +\tcat >input <<-EOF &&\n> +\tcapabilities\n> +\toption progress true\n> +\tget $url file3\n> +\n> +\tEOF\n> +\n> +\tgit remote-http $url err &&\n> + test_grep \"^Downloading via HTTP: 100%\" err\n> +'\n\nAre we sure that by the time we finish the transfer we know the\ntotal? Does this rely on the fact that somehow we are in\ncontrol of the server and can force it to use content-length:\n(instead of chunked encoding without any explicit indication of the\ntotal length)?\n\nAlso, I am curious if we know how often the progress indicator is\nupdated by cURL library with the API used in [2/4] (and if we can\ncontrol the frequency if we wanted to). Note that this is me just\nbeing curious, not a request to make it tweakable. The findings may\nhelp polish the [2/4], though.\n\nThanks.\n\n> +\n> test_done\n"},{"id":"494336","messageId":"xmqqr0ebx3s5.fsf@gitster.g","threadId":"61434","inReplyTo":"20240508124453.600871-1-toon@iotcl.com","subject":"Re: [PATCH 0/4] bundle-uri: show progress when downloading from bundle URIs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-08T23:49:46Z","receivedAt":"2024-05-08T23:49:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> Toon Claes (4):\n> progress: add function to set total\n> http: add the ability to log progress\n> remote-curl: optionally show progress for HTTP get\n> bundle-uri: enable git-remote-https progress\n>\n> bundle-uri.c | 4 +++-\n> http.c | 32 ++++++++++++++++++++++++++++++++\n> http.h | 5 +++++\n> progress.c | 6 ++++++\n> progress.h | 1 +\n> remote-curl.c | 8 +++++++-\n> t/helper/test-progress.c | 5 +++++\n> t/t0500-progress-display.sh | 24 ++++++++++++++++++++++++\n> t/t5557-http-get.sh | 15 +++++++++++++++\n> 9 files changed, 98 insertions(+), 2 deletions(-)\n\nThis topic, when built on tip of recent tip of 'master'\n(d4cc1ec35f), or merged to 'seen', seems to break t5558.\n\n\n$ sh t5558-clone-bundle-uri.sh -i -v\nInitialized empty Git repository in /home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/.git/\nexpecting success of 5558.1 'fail to clone from non-existent file': \n\ttest_when_finished rm -rf test &&\n\tgit clone --bundle-uri=\"$(pwd)/does-not-exist\" . test 2>err &&\n\tgrep \"failed to download bundle from URI\" err\n\nwarning: failed to download bundle from URI '/home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/does-not-exist'\nok 1 - fail to clone from non-existent file\n\nexpecting success of 5558.2 'fail to clone from non-bundle file': \n\ttest_when_finished rm -rf test &&\n\techo bogus >bogus &&\n\tgit clone --bundle-uri=\"$(pwd)/bogus\" . test 2>err &&\n\tgrep \"is not a bundle\" err\n\nwarning: file at URI '/home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/bogus' is not a bundle or bundle list\nok 2 - fail to clone from non-bundle file\n\nexpecting success of 5558.3 'create bundle': \n\tgit init clone-from &&\n\tgit -C clone-from checkout -b topic &&\n\ttest_commit -C clone-from A &&\n\ttest_commit -C clone-from B &&\n\tgit -C clone-from bundle create B.bundle topic\n\nInitialized empty Git repository in /home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/clone-from/.git/\nSwitched to a new branch 'topic'\n[topic (root-commit) 0ddfaf1] A\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 A.t\n[topic d9df450] B\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 B.t\nok 3 - create bundle\n\nexpecting success of 5558.4 'clone with path bundle': \n\tgit clone --bundle-uri=\"clone-from/B.bundle\" \\\n\t\tclone-from clone-path &&\n\tgit -C clone-path rev-parse refs/bundles/topic >actual &&\n\tgit -C clone-from rev-parse topic >expect &&\n\ttest_cmp expect actual\n\nCloning into 'clone-path'...\ndone.\nok 4 - clone with path bundle\n\nexpecting success of 5558.5 'clone with path bundle and non-default hash': \n\ttest_when_finished \"rm -rf clone-path-non-default-hash\" &&\n\tGIT_DEFAULT_HASH=sha256 git clone --bundle-uri=\"clone-from/B.bundle\" \\\n\t\tclone-from clone-path-non-default-hash &&\n\tgit -C clone-path-non-default-hash rev-parse refs/bundles/topic >actual &&\n\tgit -C clone-from rev-parse topic >expect &&\n\ttest_cmp expect actual\n\nCloning into 'clone-path-non-default-hash'...\ndone.\nok 5 - clone with path bundle and non-default hash\n\nexpecting success of 5558.6 'clone with file:// bundle': \n\tgit clone --bundle-uri=\"file://$(pwd)/clone-from/B.bundle\" \\\n\t\tclone-from clone-file &&\n\tgit -C clone-file rev-parse refs/bundles/topic >actual &&\n\tgit -C clone-from rev-parse topic >expect &&\n\ttest_cmp expect actual\n\nCloning into 'clone-file'...\ndone.\nok 6 - clone with file:// bundle\n\nexpecting success of 5558.7 'construct incremental bundle list': \n\t(\n\t\tcd clone-from &&\n\t\tgit checkout -b base &&\n\t\ttest_commit 1 &&\n\t\tgit checkout -b left &&\n\t\ttest_commit 2 &&\n\t\tgit checkout -b right base &&\n\t\ttest_commit 3 &&\n\t\tgit checkout -b merge left &&\n\t\tgit merge right -m \"4\" &&\n\n\t\tgit bundle create bundle-1.bundle base &&\n\t\tgit bundle create bundle-2.bundle base..left &&\n\t\tgit bundle create bundle-3.bundle base..right &&\n\t\tgit bundle create bundle-4.bundle merge --not left right\n\t)\n\nSwitched to a new branch 'base'\n[base 65d47d5] 1\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 1.t\nSwitched to a new branch 'left'\n[left 918de9f] 2\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 2.t\nSwitched to a new branch 'right'\n[right d1bc9f9] 3\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 3.t\nSwitched to a new branch 'merge'\nMerge made by the 'ort' strategy.\n 3.t | 1 +\n 1 file changed, 1 insertion(+)\n create mode 100644 3.t\nok 7 - construct incremental bundle list\n\nexpecting success of 5558.8 'clone bundle list (file, no heuristic)': \n\tcat >bundle-list <<-EOF &&\n\t[bundle]\n\t\tversion = 1\n\t\tmode = all\n\n\t[bundle \"bundle-1\"]\n\t\turi = file://$(pwd)/clone-from/bundle-1.bundle\n\n\t[bundle \"bundle-2\"]\n\t\turi = file://$(pwd)/clone-from/bundle-2.bundle\n\n\t[bundle \"bundle-3\"]\n\t\turi = file://$(pwd)/clone-from/bundle-3.bundle\n\n\t[bundle \"bundle-4\"]\n\t\turi = file://$(pwd)/clone-from/bundle-4.bundle\n\tEOF\n\n\tgit clone --bundle-uri=\"file://$(pwd)/bundle-list\" \\\n\t\tclone-from clone-list-file 2>err &&\n\t! grep \"Repository lacks these prerequisite commits\" err &&\n\n\tgit -C clone-from for-each-ref --format=\"%(objectname)\" >oids &&\n\tgit -C clone-list-file cat-file --batch-check refs &&\n\tgrep \"refs/bundles/\" refs >actual &&\n\tcat >expect <<-\\EOF &&\n\trefs/bundles/base\n\trefs/bundles/left\n\trefs/bundles/merge\n\trefs/bundles/right\n\tEOF\n\ttest_cmp expect actual\n\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\n475812ed564e3085586d0b0acc392cec0a90c18e commit 261\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\n0ddfaf193ff13d6ab39b7cbd9eed645e3ee2f050 commit 165\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\nok 8 - clone bundle list (file, no heuristic)\n\nexpecting success of 5558.9 'clone bundle list (file, all mode, some failures)': \n\tcat >bundle-list <<-EOF &&\n\t[bundle]\n\t\tversion = 1\n\t\tmode = all\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-0\"]\n\t\turi = file://$(pwd)/clone-from/bundle-0.bundle\n\n\t[bundle \"bundle-1\"]\n\t\turi = file://$(pwd)/clone-from/bundle-1.bundle\n\n\t[bundle \"bundle-2\"]\n\t\turi = file://$(pwd)/clone-from/bundle-2.bundle\n\n\t# No bundle-3 means bundle-4 will not apply.\n\n\t[bundle \"bundle-4\"]\n\t\turi = file://$(pwd)/clone-from/bundle-4.bundle\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-5\"]\n\t\turi = file://$(pwd)/clone-from/bundle-5.bundle\n\tEOF\n\n\tGIT_TRACE2_PERF=1 \\\n\tgit clone --bundle-uri=\"file://$(pwd)/bundle-list\" \\\n\t\tclone-from clone-all-some 2>err &&\n\t! grep \"Repository lacks these prerequisite commits\" err &&\n\t! grep \"fatal\" err &&\n\tgrep \"warning: failed to download bundle from URI\" err &&\n\n\tgit -C clone-from for-each-ref --format=\"%(objectname)\" >oids &&\n\tgit -C clone-all-some cat-file --batch-check refs &&\n\tgrep \"refs/bundles/\" refs >actual &&\n\tcat >expect <<-\\EOF &&\n\trefs/bundles/base\n\trefs/bundles/left\n\tEOF\n\ttest_cmp expect actual\n\nwarning: failed to download bundle from URI 'file:///home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/clone-from/bundle-5.bundle'\nwarning: failed to download bundle from URI 'file:///home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/clone-from/bundle-0.bundle'\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\n475812ed564e3085586d0b0acc392cec0a90c18e commit 261\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\n0ddfaf193ff13d6ab39b7cbd9eed645e3ee2f050 commit 165\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\nok 9 - clone bundle list (file, all mode, some failures)\n\nexpecting success of 5558.10 'clone bundle list (file, all mode, all failures)': \n\tcat >bundle-list <<-EOF &&\n\t[bundle]\n\t\tversion = 1\n\t\tmode = all\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-0\"]\n\t\turi = file://$(pwd)/clone-from/bundle-0.bundle\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-5\"]\n\t\turi = file://$(pwd)/clone-from/bundle-5.bundle\n\tEOF\n\n\tgit clone --bundle-uri=\"file://$(pwd)/bundle-list\" \\\n\t\tclone-from clone-all-fail 2>err &&\n\t! grep \"Repository lacks these prerequisite commits\" err &&\n\t! grep \"fatal\" err &&\n\tgrep \"warning: failed to download bundle from URI\" err &&\n\n\tgit -C clone-from for-each-ref --format=\"%(objectname)\" >oids &&\n\tgit -C clone-all-fail cat-file --batch-check refs &&\n\t! grep \"refs/bundles/\" refs\n\nwarning: failed to download bundle from URI 'file:///home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/clone-from/bundle-5.bundle'\nwarning: failed to download bundle from URI 'file:///home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/clone-from/bundle-0.bundle'\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\n475812ed564e3085586d0b0acc392cec0a90c18e commit 261\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\n0ddfaf193ff13d6ab39b7cbd9eed645e3ee2f050 commit 165\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\nok 10 - clone bundle list (file, all mode, all failures)\n\nexpecting success of 5558.11 'clone bundle list (file, any mode)': \n\tcat >bundle-list <<-EOF &&\n\t[bundle]\n\t\tversion = 1\n\t\tmode = any\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-0\"]\n\t\turi = file://$(pwd)/clone-from/bundle-0.bundle\n\n\t[bundle \"bundle-1\"]\n\t\turi = file://$(pwd)/clone-from/bundle-1.bundle\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-5\"]\n\t\turi = file://$(pwd)/clone-from/bundle-5.bundle\n\tEOF\n\n\tgit clone --bundle-uri=\"file://$(pwd)/bundle-list\" \\\n\t\tclone-from clone-any-file 2>err &&\n\t! grep \"Repository lacks these prerequisite commits\" err &&\n\n\tgit -C clone-from for-each-ref --format=\"%(objectname)\" >oids &&\n\tgit -C clone-any-file cat-file --batch-check refs &&\n\tgrep \"refs/bundles/\" refs >actual &&\n\tcat >expect <<-\\EOF &&\n\trefs/bundles/base\n\tEOF\n\ttest_cmp expect actual\n\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\n475812ed564e3085586d0b0acc392cec0a90c18e commit 261\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\n0ddfaf193ff13d6ab39b7cbd9eed645e3ee2f050 commit 165\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\nok 11 - clone bundle list (file, any mode)\n\nexpecting success of 5558.12 'clone bundle list (file, any mode, all failures)': \n\tcat >bundle-list <<-EOF &&\n\t[bundle]\n\t\tversion = 1\n\t\tmode = any\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-0\"]\n\t\turi = $HTTPD_URL/bundle-0.bundle\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-5\"]\n\t\turi = $HTTPD_URL/bundle-5.bundle\n\tEOF\n\n\tgit clone --bundle-uri=\"file://$(pwd)/bundle-list\" \\\n\t\tclone-from clone-any-fail 2>err &&\n\t! grep \"fatal\" err &&\n\tgrep \"warning: failed to download bundle from URI\" err &&\n\n\tgit -C clone-from for-each-ref --format=\"%(objectname)\" >oids &&\n\tgit -C clone-any-fail cat-file --batch-check refs &&\n\t! grep \"refs/bundles/\" refs\n\nwarning: failed to download bundle from URI '/bundle-5.bundle'\nwarning: failed to download bundle from URI '/bundle-0.bundle'\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\n475812ed564e3085586d0b0acc392cec0a90c18e commit 261\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\n0ddfaf193ff13d6ab39b7cbd9eed645e3ee2f050 commit 165\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\nok 12 - clone bundle list (file, any mode, all failures)\n\nchecking prerequisite: NOT_ROOT\n\nmkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-NOT_ROOT\" &&\n(\n\tcd \"$TRASH_DIRECTORY/prereq-test-dir-NOT_ROOT\" &&\n\tuid=$(id -u) &&\n\ttest \"$uid\" != 0\n\n)\nprerequisite NOT_ROOT ok\nexpecting success of 5558.13 'fail to fetch from non-existent HTTP URL': \n\ttest_when_finished rm -rf test &&\n\tgit clone --bundle-uri=\"$HTTPD_URL/does-not-exist\" . test 2>err &&\n\tgrep \"failed to download bundle from URI\" err\n\nwarning: failed to download bundle from URI 'http://127.0.0.1:5558/does-not-exist'\nok 13 - fail to fetch from non-existent HTTP URL\n\nexpecting success of 5558.14 'fail to fetch from non-bundle HTTP URL': \n\ttest_when_finished rm -rf test &&\n\techo bogus >\"$HTTPD_DOCUMENT_ROOT_PATH/bogus\" &&\n\tgit clone --bundle-uri=\"$HTTPD_URL/bogus\" . test 2>err &&\n\tgrep \"is not a bundle\" err\n\nwarning: file at URI 'http://127.0.0.1:5558/bogus' is not a bundle or bundle list\nok 14 - fail to fetch from non-bundle HTTP URL\n\nexpecting success of 5558.15 'clone HTTP bundle': \n\tcp clone-from/B.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/B.bundle\" &&\n\n\tgit clone --no-local --mirror clone-from \\\n\t\t\"$HTTPD_DOCUMENT_ROOT_PATH/fetch.git\" &&\n\n\tgit clone --bundle-uri=\"$HTTPD_URL/B.bundle\" \\\n\t\t\"$HTTPD_URL/smart/fetch.git\" clone-http &&\n\tgit -C clone-http rev-parse refs/bundles/topic >actual &&\n\tgit -C clone-from rev-parse topic >expect &&\n\ttest_cmp expect actual &&\n\n\ttest_config -C clone-http log.excludedecoration refs/bundle/\n\nCloning into bare repository '/home/gitster/w/git.git/t/trash directory.t5558-clone-bundle-uri/httpd/www/fetch.git'...\nCloning into 'clone-http'...\nDownloading via HTTP: 100% (520/520)^MDownloading via HTTP: 100% (520/520), 520 bytes | 520.00 KiB/s, done.\nok 15 - clone HTTP bundle\n\nexpecting success of 5558.16 'clone HTTP bundle with non-default hash': \n\ttest_when_finished \"rm -rf clone-http-non-default-hash\" &&\n\tGIT_DEFAULT_HASH=sha256 git clone --bundle-uri=\"$HTTPD_URL/B.bundle\" \\\n\t\t\"$HTTPD_URL/smart/fetch.git\" clone-http-non-default-hash &&\n\tgit -C clone-http-non-default-hash rev-parse refs/bundles/topic >actual &&\n\tgit -C clone-from rev-parse topic >expect &&\n\ttest_cmp expect actual\n\nCloning into 'clone-http-non-default-hash'...\nDownloading via HTTP: 100% (520/520)^MDownloading via HTTP: 100% (520/520), 520 bytes | 520.00 KiB/s, done.\nok 16 - clone HTTP bundle with non-default hash\n\nexpecting success of 5558.17 'clone bundle list (HTTP, no heuristic)': \n\ttest_when_finished rm -f trace*.txt &&\n\n\tcp clone-from/bundle-*.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/\" &&\n\tcat >\"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list\" <<-EOF &&\n\t[bundle]\n\t\tversion = 1\n\t\tmode = all\n\n\t[bundle \"bundle-1\"]\n\t\turi = $HTTPD_URL/bundle-1.bundle\n\n\t[bundle \"bundle-2\"]\n\t\turi = $HTTPD_URL/bundle-2.bundle\n\n\t[bundle \"bundle-3\"]\n\t\turi = $HTTPD_URL/bundle-3.bundle\n\n\t[bundle \"bundle-4\"]\n\t\turi = $HTTPD_URL/bundle-4.bundle\n\tEOF\n\n\tGIT_TRACE2_EVENT=\"$(pwd)/trace-clone.txt\" \\\n\t\tgit clone --bundle-uri=\"$HTTPD_URL/bundle-list\" \\\n\t\tclone-from clone-list-http 2>err &&\n\t! grep \"Repository lacks these prerequisite commits\" err &&\n\n\tgit -C clone-from for-each-ref --format=\"%(objectname)\" >oids &&\n\tgit -C clone-list-http cat-file --batch-check expect <<-EOF &&\n\t$HTTPD_URL/bundle-1.bundle\n\t$HTTPD_URL/bundle-2.bundle\n\t$HTTPD_URL/bundle-3.bundle\n\t$HTTPD_URL/bundle-4.bundle\n\t$HTTPD_URL/bundle-list\n\tEOF\n\n\t# Sort the list, since the order is not well-defined\n\t# without a heuristic.\n\ttest_remote_https_urls actual &&\n\ttest_cmp expect actual\n\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\n475812ed564e3085586d0b0acc392cec0a90c18e commit 261\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\n65d47d507bde981075d2f2532bb104e4b58b9170 commit 213\n918de9f3988f29866b94f657626edb7247c7ddbd commit 213\nd1bc9f9c222df7b7de791db57cdf7e8ba0627c3e commit 213\n0ddfaf193ff13d6ab39b7cbd9eed645e3ee2f050 commit 165\nd9df4505cb3522088b9e29d6051ac16f1564154a commit 213\nok 17 - clone bundle list (HTTP, no heuristic)\n\nexpecting success of 5558.18 'clone bundle list (HTTP, any mode)': \n\tcp clone-from/bundle-*.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/\" &&\n\tcat >\"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list\" <<-EOF &&\n\t[bundle]\n\t\tversion = 1\n\t\tmode = any\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-0\"]\n\t\turi = $HTTPD_URL/bundle-0.bundle\n\n\t[bundle \"bundle-1\"]\n\t\turi = $HTTPD_URL/bundle-1.bundle\n\n\t# Does not exist. Should be skipped.\n\t[bundle \"bundle-5\"]\n\t\turi = $HTTPD_URL/bundle-5.bundle\n\tEOF\n\n\tgit clone --bundle-uri=\"$HTTPD_URL/bundle-list\" \\\n\t\tclone-from clone-any-http 2>err &&\n\t! grep \"fatal\" err &&\n\tgrep \"warning: failed to download bundle from URI\" err &&\n\n\tgit -C clone-from for-each-ref --format=\"%(objectname)\" >oids &&\n\tgit -C clone-any-http cat-file --batch-check refs &&\n\tgrep \"refs/bundles/\" refs >actual &&\n\tcat >expect <<-\\EOF &&\n\trefs/bundles/base\n\trefs/bundles/left\n\trefs/bundles/merge\n\trefs/bundles/right\n\tEOF\n\ttest_cmp expect actual\n\nfatal: failed to download file at URL 'http://127.0.0.1:5558/bundle-5.bundle'\nfatal: failed to download file at URL 'http://127.0.0.1:5558/bundle-0.bundle'\nnot ok 18 - clone bundle list (HTTP, any mode)\n#\t\n#\t\tcp clone-from/bundle-*.bundle \"$HTTPD_DOCUMENT_ROOT_PATH/\" &&\n#\t\tcat >\"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list\" <<-EOF &&\n#\t\t[bundle]\n#\t\t\tversion = 1\n#\t\t\tmode = any\n#\t\n#\t\t# Does not exist. Should be skipped.\n#\t\t[bundle \"bundle-0\"]\n#\t\t\turi = $HTTPD_URL/bundle-0.bundle\n#\t\n#\t\t[bundle \"bundle-1\"]\n#\t\t\turi = $HTTPD_URL/bundle-1.bundle\n#\t\n#\t\t# Does not exist. Should be skipped.\n#\t\t[bundle \"bundle-5\"]\n#\t\t\turi = $HTTPD_URL/bundle-5.bundle\n#\t\tEOF\n#\t\n#\t\tgit clone --bundle-uri=\"$HTTPD_URL/bundle-list\" \\\n#\t\t\tclone-from clone-any-http 2>err &&\n#\t\t! grep \"fatal\" err &&\n#\t\tgrep \"warning: failed to download bundle from URI\" err &&\n#\t\n#\t\tgit -C clone-from for-each-ref --format=\"%(objectname)\" >oids &&\n#\t\tgit -C clone-any-http cat-file --batch-check refs &&\n#\t\tgrep \"refs/bundles/\" refs >actual &&\n#\t\tcat >expect <<-\\EOF &&\n#\t\trefs/bundles/base\n#\t\trefs/bundles/left\n#\t\trefs/bundles/merge\n#\t\trefs/bundles/right\n#\t\tEOF\n#\t\ttest_cmp expect actual\n#\t\n1..18\n"},{"id":"494362","messageId":"20240509163414.GA1708095@coredump.intra.peff.net","threadId":"61434","inReplyTo":"20240508124453.600871-3-toon@iotcl.com","subject":"Re: [PATCH 2/4] http: add the ability to log progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-05-09T16:34:14Z","receivedAt":"2024-05-09T16:34:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 08, 2024 at 02:44:51PM +0200, Toon Claes wrote:\n\n> @@ -1457,6 +1458,9 @@ struct active_request_slot *get_active_slot(void)\n> \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n> \tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);\n> \tcurl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);\n> +\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 1L);\n> +\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, NULL);\n> +\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, NULL);\n\nThese last two CURLOPTs appeared in 7.32.0, but our INSTALL doc claims\nto support back to 7.21.3. Before that you're supposed to use\nPROGRESSFUNCTION instead, which has a slightly different signature. I\nthink you could support both, though it would also be OK to just disable\nthis extra progress for antique curl.\n\nIt might also be reasonable to just bump to 7.32.0 as our minimum. The\nlast bump was recent via c28ee09503 (INSTALL: bump libcurl version to\n7.21.3, 2024-04-02), and the version picked there was arbitrary-ish (it\nwas something we had happened to depend on accidentally). 7.32.0 is\nitself almost 11 years old now.\n\n-Peff\n"},{"id":"494364","messageId":"20240509164646.GB1708095@coredump.intra.peff.net","threadId":"61434","inReplyTo":"20240508124453.600871-5-toon@iotcl.com","subject":"Re: [PATCH 4/4] bundle-uri: enable git-remote-https progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-05-09T16:46:46Z","receivedAt":"2024-05-09T16:46:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 08, 2024 at 02:44:53PM +0200, Toon Claes wrote:\n\n> diff --git a/bundle-uri.c b/bundle-uri.c\n> index ca32050a78..462f00f668 100644\n> --- a/bundle-uri.c\n> +++ b/bundle-uri.c\n> @@ -293,7 +293,6 @@ static int download_https_uri_to_file(const char *file, const char *uri)\n> \tint found_get = 0;\n> \n> \tstrvec_pushl(&cp.args, \"git-remote-https\", uri, NULL);\n> -\tcp.err = -1;\n> \tcp.in = -1;\n> \tcp.out = -1;\n\nThis is the cause of at least one test failure in t5558, I think. We\nspawn a remote-https to try to download the bundle, but it may not be\npresent, and we continue without it. In that case, the child\nremote-https says something like:\n\n fatal: failed to download file at URL 'http://127.0.0.1:5558/bundle-5.bundle'\n\nand then the caller says:\n\n warning: failed to download bundle from URI 'http://127.0.0.1:5558/bundle-5.bundle'\n\nPrior to this patch, the \"fatal\" part coming from the child process was\nsuppressed (and the test checks that this is so, which is why it fails,\neven though the clone itself works fine).\n\nObviously you need to enable stderr to see the progress, so I'm not sure\nhow to resolve it. In an ideal world, you'd ask for the two over\nseparate descriptors, but I think run_command() only supports 0/1/2\nstdio due to portability limitations for Windows.\n\nOne option is that remote-https could ferry machine-readable output back\nto the parent over stdout, which could then format it for the user.\n\nAnother is that we could somehow ask remote-https to squelch\nnon-progress errors, though that feels a bit weird (and awkward to\nimplement, since the message comes from a die() call).\n\n> @@ -328,6 +327,9 @@ static int download_https_uri_to_file(const char *file, const char *uri)\n> \t\tgoto cleanup;\n> \t}\n> \n> +\tfprintf(child_in, \"option progress true\\n\");\n> +\tfflush(child_in);\n> +\n> \tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n\nI was curious why you'd flush here. It makes sense if you are going to\nthen read back the response from stdout, but we never do (it should be a\nsingle \"ok\" line). I think it's OK to ignore it, given that we'd just\ncontinue without progress in that case.\n\nBut I did wonder why this wasn't messing up reading the response from\nthe \"get\" command just below. The answer is that we don't read that\nresponse either! We just assume that the process will exit successfully\nor not based on the result. We are relying on the pipe buffer to avoid\ndeadlock, but I think that's OK since the output is always going to be\nsmall.\n\n-Peff\n"},{"id":"494365","messageId":"xmqqbk5fudw8.fsf@gitster.g","threadId":"61434","inReplyTo":"20240509163414.GA1708095@coredump.intra.peff.net","subject":"Re: [PATCH 2/4] http: add the ability to log progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-09T16:51:51Z","receivedAt":"2024-05-09T16:51:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, May 08, 2024 at 02:44:51PM +0200, Toon Claes wrote:\n>\n>> @@ -1457,6 +1458,9 @@ struct active_request_slot *get_active_slot(void)\n>> \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n>> \tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);\n>> \tcurl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);\n>> +\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 1L);\n>> +\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, NULL);\n>> +\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, NULL);\n>\n> These last two CURLOPTs appeared in 7.32.0, but our INSTALL doc claims\n> to support back to 7.21.3. Before that you're supposed to use\n> PROGRESSFUNCTION instead, which has a slightly different signature. I\n> think you could support both, though it would also be OK to just disable\n> this extra progress for antique curl.\n>\n> It might also be reasonable to just bump to 7.32.0 as our minimum. The\n> last bump was recent via c28ee09503 (INSTALL: bump libcurl version to\n> 7.21.3, 2024-04-02), and the version picked there was arbitrary-ish (it\n> was something we had happened to depend on accidentally). 7.32.0 is\n> itself almost 11 years old now.\n\nThe last bump was 7.19.5 (May 2009, 14.9 years) to 7.21.3 (Dec 2010,\n13.3 years). As 10 is a nice round number, we may even be able to\npick randomly a slightly newer one, say, 7.35.0 (Mar 2014, 10.0\nyears).\n\nIt is in a sense an inferiour way to pick the minimum dependency\nthan the choice of 7.32.0, which is backed by \"we use this and that,\nwhich appeared in that version\", of course.\n\nBut being able to update mechanically without thinking is tempting,\nand as long as the horizon is sufficiently long, such an approach\nwould not have a huge downside.\n\nThanks.\n"},{"id":"494366","messageId":"20240509165212.GC1708095@coredump.intra.peff.net","threadId":"61434","inReplyTo":"20240508124453.600871-3-toon@iotcl.com","subject":"Re: [PATCH 2/4] http: add the ability to log progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-05-09T16:52:12Z","receivedAt":"2024-05-09T16:52:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 08, 2024 at 02:44:51PM +0200, Toon Claes wrote:\n\n> @@ -2017,6 +2021,21 @@ static void http_opt_request_remainder(CURL *curl, off_t pos)\n> #define HTTP_REQUEST_STRBUF\t0\n> #define HTTP_REQUEST_FILE\t1\n> \n> +static int http_progress_callback(void *clientp, curl_off_t dltotal,\n> +\t\t\t\t curl_off_t dlnow, curl_off_t ultotal,\n> +\t\t\t\t curl_off_t ulnow)\n> +{\n> +\tstruct progress *progress = clientp;\n> +\n> +\tif (progress) {\n> +\t\tprogress_set_total(progress, dltotal);\n> +\t\tdisplay_progress(progress, dlnow);\n> +\t\tdisplay_throughput(progress, dlnow);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n\nA very long time ago I implemented something similar (for a\nproto-bundle-uri feature), and I found out the hard way that both curl\nand our progress code may rely on SIGALRM (curl may use it to put a\ntimeout on blocking calls that don't support select, like DNS\nresolution). Making things even more confusing, it's platform dependent,\nsince on modern platforms it spawns a separate resolving thread to avoid\nblocking the main one.\n\nSo it seems to work OK for me on Linux without anything further, but we\nmay want this to be extra careful:\n\n-- >8 --\nDate: Thu, 10 Nov 2011 01:29:55 -0500\nSubject: [PATCH] http: turn off curl signals\n\nCurl sets and clears the handler for SIGALRM, which makes it\nincompatible with git's progress code. However, we can ask\ncurl not to do this.\n\nSigned-off-by: Jeff King \n---\n http.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex 752c879c1f..e7986e1353 100644\n--- a/http.c\n+++ b/http.c\n@@ -1230,6 +1230,8 @@ static CURL *get_curl_handle(void)\n \n \tset_curl_keepalive(result);\n \n+\tcurl_easy_setopt(result, CURLOPT_NOSIGNAL, 1);\n+\n \treturn result;\n }\n \n-- 8< --\n\nIn the other part of the thread I suggested the possibility of having\nremote-https shuttle machine-readable progress back to the caller. That\nwould also solve this issue, because the actual display_progress() calls\nwould happen in the parent.\n\n-Peff\n"},{"id":"494369","messageId":"20240509170453.GE1708095@coredump.intra.peff.net","threadId":"61434","inReplyTo":"xmqqbk5fudw8.fsf@gitster.g","subject":"Re: [PATCH 2/4] http: add the ability to log progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-05-09T17:04:53Z","receivedAt":"2024-05-09T17:04:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 09, 2024 at 09:51:51AM -0700, Junio C Hamano wrote:\n\n> > It might also be reasonable to just bump to 7.32.0 as our minimum. The\n> > last bump was recent via c28ee09503 (INSTALL: bump libcurl version to\n> > 7.21.3, 2024-04-02), and the version picked there was arbitrary-ish (it\n> > was something we had happened to depend on accidentally). 7.32.0 is\n> > itself almost 11 years old now.\n> \n> The last bump was 7.19.5 (May 2009, 14.9 years) to 7.21.3 (Dec 2010,\n> 13.3 years). As 10 is a nice round number, we may even be able to\n> pick randomly a slightly newer one, say, 7.35.0 (Mar 2014, 10.0\n> years).\n> \n> It is in a sense an inferiour way to pick the minimum dependency\n> than the choice of 7.32.0, which is backed by \"we use this and that,\n> which appeared in that version\", of course.\n\nI am OK with just using round numbers, too.\n\nThe biggest thing for a time-oriented scheme is really what made the\ncutoff for long-term distro releases. With a 10-year support term, we\nare often looking at 11 or so years to have made it into a release.\nOTOH, if somebody using a 10-year-old distro is forced to use a slightly\nolder version of Git to match, IMHO that is not the end of the world.\n\nBumping to 7.35.0 would also let us simplify some curl-compat code,\nwhich is always my real goal. ;)\n\n-Peff\n"},{"id":"512414","messageId":"87o6z43gz8.fsf@iotcl.com","threadId":"61434","inReplyTo":"20240509164646.GB1708095@coredump.intra.peff.net","subject":"Re: [PATCH 4/4] bundle-uri: enable git-remote-https progress","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-14T11:26:03Z","receivedAt":"2025-02-14T11:26:17Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Hi Peff,\n\nI am attempting to revive this patch series, and ran into your valuable\nfeedback below:\n\nJeff King writes:\n\n> On Wed, May 08, 2024 at 02:44:53PM +0200, Toon Claes wrote:\n>\n>> diff --git a/bundle-uri.c b/bundle-uri.c\n>> index ca32050a78..462f00f668 100644\n>> --- a/bundle-uri.c\n>> +++ b/bundle-uri.c\n>> @@ -293,7 +293,6 @@ static int download_https_uri_to_file(const char *file, const char *uri)\n>> \tint found_get = 0;\n>> \n>> \tstrvec_pushl(&cp.args, \"git-remote-https\", uri, NULL);\n>> -\tcp.err = -1;\n>> \tcp.in = -1;\n>> \tcp.out = -1;\n>\n> This is the cause of at least one test failure in t5558, I think. We\n> spawn a remote-https to try to download the bundle, but it may not be\n> present, and we continue without it. In that case, the child\n> remote-https says something like:\n>\n> fatal: failed to download file at URL 'http://127.0.0.1:5558/bundle-5.bundle'\n>\n> and then the caller says:\n>\n> warning: failed to download bundle from URI 'http://127.0.0.1:5558/bundle-5.bundle'\n>\n> Prior to this patch, the \"fatal\" part coming from the child process was\n> suppressed (and the test checks that this is so, which is why it fails,\n> even though the clone itself works fine).\n>\n> Obviously you need to enable stderr to see the progress, so I'm not sure\n> how to resolve it. In an ideal world, you'd ask for the two over\n> separate descriptors, but I think run_command() only supports 0/1/2\n> stdio due to portability limitations for Windows.\n>\n> One option is that remote-https could ferry machine-readable output back\n> to the parent over stdout, which could then format it for the user.\n>\n> Another is that we could somehow ask remote-https to squelch\n> non-progress errors, though that feels a bit weird (and awkward to\n> implement, since the message comes from a die() call).\n\nI've been playing around with things and I haven't found a good way\nforward with this. We could have the parent process ingest stderr of\ngit-remote-https and swallow messages that match `/^fatal:/`, but that\nfeels like a hack and not foolproof.\n\nI was thinking if we could override `die()` in the child process to have\nit not print anything, but because git-remote-http basically can call\ndie() basically from anywhere in the codebase, I don't think we can\nensure the silenced die() function is called.\n\nOr what do you mean by \"squelch non-progress errors\"?\n\nAnd yes, sending progress logging over a separate fd seems like the\nideal approach, but I haven't tried it yet. I'm afraid it's not worth\nattempting so.\n\nSo I think that leaves us with your suggestion to \"ferry\nmachine-readable output back to the parent\". If I understand correctly\nyou mean the child process will not write progress logging to stderr but\nto stdout (with some kind of command prefix the parent process knows\nwhat to do with this)?\n\nI imagine communication between parent and child will then look\nsomething like this:\n\n-> capabilities\n<- stateless-connect\n<- fetch\n<- get\n<- option\n<- push\n<- check-connectivity\n<- object-format\n-> option progress true\n<- ok\n-> get http://example.com git.bundle\n<- progress 123 345 40\n<- progress 234 345 50\n<- progress 345 345 40\n\n~fin~\n\nBut then we need to decide on the format the child sends back to the\nparent. In the above example it's something like `progress \n `. An alternative proposal could be:\n\n<- log Downloading via HTTP: \n<- log Downloading via HTTP: 200.00 KiB | 100.00 KiB/s\n<- log Downloading via HTTP: 300.00 KiB | 100.00 KiB/s\n<- log Downloading via HTTP: 400.00 KiB | 100.00 KiB/s\n<- log Downloading via HTTP: 400.00 KiB | 100.00 KiB/s, done.\n\nSo the child sends the progress text with a `log` prefix, which the\nparent simply has to send that logging to where it wants it to go.\n\nOr am I completely misunderstanding your proposal? Do you maybe happen\nto have any examples of a similar solution?\n\n\n--\nToon\n"},{"id":"512702","messageId":"20250219-toon-bundleuri-progress-v2-0-a84e7ffa921a@iotcl.com","threadId":"61434","inReplyTo":"20240508124453.600871-1-toon@iotcl.com","subject":"[PATCH v2 0/7] Show progress when downloading from bundle URIs","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-19T14:30:18Z","receivedAt":"2025-02-19T14:30:26Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When a user clones a repository, they see what's happening in the\nmessages like \"Enumerating objects\" and \"Receiving objects\". But when\na user clones a repository that uses bundle URIs they see:\n\n Cloning into 'repo.git'\n\nAnd then they have to wait until all bundles are downloaded before they\nsee any other message. When the bundles are large, this can take a lot\nof time and the user might consider the process hangs and they kill it.\n\nThis patch series introduces progress displayed to the user while\nbundles are downloaded. The full output of a clone using bundle URIs\nwill look something like:\n\n Cloning into 'repo.git'...\n Downloading via HTTP: 21% (351812809/1620086598), 315.34 MiB | 49.84 MiB/s\n Downloading via HTTP: 77% (1247493865/1620086598), 1.15 GiB | 34.31 MiB/s\n Downloading via HTTP: 100% (1620086598/1620086598), 1.50 GiB | 37.97 MiB/s, done.\n remote: Enumerating objects: 1322255, done.\n remote: Counting objects: 100% (611708/611708), done.\n remote: Total 1322255 (delta 611708), reused 611708 (delta 611708), pack-reused 710547\n Receiving objects: 100% (1322255/1322255), 539.66 MiB | 31.57 MiB/s, done.\n etc...\n\nIn this version I'm adding two commits from Peff. And I'm adding one\ncommit on top to fix the failure in t5558. I'd like to get some feedback\non that commit in particular.\n\n---\nChanges in v2:\n- Added commit \"http: silence stderr when progress is enabled\"\n- Added commit \"progress: allow pure-throughput progress meters\" from\n https://lore.kernel.org/git/20111110075300.GK27950@sigill.intra.peff.net/\n- Added commit \"http: turn off curl signals\" from\n https://lore.kernel.org/git/20240509165212.GC1708095@coredump.intra.peff.net/\n- Cleanup both CURLOPT_NOPROGRESS and CURLOPT_NOPROGRESS.\n- Remove unneeded fflush().\n- Link to v1: https://lore.kernel.org/git/20240508124453.600871-1-toon@iotcl.com/\n\nSigned-off-by: Toon Claes \n\n---\nJeff King (2):\n progress: allow pure-throughput progress meters\n http: turn off curl signals\n\nToon Claes (5):\n progress: add function to set total\n http: add the ability to log progress\n remote-curl: optionally show progress for HTTP get\n bundle-uri: enable git-remote-https progress\n http: silence stderr when progress is enabled\n\n bundle-uri.c | 2 +-\n http.c | 39 ++++++++++++++++++++++++++++++++\n http.h | 5 +++++\n progress.c | 40 ++++++++++++++++++++++++---------\n progress.h | 2 ++\n remote-curl.c | 8 ++++++-\n t/helper/test-progress.c | 5 +++++\n t/t0500-progress-display.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++\n t/t5557-http-get.sh | 15 +++++++++++++\n 9 files changed, 159 insertions(+), 12 deletions(-)\n---\n\nRange-diff versus v1:\n\n1: 3eb1958b8b ! 1: 7acc83d92b progress: add function to set total\n @@ Commit message\n Signed-off-by: Toon Claes \n \n ## progress.c ##\n -@@ progress.c: static struct progress *start_progress_delay(const char *title, uint64_t total,\n +@@ progress.c: static struct progress *start_progress_delay(struct repository *r,\n \treturn progress;\n }\n \n @@ progress.h: void progress_test_force_update(void);\n void display_throughput(struct progress *progress, uint64_t total);\n void display_progress(struct progress *progress, uint64_t n);\n +void progress_set_total(struct progress *progress, uint64_t total);\n - struct progress *start_progress(const char *title, uint64_t total);\n - struct progress *start_sparse_progress(const char *title, uint64_t total);\n - struct progress *start_delayed_progress(const char *title, uint64_t total);\n + struct progress *start_progress(struct repository *r,\n + \t\t\t\tconst char *title, uint64_t total);\n + struct progress *start_sparse_progress(struct repository *r,\n \n ## t/helper/test-progress.c ##\n @@ t/helper/test-progress.c: int cmd__progress(int argc, const char **argv)\n \t\t\tif (*end != '\\0')\n - \t\t\t\tdie(\"invalid input: '%s'\\n\", line.buf);\n + \t\t\t\tdie(\"invalid input: '%s'\", line.buf);\n \t\t\tdisplay_progress(progress, item_count);\n +\t\t} else if (skip_prefix(line.buf, \"total \", (const char **) &end)) {\n +\t\t\tuint64_t total = strtoull(end, &end, 10);\n-: ---------- > 2: 7a81cda492 progress: allow pure-throughput progress meters\n-: ---------- > 3: 86a9120539 http: turn off curl signals\n2: b74ce999a1 ! 4: 3a73d68448 http: add the ability to log progress\n @@ http.c: static void http_opt_request_remainder(CURL *curl, off_t pos)\n #define HTTP_REQUEST_FILE\t1\n \n +static int http_progress_callback(void *clientp, curl_off_t dltotal,\n -+\t\t\t\t curl_off_t dlnow, curl_off_t ultotal,\n -+\t\t\t\t curl_off_t ulnow)\n ++\t\t\t\t curl_off_t dlnow, curl_off_t ultotal UNUSED,\n ++\t\t\t\t curl_off_t ulnow UNUSED)\n +{\n +\tstruct progress *progress = clientp;\n +\n @@ http.c: static int http_request(const char *url,\n \t http_follow_config == HTTP_FOLLOW_INITIAL)\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n +\tif (options && options->progress) {\n -+\t\tprogress = start_progress(_(\"Downloading via HTTP\"), 0);\n ++\t\tprogress = start_progress(the_repository, _(\"Downloading via HTTP\"), 0);\n +\n +\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 0L);\n +\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, progress);\n @@ http.c: static int http_request(const char *url,\n \tret = run_one_slot(slot, &results);\n \n +\tif (progress) {\n ++\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 1L);\n +\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, NULL);\n ++\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, NULL);\n +\t\tstop_progress(&progress);\n +\t}\n +\n3: 5b5e7e86c5 ! 5: 01b2a0042f remote-curl: optionally show progress for HTTP get\n @@ t/t5557-http-get.sh: test_expect_success 'get by URL: 200' '\n +\tEOF\n +\n +\tgit remote-http $url err &&\n -+ test_grep \"^Downloading via HTTP: 100%\" err\n ++\ttest_grep \"^Downloading via HTTP: 100%\" err\n +'\n +\n test_done\n4: 98ef4dfade ! 6: 3df90aa17b bundle-uri: enable git-remote-https progress\n @@ bundle-uri.c: static int download_https_uri_to_file(const char *file, const char\n \t}\n \n +\tfprintf(child_in, \"option progress true\\n\");\n -+\tfflush(child_in);\n -+\n \tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n \n cleanup:\n-: ---------- > 7: c30c3bf4fe http: silence stderr when progress is enabled\n\n\n---\n\nbase-commit: a554262210b4a2ee6fa2d594e1f09f5830888c56\nchange-id: 20250219-toon-bundleuri-progress-3d902efeba06\n\nThanks\n--\nToon\n\n"},{"id":"512703","messageId":"20250219-toon-bundleuri-progress-v2-1-a84e7ffa921a@iotcl.com","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-0-a84e7ffa921a@iotcl.com","subject":"[PATCH v2 1/7] progress: add function to set total","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-19T14:30:19Z","receivedAt":"2025-02-19T14:30:29Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"We're about to add the use of progress through curl. Although, curl\ndoesn't know the total at the start of the download, but might receive\nthis information in the Content-Length header when the download starts.\n\nTo allow users set the total size after calling start_progress(), add a\nfunction progress_set_total().\n\nSigned-off-by: Toon Claes \n---\n progress.c | 6 ++++++\n progress.h | 1 +\n t/helper/test-progress.c | 5 +++++\n t/t0500-progress-display.sh | 24 ++++++++++++++++++++++++\n 4 files changed, 36 insertions(+)\n\ndiff --git a/progress.c b/progress.c\nindex 8d5ae70f3a..e254877d2c 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -276,6 +276,12 @@ static struct progress *start_progress_delay(struct repository *r,\n \treturn progress;\n }\n \n+void progress_set_total(struct progress *progress, uint64_t total)\n+{\n+\tif (progress)\n+\t\tprogress->total = total;\n+}\n+\n static int get_default_delay(void)\n {\n \tstatic int delay_in_secs = -1;\ndiff --git a/progress.h b/progress.h\nindex ed068c7bab..2e1bd738c2 100644\n--- a/progress.h\n+++ b/progress.h\n@@ -15,6 +15,7 @@ void progress_test_force_update(void);\n \n void display_throughput(struct progress *progress, uint64_t total);\n void display_progress(struct progress *progress, uint64_t n);\n+void progress_set_total(struct progress *progress, uint64_t total);\n struct progress *start_progress(struct repository *r,\n \t\t\t\tconst char *title, uint64_t total);\n struct progress *start_sparse_progress(struct repository *r,\ndiff --git a/t/helper/test-progress.c b/t/helper/test-progress.c\nindex 1f75b7bd19..3a73d6fe0a 100644\n--- a/t/helper/test-progress.c\n+++ b/t/helper/test-progress.c\n@@ -74,6 +74,11 @@ int cmd__progress(int argc, const char **argv)\n \t\t\tif (*end != '\\0')\n \t\t\t\tdie(\"invalid input: '%s'\", line.buf);\n \t\t\tdisplay_progress(progress, item_count);\n+\t\t} else if (skip_prefix(line.buf, \"total \", (const char **) &end)) {\n+\t\t\tuint64_t total = strtoull(end, &end, 10);\n+\t\t\tif (*end != '\\0')\n+\t\t\t\tdie(\"invalid input: '%s'\\n\", line.buf);\n+\t\t\tprogress_set_total(progress, total);\n \t\t} else if (skip_prefix(line.buf, \"throughput \",\n \t\t\t\t (const char **) &end)) {\n \t\t\tuint64_t byte_count, test_ms;\ndiff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh\nindex d1a498a216..b7ed1db3a0 100755\n--- a/t/t0500-progress-display.sh\n+++ b/t/t0500-progress-display.sh\n@@ -55,6 +55,30 @@ test_expect_success 'progress display with total' '\n \ttest_cmp expect out\n '\n \n+test_expect_success 'progress display modify total' '\n+\tcat >expect <<-\\EOF &&\n+\tWorking hard: 1\n+\tWorking hard: 66% (2/3)\n+\tWorking hard: 100% (3/3)\n+\tWorking hard: 100% (3/3), done.\n+\tEOF\n+\n+\tcat >in <<-\\EOF &&\n+\tstart 0\n+\tupdate\n+\tprogress 1\n+\tupdate\n+\ttotal 3\n+\tprogress 2\n+\tprogress 3\n+\tstop\n+\tEOF\n+\ttest-tool progress stderr &&\n+\n+\tshow_cr out &&\n+\ttest_cmp expect out\n+'\n+\n test_expect_success 'progress display breaks long lines #1' '\n \tsed -e \"s/Z$//\" >expect <<\\EOF &&\n Working hard.......2.........3.........4.........5.........6: 0% (100/100000)\n\n-- \n2.48.1.658.g4767266eb4\n\n"},{"id":"512704","messageId":"20250219-toon-bundleuri-progress-v2-2-a84e7ffa921a@iotcl.com","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-0-a84e7ffa921a@iotcl.com","subject":"[PATCH v2 2/7] progress: allow pure-throughput progress meters","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-19T14:30:20Z","receivedAt":"2025-02-19T14:30:32Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"From: Jeff King \n\nThe progress code assumes we are counting something (usually\nobjects), even if we are measuring throughput. This works\nfor fetching packfiles, since they show us the object count\nalongside the throughput, like:\n\n Receiving objects: 2% (301/11968), 22.00 MiB | 10.97 MiB/s\n\nYou can also tell the progress code you don't know how many\nitems you have (by specifying a total of 0), and it looks\nlike:\n\n Counting objects: 34957\n\nHowever, if you're fetching a single large item, you want\nthroughput but you might not have a meaningful count. You\ncan say you are getting item 0 or 1 out of 1 total, but then\nthe percent meter is misleading:\n\n Downloading: 0% (0/1), 22.00 MiB | 10.97 MiB/s\n\nor\n\n Downloading: 100% (0/1), 22.00 MiB | 10.97 MiB/s\n\nNeither of those is accurate. You are probably somewhere\nbetween zero and 100 percent through the operation, but you\ndon't know how far.\n\nTelling it you don't know how many items is even uglier:\n\n Downloading: 1, 22.00 MiB | 10.97 MiB/s\n\nInstead, this patch will omit the count entirely if you are\non the zero-th item of an unknown number of items. It looks\nlike:\n\n Downloading: 22.00 MiB | 10.97 MiB/s\n\nSigned-off-by: Jeff King \n---\n progress.c | 17 +++++++++++------\n t/t0500-progress-display.sh | 31 +++++++++++++++++++++++++++++++\n 2 files changed, 42 insertions(+), 6 deletions(-)\n\ndiff --git a/progress.c b/progress.c\nindex e254877d2c..89abb231ae 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -135,7 +135,11 @@ static void display(struct progress *progress, uint64_t n, const char *done)\n \t\t}\n \t} else if (progress_update) {\n \t\tstrbuf_reset(counters_sb);\n-\t\tstrbuf_addf(counters_sb, \"%\"PRIuMAX\"%s\", (uintmax_t)n, tp);\n+\t\tif (n > 0)\n+\t\t\tstrbuf_addf(counters_sb, \"%\" PRIuMAX \"%s\",\n+\t\t\t\t (uintmax_t)n, tp);\n+\t\telse\n+\t\t\tstrbuf_addstr(counters_sb, tp);\n \t\tshow_update = 1;\n \t}\n \n@@ -170,11 +174,12 @@ static void display(struct progress *progress, uint64_t n, const char *done)\n \t}\n }\n \n-static void throughput_string(struct strbuf *buf, uint64_t total,\n-\t\t\t unsigned int rate)\n+static void throughput_string(struct progress *progress, struct strbuf *buf,\n+\t\t\t uint64_t total, unsigned int rate)\n {\n \tstrbuf_reset(buf);\n-\tstrbuf_addstr(buf, \", \");\n+\tif (progress->total || progress->last_value > 0)\n+\t\tstrbuf_addstr(buf, \", \");\n \tstrbuf_humanise_bytes(buf, total);\n \tstrbuf_addstr(buf, \" | \");\n \tstrbuf_humanise_rate(buf, rate * 1024);\n@@ -243,7 +248,7 @@ void display_throughput(struct progress *progress, uint64_t total)\n \ttp->last_misecs[tp->idx] = misecs;\n \ttp->idx = (tp->idx + 1) % TP_IDX_MAX;\n \n-\tthroughput_string(&tp->display, total, rate);\n+\tthroughput_string(progress, &tp->display, total, rate);\n \tif (progress->last_value != -1 && progress_update)\n \t\tdisplay(progress, progress->last_value, NULL);\n }\n@@ -343,7 +348,7 @@ static void force_last_update(struct progress *progress, const char *msg)\n \t\tunsigned int misecs, rate;\n \t\tmisecs = ((now_ns - progress->start_ns) * 4398) >> 32;\n \t\trate = tp->curr_total / (misecs ? misecs : 1);\n-\t\tthroughput_string(&tp->display, tp->curr_total, rate);\n+\t\tthroughput_string(progress, &tp->display, tp->curr_total, rate);\n \t}\n \tprogress_update = 1;\n \tbuf = xstrfmt(\", %s.\\n\", msg);\ndiff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh\nindex b7ed1db3a0..582b9fa899 100755\n--- a/t/t0500-progress-display.sh\n+++ b/t/t0500-progress-display.sh\n@@ -263,6 +263,37 @@ test_expect_success 'progress display with throughput and total' '\n \ttest_cmp expect out\n '\n \n+test_expect_success 'progress display throughput without total' '\n+\tcat >expect <<-\\EOF &&\n+\tWorking hard: \n+\tWorking hard: 200.00 KiB | 100.00 KiB/s\n+\tWorking hard: 300.00 KiB | 100.00 KiB/s\n+\tWorking hard: 400.00 KiB | 100.00 KiB/s\n+\tWorking hard: 400.00 KiB | 100.00 KiB/s, done.\n+\tEOF\n+\n+\tcat >in <<-\\EOF &&\n+\tstart 0\n+\tthroughput 102400 1000\n+\tupdate\n+\tprogress 0\n+\tthroughput 204800 2000\n+\tupdate\n+\tprogress 0\n+\tthroughput 307200 3000\n+\tupdate\n+\tprogress 0\n+\tthroughput 409600 4000\n+\tupdate\n+\tprogress 0\n+\tstop\n+\tEOF\n+\ttest-tool progress stderr &&\n+\n+\tshow_cr out &&\n+\ttest_cmp expect out\n+'\n+\n test_expect_success 'cover up after throughput shortens' '\n \tcat >expect <<-\\EOF &&\n \tWorking hard: 1\n\n-- \n2.48.1.658.g4767266eb4\n\n"},{"id":"512705","messageId":"20250219-toon-bundleuri-progress-v2-3-a84e7ffa921a@iotcl.com","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-0-a84e7ffa921a@iotcl.com","subject":"[PATCH v2 3/7] http: turn off curl signals","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-19T14:30:21Z","receivedAt":"2025-02-19T14:30:36Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"From: Jeff King \n\nCurl sets and clears the handler for SIGALRM, which makes it\nincompatible with git's progress code. However, we can ask\ncurl not to do this.\n\nSigned-off-by: Jeff King \n---\n http.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex f4504133e8..38c7c0cd54 100644\n--- a/http.c\n+++ b/http.c\n@@ -1245,6 +1245,8 @@ static CURL *get_curl_handle(void)\n \n \tset_curl_keepalive(result);\n \n+\tcurl_easy_setopt(result, CURLOPT_NOSIGNAL, 1);\n+\n \treturn result;\n }\n \n\n-- \n2.48.1.658.g4767266eb4\n\n"},{"id":"512706","messageId":"20250219-toon-bundleuri-progress-v2-4-a84e7ffa921a@iotcl.com","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-0-a84e7ffa921a@iotcl.com","subject":"[PATCH v2 4/7] http: add the ability to log progress","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-19T14:30:22Z","receivedAt":"2025-02-19T14:30:39Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Add an option `progress` to `struct http_get_options` to allow the\ncaller to enable download progress using the progress.c API.\n\nSigned-off-by: Toon Claes \n---\n http.c | 34 ++++++++++++++++++++++++++++++++++\n http.h | 5 +++++\n 2 files changed, 39 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex 38c7c0cd54..5517863808 100644\n--- a/http.c\n+++ b/http.c\n@@ -13,6 +13,7 @@\n #include \"credential.h\"\n #include \"version.h\"\n #include \"pkt-line.h\"\n+#include \"progress.h\"\n #include \"gettext.h\"\n #include \"trace.h\"\n #include \"transport.h\"\n@@ -1504,6 +1505,9 @@ struct active_request_slot *get_active_slot(void)\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);\n \tcurl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 1L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, NULL);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, NULL);\n \n \t/*\n \t * Default following to off unless \"ALWAYS\" is configured; this gives\n@@ -2068,6 +2072,21 @@ static void http_opt_request_remainder(CURL *curl, off_t pos)\n #define HTTP_REQUEST_STRBUF\t0\n #define HTTP_REQUEST_FILE\t1\n \n+static int http_progress_callback(void *clientp, curl_off_t dltotal,\n+\t\t\t\t curl_off_t dlnow, curl_off_t ultotal UNUSED,\n+\t\t\t\t curl_off_t ulnow UNUSED)\n+{\n+\tstruct progress *progress = clientp;\n+\n+\tif (progress) {\n+\t\tprogress_set_total(progress, dltotal);\n+\t\tdisplay_progress(progress, dlnow);\n+\t\tdisplay_throughput(progress, dlnow);\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int http_request(const char *url,\n \t\t\tvoid *result, int target,\n \t\t\tconst struct http_get_options *options)\n@@ -2076,6 +2095,7 @@ static int http_request(const char *url,\n \tstruct slot_results results;\n \tstruct curl_slist *headers = http_copy_default_headers();\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct progress *progress = NULL;\n \tconst char *accept_language;\n \tint ret;\n \n@@ -2112,6 +2132,13 @@ static int http_request(const char *url,\n \tif (options && options->initial_request &&\n \t http_follow_config == HTTP_FOLLOW_INITIAL)\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n+\tif (options && options->progress) {\n+\t\tprogress = start_progress(the_repository, _(\"Downloading via HTTP\"), 0);\n+\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 0L);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, progress);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, &http_progress_callback);\n+\t}\n \n \theaders = curl_slist_append(headers, buf.buf);\n \n@@ -2134,6 +2161,13 @@ static int http_request(const char *url,\n \n \tret = run_one_slot(slot, &results);\n \n+\tif (progress) {\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 1L);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, NULL);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFOFUNCTION, NULL);\n+\t\tstop_progress(&progress);\n+\t}\n+\n \tif (options && options->content_type) {\n \t\tstruct strbuf raw = STRBUF_INIT;\n \t\tcurlinfo_strbuf(slot->curl, CURLINFO_CONTENT_TYPE, &raw);\ndiff --git a/http.h b/http.h\nindex 36202139f4..09ebbdfefe 100644\n--- a/http.h\n+++ b/http.h\n@@ -146,6 +146,11 @@ struct http_get_options {\n \t * request has completed.\n \t */\n \tstruct string_list *extra_headers;\n+\n+\t/*\n+\t * If not zero, display the progress.\n+\t */\n+\tint progress;\n };\n \n /* Return values for http_get_*() */\n\n-- \n2.48.1.658.g4767266eb4\n\n"},{"id":"512707","messageId":"20250219-toon-bundleuri-progress-v2-5-a84e7ffa921a@iotcl.com","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-0-a84e7ffa921a@iotcl.com","subject":"[PATCH v2 5/7] remote-curl: optionally show progress for HTTP get","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-19T14:30:23Z","receivedAt":"2025-02-19T14:30:41Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"git-remote-curl supports the `option progress` basically since it's\ninception. But this option had no effect for regular HTTP(S) downloads.\n\nAdd progress indicator when downloading files through curl HTTP GET.\n\nSigned-off-by: Toon Claes \n---\n remote-curl.c | 8 +++++++-\n t/t5557-http-get.sh | 15 +++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 1273507a96..f710d6b3cb 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -1317,6 +1317,7 @@ static void parse_get(const char *arg)\n {\n \tstruct strbuf url = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n+\tstruct http_get_options http_options = {0};\n \tconst char *space;\n \n \tspace = strchr(arg, ' ');\n@@ -1327,7 +1328,12 @@ static void parse_get(const char *arg)\n \tstrbuf_add(&url, arg, space - arg);\n \tstrbuf_addstr(&path, space + 1);\n \n-\tif (http_get_file(url.buf, path.buf, NULL))\n+\thttp_options.initial_request = 1;\n+\n+\tif (options.progress)\n+\t\thttp_options.progress = 1;\n+\n+\tif (http_get_file(url.buf, path.buf, &http_options))\n \t\tdie(_(\"failed to download file at URL '%s'\"), url.buf);\n \n \tstrbuf_release(&url);\ndiff --git a/t/t5557-http-get.sh b/t/t5557-http-get.sh\nindex 67fcc23f11..41f3d16ef9 100755\n--- a/t/t5557-http-get.sh\n+++ b/t/t5557-http-get.sh\n@@ -35,4 +35,19 @@ test_expect_success 'get by URL: 200' '\n \ttest_cmp \"$HTTPD_DOCUMENT_ROOT_PATH/exists.txt\" file2\n '\n \n+test_expect_success 'get by URL with progress' '\n+\techo hello >\"$HTTPD_DOCUMENT_ROOT_PATH/hello.txt\" &&\n+\n+\turl=\"$HTTPD_URL/hello.txt\" &&\n+\tcat >input <<-EOF &&\n+\tcapabilities\n+\toption progress true\n+\tget $url file3\n+\n+\tEOF\n+\n+\tgit remote-http $url err &&\n+\ttest_grep \"^Downloading via HTTP: 100%\" err\n+'\n+\n test_done\n\n-- \n2.48.1.658.g4767266eb4\n\n"},{"id":"512708","messageId":"20250219-toon-bundleuri-progress-v2-6-a84e7ffa921a@iotcl.com","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-0-a84e7ffa921a@iotcl.com","subject":"[PATCH v2 6/7] bundle-uri: enable git-remote-https progress","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-19T14:30:24Z","receivedAt":"2025-02-19T14:30:45Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"When using bundle URIs large files might get downloaded during clone.\nDuring that time, there's no feedback to the user what is happening.\n\nEnable HTTP download progress for bundle URIs to inform the user.\n\nSigned-off-by: Toon Claes \n---\n bundle-uri.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex 744257c49c..0ef96cd00f 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -298,7 +298,6 @@ static int download_https_uri_to_file(const char *file, const char *uri)\n \tint found_get = 0;\n \n \tstrvec_pushl(&cp.args, \"git-remote-https\", uri, NULL);\n-\tcp.err = -1;\n \tcp.in = -1;\n \tcp.out = -1;\n \n@@ -333,6 +332,7 @@ static int download_https_uri_to_file(const char *file, const char *uri)\n \t\tgoto cleanup;\n \t}\n \n+\tfprintf(child_in, \"option progress true\\n\");\n \tfprintf(child_in, \"get %s %s\\n\\n\", uri, file);\n \n cleanup:\n\n-- \n2.48.1.658.g4767266eb4\n\n"},{"id":"512709","messageId":"20250219-toon-bundleuri-progress-v2-7-a84e7ffa921a@iotcl.com","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-0-a84e7ffa921a@iotcl.com","subject":"[PATCH v2 7/7] http: silence stderr when progress is enabled","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-02-19T14:30:25Z","receivedAt":"2025-02-19T14:30:49Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"To download bundle URI bundles over HTTP(s), git-clone(1) spawns a\ngit-remote-http(1) subprocess. Because clone can continue without\nbundles, all errors sent by the child process over stderr are\nsuppressed.\n\nIn previous commits, we've added progress output in the child process,\nand this happens over stderr, so we can no longer silence stderr. But in\ncase a bundle could not be downloaded, the user sees the following\nmessages:\n\n fatal: failed to download file at URL 'http://127.0.0.1:5558/bundle-5.bundle'\n warning: failed to download bundle from URI 'http://127.0.0.1:5558/bundle-5.bundle'\n\nHere the child git-remote-http(1) prints a \"fatal\" error and then the\nparent git-clone(1) prints a \"warning\". This is confusing to the user.\n\nInstead of suppressing stderr from the parent process, like we did\nbefore, modify stderr to write to /dev/null in the child process itself,\nwhile keep using the original stderr for progress logging only.\n\nSigned-off-by: Toon Claes \n---\n http.c | 5 ++++-\n progress.c | 17 +++++++++++++----\n progress.h | 1 +\n 3 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 5517863808..5c0c6ef204 100644\n--- a/http.c\n+++ b/http.c\n@@ -2133,7 +2133,10 @@ static int http_request(const char *url,\n \t http_follow_config == HTTP_FOLLOW_INITIAL)\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n \tif (options && options->progress) {\n-\t\tprogress = start_progress(the_repository, _(\"Downloading via HTTP\"), 0);\n+\t\tprogress = start_progress(the_repository,\n+\t\t\t\t\t _(\"Downloading via HTTP\"), 0);\n+\t\tprogress_set_fd(progress, fileno(stderr));\n+\t\tfreopen(\"/dev/null\", \"w\", stderr);\n \n \t\tcurl_easy_setopt(slot->curl, CURLOPT_NOPROGRESS, 0L);\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_XFERINFODATA, progress);\ndiff --git a/progress.c b/progress.c\nindex 89abb231ae..1955262000 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -40,6 +40,7 @@ struct progress {\n \tconst char *title;\n \tuint64_t last_value;\n \tuint64_t total;\n+\tint fd;\n \tunsigned last_percent;\n \tunsigned delay;\n \tunsigned sparse;\n@@ -144,7 +145,9 @@ static void display(struct progress *progress, uint64_t n, const char *done)\n \t}\n \n \tif (show_update) {\n-\t\tif (is_foreground_fd(fileno(stderr)) || done) {\n+\t\tint fd = progress->fd ? progress->fd : fileno(stderr);\n+\n+\t\tif (is_foreground_fd(fd) || done) {\n \t\t\tconst char *eol = done ? done : \"\\r\";\n \t\t\tsize_t clear_len = counters_sb->len < last_count_len ?\n \t\t\t\t\tlast_count_len - counters_sb->len + 1 :\n@@ -155,17 +158,17 @@ static void display(struct progress *progress, uint64_t n, const char *done)\n \t\t\tint cols = term_columns();\n \n \t\t\tif (progress->split) {\n-\t\t\t\tfprintf(stderr, \" %s%*s\", counters_sb->buf,\n+\t\t\t\tdprintf(fd, \" %s%*s\", counters_sb->buf,\n \t\t\t\t\t(int) clear_len, eol);\n \t\t\t} else if (!done && cols < progress_line_len) {\n \t\t\t\tclear_len = progress->title_len + 1 < cols ?\n \t\t\t\t\t cols - progress->title_len - 1 : 0;\n-\t\t\t\tfprintf(stderr, \"%s:%*s\\n %s%s\",\n+\t\t\t\tdprintf(fd, \"%s:%*s\\n %s%s\",\n \t\t\t\t\tprogress->title, (int) clear_len, \"\",\n \t\t\t\t\tcounters_sb->buf, eol);\n \t\t\t\tprogress->split = 1;\n \t\t\t} else {\n-\t\t\t\tfprintf(stderr, \"%s: %s%*s\", progress->title,\n+\t\t\t\tdprintf(fd, \"%s: %s%*s\", progress->title,\n \t\t\t\t\tcounters_sb->buf, (int) clear_len, eol);\n \t\t\t}\n \t\t\tfflush(stderr);\n@@ -287,6 +290,12 @@ void progress_set_total(struct progress *progress, uint64_t total)\n \t\tprogress->total = total;\n }\n \n+void progress_set_fd(struct progress *progress, int fd)\n+{\n+\tif (progress)\n+\t\tprogress->fd = fd;\n+}\n+\n static int get_default_delay(void)\n {\n \tstatic int delay_in_secs = -1;\ndiff --git a/progress.h b/progress.h\nindex 2e1bd738c2..f12c82adc4 100644\n--- a/progress.h\n+++ b/progress.h\n@@ -16,6 +16,7 @@ void progress_test_force_update(void);\n void display_throughput(struct progress *progress, uint64_t total);\n void display_progress(struct progress *progress, uint64_t n);\n void progress_set_total(struct progress *progress, uint64_t total);\n+void progress_set_fd(struct progress *progress, int fd);\n struct progress *start_progress(struct repository *r,\n \t\t\t\tconst char *title, uint64_t total);\n struct progress *start_sparse_progress(struct repository *r,\n\n-- \n2.48.1.658.g4767266eb4\n\n"},{"id":"512791","messageId":"20250221073605.GA1988395@coredump.intra.peff.net","threadId":"61434","inReplyTo":"87o6z43gz8.fsf@iotcl.com","subject":"Re: [PATCH 4/4] bundle-uri: enable git-remote-https progress","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-02-21T07:36:05Z","receivedAt":"2025-02-21T07:36:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 14, 2025 at 12:26:03PM +0100, Toon Claes wrote:\n\n> I've been playing around with things and I haven't found a good way\n> forward with this. We could have the parent process ingest stderr of\n> git-remote-https and swallow messages that match `/^fatal:/`, but that\n> feels like a hack and not foolproof.\n\nYeah, agree that feels quite hacky (and you'd probably want to swallow\n/^error:/, /^warning:/, etc, too).\n\n> I was thinking if we could override `die()` in the child process to have\n> it not print anything, but because git-remote-http basically can call\n> die() basically from anywhere in the codebase, I don't think we can\n> ensure the silenced die() function is called.\n> \n> Or what do you mean by \"squelch non-progress errors\"?\n\nI was thinking of having some kind of \"very quiet\" mode where\ngit-remote-http would not print any errors (except for progress). But I\nagree that doing it is non-trivial. Our die/warning/error functions are\nall pluggable, so remote-http could add its own implementations using\nset_die_routine(), etc.\n\nBut that does feel pretty heavyweight, and you'd still have to pass\nthrough the \"please suppress all your die calls\" option into\nremote-http. Plus it wouldn't catch any spots in the code that happen to\ncall fprintf(stderr), etc.\n\n> And yes, sending progress logging over a separate fd seems like the\n> ideal approach, but I haven't tried it yet. I'm afraid it's not worth\n> attempting so.\n> \n> So I think that leaves us with your suggestion to \"ferry\n> machine-readable output back to the parent\". If I understand correctly\n> you mean the child process will not write progress logging to stderr but\n> to stdout (with some kind of command prefix the parent process knows\n> what to do with this)?\n\nExactly.\n\n> I imagine communication between parent and child will then look\n> something like this:\n> \n> -> capabilities\n> <- stateless-connect\n> <- fetch\n> <- get\n> <- option\n> <- push\n> <- check-connectivity\n> <- object-format\n> -> option progress true\n> <- ok\n> -> get http://example.com git.bundle\n> <- progress 123 345 40\n> <- progress 234 345 50\n> <- progress 345 345 40\n> \n> ~fin~\n> \n> But then we need to decide on the format the child sends back to the\n> parent. In the above example it's something like `progress \n> `. An alternative proposal could be:\n> \n> <- log Downloading via HTTP: \n> <- log Downloading via HTTP: 200.00 KiB | 100.00 KiB/s\n> <- log Downloading via HTTP: 300.00 KiB | 100.00 KiB/s\n> <- log Downloading via HTTP: 400.00 KiB | 100.00 KiB/s\n> <- log Downloading via HTTP: 400.00 KiB | 100.00 KiB/s, done.\n> \n> So the child sends the progress text with a `log` prefix, which the\n> parent simply has to send that logging to where it wants it to go.\n> \n> Or am I completely misunderstanding your proposal? Do you maybe happen\n> to have any examples of a similar solution?\n\nNo, I think you understand it perfectly.\n\nYour \"log\" example with arbitrary text seems like the simplest approach,\nand might be enough. Then we wouldn't have to define a schema for\nprogress numbers (and we have at least two types of progress: counts and\nthroughput, though I guess maybe this would always be throughput?).\n\nBut I do wonder if we'd want the flexibility of the machine-readable\nnumbers. In particular, would we ever fetch multiple bundle-uri files in\nparallel? If so, then we wouldn't want them stomping on each other's\nprogress. You'd probably want the caller to present a unified view based\non the progress reports from all of the child processes.\n\n-Peff\n"},{"id":"512792","messageId":"20250221074308.GB1988395@coredump.intra.peff.net","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-1-a84e7ffa921a@iotcl.com","subject":"Re: [PATCH v2 1/7] progress: add function to set total","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-02-21T07:43:08Z","receivedAt":"2025-02-21T07:43:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 19, 2025 at 03:30:19PM +0100, Toon Claes wrote:\n\n> We're about to add the use of progress through curl. Although, curl\n> doesn't know the total at the start of the download, but might receive\n> this information in the Content-Length header when the download starts.\n> \n> To allow users set the total size after calling start_progress(), add a\n> function progress_set_total().\n\nMakes sense.\n\n> +void progress_set_total(struct progress *progress, uint64_t total)\n> +{\n> +\tif (progress)\n> +\t\tprogress->total = total;\n> +}\n\nI wondered if we'd need to do any other computation here (that would\nhave been done in start_progress() if we specified the total then). But\nit looks like we don't look at progress->total until we're ready to\ndisplay something.\n\nWould we want to call display_progress() or similar here, to update the\noriginal view like:\n\n Working hard: 1\n\nto:\n\n Working hard: 33% (1/3)\n\nimmediately, rather than waiting for more progress to be made? I guess\nit probably doesn't matter that much in practice as we'd remain stale\nfor a brief period in most cases (particularly for transfers where we're\nupdating based on bytes received, which is a pretty small unit of work).\n\nPlus I think it may be awkward, because we don't know whether to call\ndisplay_progress() or display_throughput().\n\n-Peff\n"},{"id":"512803","messageId":"20250221074854.GC1988395@coredump.intra.peff.net","threadId":"61434","inReplyTo":"20250219-toon-bundleuri-progress-v2-7-a84e7ffa921a@iotcl.com","subject":"Re: [PATCH v2 7/7] http: silence stderr when progress is enabled","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-02-21T07:48:54Z","receivedAt":"2025-02-21T07:48:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 19, 2025 at 03:30:25PM +0100, Toon Claes wrote:\n\n> diff --git a/http.c b/http.c\n> index 5517863808..5c0c6ef204 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -2133,7 +2133,10 @@ static int http_request(const char *url,\n> \t http_follow_config == HTTP_FOLLOW_INITIAL)\n> \t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n> \tif (options && options->progress) {\n> -\t\tprogress = start_progress(the_repository, _(\"Downloading via HTTP\"), 0);\n> +\t\tprogress = start_progress(the_repository,\n> +\t\t\t\t\t _(\"Downloading via HTTP\"), 0);\n> +\t\tprogress_set_fd(progress, fileno(stderr));\n> +\t\tfreopen(\"/dev/null\", \"w\", stderr);\n\nHmm. I can't think of any reason this wouldn't work, and it certainly is\nless ugly than overriding die() and friends.\n\nIt does still feel a bit hacky to me. For one thing, asking for progress\ndoes not necessarily mean you _also_ want to suppress errors. So it\nwould have to be a separate option.\n\nBut mostly the more I think about it, the more the \"send progress data\nback to the caller over stdout\" thing makes sense. I think we probably\ndo eventually want to support parallel fetches, which would require\ncoordination in the caller.\n\n-Peff\n"}]}