Re: [PATCH 3/7] checkout-index: add parallel checkout support

From: Derrick Stolee <stolee@gmail.com>
Date: Apr 23, 2021, 18:32 UTC
Message-ID: <36615f6a-ce58-3737-8c1d-fa1253a85439@gmail.com>
In-Reply-To: <6bf1db2fd8e807f9f259787ef75651d43b61e03a.1619104091.git.matheus.bernardino@usp.br>

On 4/22/2021 11:17 AM, Matheus Tavares wrote:

Show 9 quoted lines

> @@ -142,11 +143,7 @@ static void checkout_all(const char *prefix, int prefix_length)
>  	}
>  	if (last_ce && to_tempfile)
>  		write_tempfile_record(last_ce->name, prefix);
> -	if (errs)
> -		/* we have already done our error reporting.
> -		 * exit with the same code as die().
> -		 */
> -		exit(128);

Here, it makes note of returning 128 as if it were a die(), but

Show 9 quoted lines

> +	if (all)
> +		err |= checkout_all(prefix, prefix_length);
> +
> +	if (pc_workers > 1)
> +		err |= run_parallel_checkout(&state, pc_workers, pc_threshold,
> +					     NULL, NULL);
> +
>  	if (err)
>  		return 1;

This returns 1 instead. Should we `return err` and use an error code specific to the response? I imagine there are other reasons that could cause a non-zero return for checkout_all() and run_parallel_checkout().

I suppose: is there a value in persisting the 128 response code here, or is an exit of 1 sufficient? There is no documentation about the exit code, so any dependence on 128 is not based on a "written promise" but I thought it was worth mentioning.

Thanks, -Stolee