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

From: Jeff King <peff@peff.net>
Date: Oct 4, 2016, 20:49 UTC
Message-ID: <20161004204933.ygfhoy24g6psyf6h@sigill.intra.peff.net>
In-Reply-To: <CA+P7+xok-8vhikxkp+t8pu53YJAyUjZ0NiAwejEW2j3+eP_2Xw@mail.gmail.com>

On Tue, Oct 04, 2016 at 01:44:23PM -0700, Jacob Keller wrote:

Show 39 quoted lines

> 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.

-Peff