Re: bug:git-check-ignore exit status is wrong for negative patterns when -v option used

From: Jeremy Faith <jeremy.faith@jci.com>
Date: Apr 30, 2021, 09:56 UTC
Message-ID: <CY4P132MB00883DC8ABC72790A15C9630855E9@CY4P132MB0088.NAMP132.PROD.OUTLOOK.COM>
In-Reply-To: <xmqq5z04fwoz.fsf@gitster.g>

On 30 April 2021 01:22 Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

>Jeremy Faith <jeremy.faith@jci.com> writes:
>
>> man git-check-ignore states:-
>> EXIT STATUS
>> -----------
>> 0::
>>        One or more of the provided paths is ignored.
>> 1::
>>        None of the provided paths are ignored.
>> 128::
>>        A fatal error was encountered.
>>
>> So my change matches what the manual states.

>That is part of what I meant by "understandable, given the history".

Show 24 quoted lines

>The above description is _wrong_ ever since it was introduced, along
>with check-ignore, at 368aa529 (add git-check-ignore sub-command,
>2013-01-06).  It should have said "has/have entry that affects it in
>the gitignore/exclude files" instead of "is/are ignored".  After
>all, that is what the tool was written to do, i.e. to help debugging
>the gitignore/exclude files.
>
>    git-check-ignore(1)
>    ===================
>
>    NAME
>    ----
>    git-check-ignore - Debug gitignore / exclude files
>
>Having said all that.
>
>It is a misunderstanding that check-ignore is a tool to learn if a
>path is or is not ignored, but the misunderstanding is so widespread.
>
>I wonder if we can repurpose the command to "match" the
>misunderstanding, without hurting existing users, by
>
> (1) updating the one-liner description of the command in the
>     documentation;

Show 5 quoted lines

> (2) keep the EXIT STATUS section as-is; and
>
> (3) adjust the code to exit with status that reflects if there was
>     at least one path that was ignored (not "that had an entry in
>     the gitignore/exclude files that affected its fate").

If I understand correctly:-
  (2) requires no change
  (3) I believe my one line change does this

Which just leaves (1) where current line is
   git-check-ignore - Debug gitignore / exclude files
I think with the exit status operating as documented this description still
works i.e. check-ignore can be used to test if the .gitignore/exclude files
are working as desired.

The longer Description is:
  DESCRIPTION
       For each pathname given via the command-line or from a file via
       --stdin, check whether the file is excluded by .gitignore (or other
       input files to the exclude mechanism) and output the path if it is
       excluded.
Which matches the exit status meaning as is.

Show 6 quoted lines

>That certainly is a backward compatible change, but I suspect that
>we may be able to sell it as a bugfix, taking advantage of the
>documentation bug you quoted above.  Of course, people do not read
>documentation, so scripts that used to use the command in the way it
>was intended to be used (as opposed to "the way it was documented")
>will still get broken with such a change, though.

I'm not sure how the old exit status could be used in a useful way but you are correct there is a chance that some existing scripts depend on it.

I was originally confused by the exit status when using git versions 1.8.3.1 and 2.25.1. With these versions check-ignore returned 0 when a matching pattern was found, it did not matter if it was a positive or negative pattern. This did not match the exit status documented in the man page so I thought my .gitignore patterns were not working when they were. Perhaps I should stop reading man pages...

I then tried the same patterns on an up to date version of git built from a a git clone and found that without -v it returned the exit status as the man page states but with -v the older exit status was given. The commit that changed the exit status was 7ec8125 from 2020-02-18, first release that included this commit was 2.26.

So in 2.26 the exit status without -v was changed, maybe accidentally, in a way that made it match the man page. But this commit broke scripts(that do not use -v) that depend on exit status. My change extends the breakage to scripts that do use -v. On the other hand scripts written that expect the exit status to match the man page will be fixed!

I'm not sure what the best course of action is but at least with my change the exit status matches the man page(even with -v).