Re: [BUG?] fetch into shallow sends a large number of objects

[Jeff King <peff@peff.net>]

On Thu, Mar 10, 2016 at 07:20:20PM +0700, Duy Nguyen wrote:

> > +	else if (shallows.nr > 0) {
> > +		struct rev_info revs;
> > +		struct argv_array av = ARGV_ARRAY_INIT;
> > +		struct commit *c;
> > +		int i;
> > +
> > +		argv_array_push(&av, "rev-list");
> > +		argv_array_push(&av, "--boundary");
> 
> Nice. I didn't know about --boundary. But will it work correctly in
> this case?
> 
>        --- B ---- C ---- F
>           /      /
>      --- D ---- E ---- G
> 
> C and D will be current shallow cut points. People "want" F and G.
> "rev-list --boundary F G ^C ^D" would mark E as boundary/shallow too,
> correct? If so the history from G will be one depth short on a normal
> fetch.

IMHO, that is the right thing. They asked for "C" as a shallow cut-off
point, so anything that is a parent of "C" should be omitted as shallow,
too. It has nothing to do with the numeric depth, which was just the
starting point for generating the shallow cutoffs.

That's just my mental model, though. I admit I don't actually use
shallow clones myself, and maybe people would expect something else.

> > _But_, the client is not prepared to handle this. We send "shallow"
> > lines that it is not expecting, since it did not ask for any depth. So I
> > think this logic would have to kick in only when the client tells us to
> > do so.
> 
> Urgh.. not good. Perhaps a new extension to let the server know the
> client can handle spontaneous "deepen" commands and only activate new
> mode when the extension is present?

Yeah, we definitely need an extension. I'm not sure if the extension
should be "I know about spontaneous shallow/deepen responses; it's OK to
send them to me" or "I want you to include the shallow points I send as
boundary cutoffs for further shallow-ing of newly fetched history".

They amount to the same thing when implementing _this_ feature, but the
latter leaves us room in the future for a client to say "sure, I
understand your spontaneous responses, but I explicitly _don't_ want you
to do the boundary computation". I don't know if that is useful or not,
but it might not hurt to have later on (and by adding it now, it "just
works" later on with older servers/clients).

> > So what next? I think there's some protocol work here, and I think the
> > overall design of that needs to be considered alongside the other
> > "deepen" options your topic in pu adds (and of which I'm largely
> > ignorant). Does this sufficiently interest you to pick up and roll into
> > your other shallow work?
> 
> I can pick it up if you are busy with other stuff. But I'm also having
> a couple other topics at the moment, so it may not progress very fast.

Thanks. I don't think it is too urgent; it has been that way for a
while. I certainly have plenty of other things to work on, but mostly I
just feel a bit out of my depth on the shallow stuff. I haven't given it
any real thought, and you obviously have.

-Peff