From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 07 Aug 2026 07:03:33 GMT
Subject: Re: [PATCH 2/6] builtin/receive-pack: pass shallow file explicitly
Message-ID: <anWDRVA0mSQva2QX@pks.im>
In-Reply-To: <20260806213859.816157-3-jltobler@gmail.com>

On Thu, Aug 06, 2026 at 04:38:55PM -0500, Justin Tobler wrote:
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 420de9aa7f..6da854fca2 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -86,7 +86,6 @@ static const char *head_name;
>  static void *head_name_to_free;
>  static int sent_capabilities;
>  static int shallow_update;
> -static const char *alt_shallow_file;
>  static struct strbuf push_cert = STRBUF_INIT;
>  static struct object_id push_cert_oid;
>  static struct signature_check sigcheck;

I always like seeing less global state.

> @@ -2354,10 +2353,9 @@ static const char *unpack(int err_fd, struct shallow_info *si,
>  		return hdr_err;
>  	}
>  
> -	if (si->nr_ours || si->nr_theirs) {
> -		alt_shallow_file = setup_temporary_shallow(si->shallow);
> +	if (shallow_file) {
>  		strvec_push(&child.args, "--shallow-file");
> -		strvec_push(&child.args, alt_shallow_file);
> +		strvec_push(&child.args, shallow_file);
>  	}
>  
>  	odb_transaction_env(transaction, &child.env);

Okay, so instead of creating the shallow file here, ...

> @@ -2705,11 +2705,17 @@ int cmd_receive_pack(int argc,
>  		if (!si.nr_ours && !si.nr_theirs)
>  			shallow_update = 0;
>  		if (!delete_only(commands)) {
> +			const char *alt_shallow_file = NULL;
> +
> +			if (si.nr_ours || si.nr_theirs)
> +				alt_shallow_file = setup_temporary_shallow(si.shallow);
> +
>  			if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
>  				unpack_status = "unable to start object transaction";
>  			else
> -				unpack_status = unpack_with_sideband(&si, transaction);
> -			update_shallow_info(commands, &si, &ref);
> +				unpack_status = unpack_with_sideband(transaction, alt_shallow_file);
> +
> +			update_shallow_info(commands, &si, &ref, alt_shallow_file);
>  		}

... we create it in a transitive caller and then pass it down the stack.
Makes sense.

It's nice that we don't have to pass the shallow information at all
anymore as a consequence.

Patrick