From: Samo Pogačnik <samo_pogacnik@t-2.net>
Date: Sun, 15 Feb 2026 11:19:17 GMT
Subject: Re: [PATCH v4 2/2] shallow: handling fetch relative-deepen
Message-ID: <52d93e12de058d3ccaaebcf8c49a48d85e455171.camel@t-2.net>
In-Reply-To: <c55447544d68c3f98532b1d32183465c8f175ff8.camel@t-2.net>

On Sat, 2026-02-14 at 10:40 +0100, Samo Pogačnik wrote:
> On Wed, 2026-02-11 at 14:38 +0100, Patrick Steinhardt wrote:
> > On Fri, Jan 16, 2026 at 10:31:01PM +0000, Samo Pogačnik via GitGitGadget
> > wrote:
> > > 	object_array_clear(&reachable_shallows);
> > >  	} else {
> > >  		struct commit_list *result;
> > >  
> > > -		result = get_shallow_commits(&data->want_obj, depth,
> > > +		if (data->deepen_relative)
> > > +			get_shallows_depth(data);
> > 
> > Okay, so here we now essentially call `get_shallow_commits()` twice. The
> > first time we compute `data->deepen_relative`, only to then pass it back
> > to `get_shallow_commits()` a second time. That feels quite strange to
> > me. Can't we have `get_shallow_commits()` handle this for us directly in
> > a single call?
> > 
> > > +		result = get_shallow_commits(&data->want_obj, NULL, NULL,
> > > +					     data->deepen_relative +
> > > depth,
> > >  					     SHALLOW, NOT_SHALLOW);
> > >  		send_shallow(data, result);
> > >  		free_commit_list(result);
> > 
> 
> I have additional dilemma regarding handling this in a single call.
> Wouldn't it be generally good/useful to have a separate function in shallow.c
> just for measuring current absolute depth instead of blending the measurement
> into get_shallow_commits()?
> 

I prepared another version which i am going to post shortly. It does make a
single call to 'get_shallow_commits()' in upload_pack.c, however in shallow.c
there is again a common internal function, which is called once for measuring
current depth and then again to get a list shallow commits. I do not know how to
perform current depth measurement and shallows list retrieval with new depth
extended by the same measurement simultaneously. However i find it important to
keep common algorithm in a single function for easier maintenance. I hope this
is ok.

Best regards, Samo