[v5,12/12] bundle-uri: suppress stderr from remote-https

Commit Message

Derrick Stolee Oct. 12, 2022, 12:52 p.m. UTC

From: Derrick Stolee <[e-mail]>

When downloading bundles from a git-remote-https subprocess, the bundle
URI logic wants to be opportunistic and download as much as possible and
work with what did succeed. This is particularly important in the "any"
mode, where any single bundle success will work.

If the URI is not available, the git-remote-https process will die()
with a "fatal:" error message, even though that error is not actually
fatal to the super process. Since stderr is passed through, it looks
like a fatal error to the user.

Suppress stderr to avoid these errors from bubbling to the surface. The
bundle URI API adds its own warning() messages on these failures.

Signed-off-by: Derrick Stolee <[e-mail]>
---
 bundle-uri.c                |  1 +
 t/t5558-clone-bundle-uri.sh | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

Comments

Junio C Hamano Oct. 26, 2022, 6:54 p.m. UTC | #1

"Derrick Stolee via GitGitGadget" <[e-mail]> writes:

> From: Derrick Stolee <[e-mail]>
>
> When downloading bundles from a git-remote-https subprocess, the bundle
> URI logic wants to be opportunistic and download as much as possible and
> work with what did succeed. This is particularly important in the "any"
> mode, where any single bundle success will work.
>
> If the URI is not available, the git-remote-https process will die()
> with a "fatal:" error message, even though that error is not actually
> fatal to the super process. Since stderr is passed through, it looks
> like a fatal error to the user.
>
> Suppress stderr to avoid these errors from bubbling to the surface. The
> bundle URI API adds its own warning() messages on these failures.
>
> Signed-off-by: Derrick Stolee <[e-mail]>
> ---
>  bundle-uri.c                |  1 +
>  t/t5558-clone-bundle-uri.sh | 16 ++++++++++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)

So this is the same in spirit as [11/12] to squelch errors from an
action that we are prepared to fail.  If we had an easy way to
squelch only one class of errors (e.g. the resource no longer exists
at the URI) while allowing others to pass (e.g. we downloaded but it
was corrupt), that might be even better when somebody is debugging
the thing, but to an end user, it is a hopefully ignorable failure
either way, as long as there are other alternatives in the set of
bundles in the "any" mode.

Patch

diff --git a/bundle-uri.c b/bundle-uri.c
index d872acf5ab0..79a914f961b 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -230,6 +230,7 @@  static int download_https_uri_to_file(const char *file, const char *uri)
 	int found_get = 0;
 
 	strvec_pushl(&cp.args, "git-remote-https", uri, NULL);
+	cp.err = -1;
 	cp.in = -1;
 	cp.out = -1;
 
diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh
index 9b159078386..9155f31fa2c 100755
--- a/t/t5558-clone-bundle-uri.sh
+++ b/t/t5558-clone-bundle-uri.sh
@@ -147,6 +147,8 @@  test_expect_success 'clone bundle list (file, all mode, some failures)' '
 	git clone --bundle-uri="file://$(pwd)/bundle-list" \
 		clone-from clone-all-some 2>err &&
 	! grep "Repository lacks these prerequisite commits" err &&
+	! grep "fatal" err &&
+	grep "warning: failed to download bundle from URI" err &&
 
 	git -C clone-from for-each-ref --format="%(objectname)" >oids &&
 	git -C clone-all-some cat-file --batch-check <oids &&
@@ -178,6 +180,8 @@  test_expect_success 'clone bundle list (file, all mode, all failures)' '
 	git clone --bundle-uri="file://$(pwd)/bundle-list" \
 		clone-from clone-all-fail 2>err &&
 	! grep "Repository lacks these prerequisite commits" err &&
+	! grep "fatal" err &&
+	grep "warning: failed to download bundle from URI" err &&
 
 	git -C clone-from for-each-ref --format="%(objectname)" >oids &&
 	git -C clone-all-fail cat-file --batch-check <oids &&
@@ -234,7 +238,11 @@  test_expect_success 'clone bundle list (file, any mode, all failures)' '
 		uri = $HTTPD_URL/bundle-5.bundle
 	EOF
 
-	git clone --bundle-uri="file://$(pwd)/bundle-list" clone-from clone-any-fail &&
+	git clone --bundle-uri="file://$(pwd)/bundle-list" \
+		clone-from clone-any-fail 2>err &&
+	! grep "fatal" err &&
+	grep "warning: failed to download bundle from URI" err &&
+
 	git -C clone-from for-each-ref --format="%(objectname)" >oids &&
 	git -C clone-any-fail cat-file --batch-check <oids &&
 
@@ -323,7 +331,11 @@  test_expect_success 'clone bundle list (HTTP, any mode)' '
 		uri = $HTTPD_URL/bundle-5.bundle
 	EOF
 
-	git clone --bundle-uri="$HTTPD_URL/bundle-list" clone-from clone-any-http &&
+	git clone --bundle-uri="$HTTPD_URL/bundle-list" \
+		clone-from clone-any-http 2>err &&
+	! grep "fatal" err &&
+	grep "warning: failed to download bundle from URI" err &&
+
 	git -C clone-from for-each-ref --format="%(objectname)" >oids &&
 	git -C clone-any-http cat-file --batch-check <oids &&