Re: [PATCH v2] fetch: show progress for packfile uri downloads
From: Junio C Hamano <hidden>
Date: 2021-04-11 01:31:01
"Albert Cui via GitGitGadget" [off-list ref] writes:
From: Albert Cui <redacted> Git appears to hang when downloading packfiles as this part of the fetch is silent, causing user confusion. This change implements progress for the number of packfiles downloaded; a progress display for bytes would involve deeper changes at the http-fetch layer instead of fetch-pack, the caller, so we do not do that in this patch.
... meaning, hopefully later we'd hook into transport->progress and implement the byte-level progress display down there? And when that happens, we'd remove this file-level progress as it would be too confusing to have both at the same time? Is this start_progress() call a way to unconditionally enable the progress display? How does it interact with transport->progress that is driven by transport_set_verbosity(), which in turn is called by builtin/fetch.c and friends? If it doesn't, shouldn't this codepath pay attention to the transport->progress and enable the progress meter only when it is enabled (i.e. the stderr going to a terminal, or --progress explicitly being asked)?
quoted hunk
@@ -1585,6 +1586,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, struct fetch_negotiator *negotiator; int seen_ack = 0; struct string_list packfile_uris = STRING_LIST_INIT_DUP; + struct progress *packfile_uri_progress; int i; struct strvec index_pack_args = STRVEC_INIT; struct oidset gitmodules_oids = OIDSET_INIT;@@ -1689,6 +1691,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, } } + packfile_uri_progress = start_progress(_("Downloading packs"), packfile_uris.nr); + for (i = 0; i < packfile_uris.nr; i++) { int j; struct child_process cmd = CHILD_PROCESS_INIT;@@ -1696,6 +1700,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, const char *uri = packfile_uris.items[i].string + the_hash_algo->hexsz + 1; + display_progress(packfile_uri_progress, i + 1); strvec_push(&cmd.args, "http-fetch"); strvec_pushf(&cmd.args, "--packfile=%.*s", (int) the_hash_algo->hexsz,@@ -1739,6 +1744,9 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, get_object_directory(), packname)); } + + stop_progress(&packfile_uri_progress); + string_list_clear(&packfile_uris, 0); strvec_clear(&index_pack_args);diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 2e1243ca40b0..0476b3f50455 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh@@ -848,10 +848,12 @@ test_expect_success 'part of packfile response provided as URI' ' configure_exclusion "$P" my-blob >h && configure_exclusion "$P" other-blob >h2 && - GIT_TRACE=1 GIT_TRACE_PACKET="$(pwd)/log" GIT_TEST_SIDEBAND_ALL=1 \ + GIT_PROGRESS_DELAY=0 GIT_TRACE=1 GIT_TRACE2_EVENT=1 \ + GIT_TRACE_PACKET="$(pwd)/log" GIT_TEST_SIDEBAND_ALL=1 \ git -c protocol.version=2 \ -c fetch.uriprotocols=http,https \ - clone "$HTTPD_URL/smart/http_parent" http_child && + clone "$HTTPD_URL/smart/http_parent" http_child \ + --progress 2>progress && # Ensure that my-blob and other-blob are in separate packfiles. for idx in http_child/.git/objects/pack/*.idx@@ -875,6 +877,8 @@ test_expect_success 'part of packfile response provided as URI' ' test -f hfound && test -f h2found && + test_i18ngrep "Downloading packs" progress && + # Ensure that there are exactly 3 packfiles with associated .idx ls http_child/.git/objects/pack/*.pack \ http_child/.git/objects/pack/*.idx >filelist &&base-commit: a5828ae6b52137b913b978e16cd2334482eb4c1f