Re: [PATCH 06/18] t5613: clarify "too deep" recursion tests

> On Tue, Oct 04, 2016 at 01:44:23PM -0700, Jacob Keller wrote:
>
>> On Tue, Oct 4, 2016 at 6:48 AM, Jeff King <peff@peff.net> wrote:
>> > On Mon, Oct 03, 2016 at 10:57:48PM -0700, Jacob Keller wrote:
>> >
>> >> > diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
>> >> > index 7bc1c3c..b393613 100755
>> >> > --- a/t/t5613-info-alternate.sh
>> >> > +++ b/t/t5613-info-alternate.sh
>> >> > @@ -39,6 +39,18 @@ test_expect_success 'preparing third repository' '
>> >> >         )
>> >> >  '
>> >> >
>> >> > +# Note: These tests depend on the hard-coded value of 5 as "too deep". We start
>> >> > +# the depth at 0 and count links, not repositories, so in a chain like:
>> >> > +#
>> >> > +#   A -> B -> C -> D -> E -> F -> G -> H
>> >> > +#      0    1    2    3    4    5    6
>> >> > +#
>> >>
>> >> Ok so we count links, but wouldn't we have 5 links when we hit F, and
>> >> not G? Or am I missing something here?
>> >
>> > This is what I was trying to get at with the "start the depth at 0". We
>> > disallow a depth greater than 5, but because we start at 0-counting,
>> > it's really six links. I guess saying "5 as too deep" is really the
>> > misleading part. It should be "5 as the maximum depth".
>> >
>> > -Peff
>>
>> Right, but if A is 0, then:
>>
>> B = 1
>> C = 2
>> D = 3
>> E = 4
>> F = 5
>> G = 6  (UhOh??)
>> H = 7
>>
>> So do you mean that *B* = 0, and C = 1??? That is not clear from this commment.
>
> No, we count links, not repositories. So the "A->B" link is "0", "B->C"
> is "1", and so on.
>

>> So either way it still feels like "6" links is what is allowed? Or the
>> first link has to not count? That's really confusing.
>
> Right, 6 links _are_ allowed. Because we count links, and because we
> start the link-counting at "0" and allow through "5". The link labeled
> "6" (which is really the seventh link!) is the one that is forbidden.

>
>> Basically I G is the 7th letter, not the 6th, so even if we're
>> subtractnig 1 it's still 6 which is 1 too deep? That means we not only
>> discard 0 (the first repository) but we discount the 2nd one as well?
>
> It's basically two off-by-ones from what you might think is correct.  I
> agree it's unintuitive, but I'm just documenting what's there. We could
> change it; it's not like anybody cares about the exact value except
> "deep enough", but _since_ nobody cares, I preferred not to modify the
> code.
>