Re: [PATCH v2 7/7] http: silence stderr when progress is enabled

From: Jeff King <peff@peff.net>
Date: Feb 21, 2025, 07:48 UTC
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:

Show 13 quoted lines

> 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