From: Jeff King <peff@peff.net>
Date: Fri, 21 Feb 2025 07:48:54 GMT
Subject: Re: [PATCH v2 7/7] http: silence stderr when progress is enabled
Message-ID: <20250221074854.GC1988395@coredump.intra.peff.net>
In-Reply-To: <20250219-toon-bundleuri-progress-v2-7-a84e7ffa921a@iotcl.com>

On Wed, Feb 19, 2025 at 03:30:25PM +0100, Toon Claes wrote:

> diff --git a/http.c b/http.c
> index 5517863808..5c0c6ef204 100644
> --- a/http.c
> +++ b/http.c
> @@ -2133,7 +2133,10 @@ static int http_request(const char *url,
>  	    http_follow_config == HTTP_FOLLOW_INITIAL)
>  		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);
>  	if (options && options->progress) {
> -		progress = start_progress(the_repository, _("Downloading via HTTP"), 0);
> +		progress = start_progress(the_repository,
> +					  _("Downloading via HTTP"), 0);
> +		progress_set_fd(progress, fileno(stderr));
> +		freopen("/dev/null", "w", stderr);

Hmm. I can't think of any reason this wouldn't work, and it certainly is
less ugly than overriding die() and friends.

It does still feel a bit hacky to me. For one thing, asking for progress
does not necessarily mean you _also_ want to suppress errors. So it
would have to be a separate option.

But mostly the more I think about it, the more the "send progress data
back to the caller over stdout" thing makes sense. I think we probably
do eventually want to support parallel fetches, which would require
coordination in the caller.

-Peff