On Fri, Mar 11, 2016 at 4:40 AM, Jeff King <peff@xxxxxxxx> wrote:
> On Thu, Mar 10, 2016 at 01:26:08PM -0800, Junio C Hamano wrote:
>
>> > 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.
>>
>> I think that is the right mental model. The statement that "C and D
>> are current cut points" does not make much sense. As you cannot
>> rewrite parents of commits after the fact, you cannot construct a
>> case like "when the shallow clone originally was made, two histories
>> were forked long time before B and D, and the cloner ended up with C
>> and D as the cutoff point, but now that we have the ancestry linkage
>> between B and D (and C and E), we need to make E a new cutoff". The
>> original "shallow" implementation does not store "starting point +
>> number of depth" and instead translates that to the cut-off point
>> for this exact reason.
Well, assume again that F and G are ref heads, and their respective
distance to C and D are the same (like the below graph), then "fetch
--deptch=<distance>" can mark C and D as shallow cut points because
--depth traverses from refs until the distance is met, it does not do
total exclusion ^C like rev-list.
--- B ---- C ---- H ---- F
/ /
--- D ---- E ---- G
> OK, good. Now there are at least two of us who view it that way. :)
But going that direction just gives me more headache. If you two are
ok with this and nobody else complains, I'm ok too :-D I guess it's a
corner case anyway that's probably hard to occur in practice.
>> > 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.
More people understanding this code is always better though. But don't
worry I'll take care of this.
>> > 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).
>>
>> I am not sure what distinction you are worried about. An updated
>> client that is capable of saying "you may give shallow/deepen
>> responses to me" can optionally be told not to say it to the server,
>> and that is equivalent to saying "I don't want you to send them", no?
>
> Mostly, I wondered if we would need to send spontaneous shallow lines
> for any other cases, and the client would not be able to say "I
> understand them and want them in case A, but not in case B".
>
> I do not have any case A in mind; it was just a general sense of "let's
> make feature flags as specific as possible to avoid painting ourselves
> into a corner". I'm OK with implementing it either way.
Another note to myself is, we do not want these spontaneous cutoff
requests to cut client's history even shorter (possibly by accident
because of buggy servers). As long as they are about sealing commits
that lead to non-existing objects, i think it's ok to make it a
general feature.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html