Re: [PATCH] fsck: avoid looking at NULL blob->object

[Jeff King <peff@peff.net>]

On Sat, Jun 09, 2018 at 04:38:54AM -0400, Eric Sunshine wrote:

> On Sat, Jun 9, 2018 at 4:32 AM, Jeff King <peff@peff.net> wrote:
> > Commit 159e7b080b (fsck: detect gitmodules files,
> > 2018-05-02) taught fsck to look at the content of
> > .gitmodules files. If the object turns out not to be a blob
> > at all, we just complain and punt on checking the content.
> > And since this was such an obvious and trivial code path, I
> > didn't even bother to add a test.
> > [...]
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> > diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh
> > @@ -151,4 +151,22 @@ test_expect_success 'fsck detects symlinked .gitmodules file' '
> > +test_expect_success 'fsck detects non-blob .gitmodules' '
> > +       git init non-blob &&
> > +       (
> > +               cd non-blob &&
> > +
> > +               # As above, make the funny directly to avoid index restrictions.
> 
> Is there a word missing after "funny"?

Oops, should be "funny tree" (that's what I get for trying to wordsmith
it at the last minute).

> > +               mkdir subdir &&
> > +               cp ../.gitmodules subdir/file &&
> > +               git add subdir/file &&
> > +               git commit -m ok &&
> > +               tree=$(git ls-tree HEAD | sed s/subdir/.gitmodules/ | git mktree) &&
> > +               commit=$(git commit-tree $tree) &&
> 
> I see that this is just mirroring the preceding test, but do you need
> to assign to variable 'commit' which is never consulted by anything
> later in the test?

No (nor above). I think originally I had planned to points refs to these
commits, but it isn't necessary for fsck. In fact, in the final form of
the patches, we do not even need the commit at all, since we will
complain about .gitmodules in _any_ tree (early versions actually did an
extra pass to find which trees were root trees).

-Peff