Re: [PATCH 8/8] t5801: teach compare_refs() to accept !

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 25, 2020, 06:36 UTC
Message-ID: <CAPig+cSJVdFEZ_V_FBtdAhR7KqocqXCgkHm1pFMDjBX_NPE+Jw@mail.gmail.com>
In-Reply-To: <xmqqv9mtx7wv.fsf@gitster.c.googlers.com>

On Wed, Mar 25, 2020 at 2:31 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 21 quoted lines

> Denton Liu <liu.denton@gmail.com> writes:
> >  compare_refs() {
> > +     fail= &&
> > +     if test "x$1" = 'x!'
> > +     then
> > +             fail='!' &&
> > +             shift
> > +     fi &&
> >       git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
> >       git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
> > -     test_cmp expect actual
> > +     eval $fail test_cmp expect actual
> >  }
> > -     test_must_fail compare_refs local signed-tag server signed-tag
> > +     compare_refs ! local signed-tag server signed-tag
>
> While this is not wrong per-se, I do not know why we cannot just use
>
>         ! compare_refs local signed-tag server signed-tag
>
> i.e. "we expect these two repositories have different tags"?

As mentioned in the commit message, if one of the git-rev-parse invocations fails unexpectedly, then compare_refs() would return early with a failure code, but the "!" would then (undesirably) turn that failure into a success. We don't want to lose a failure code from git-rev-parse, so the simple use of "! compare_refs ..." is avoided.