git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Elijah Newren <newren@gmail.com>
Date
Apr 30, 2021, 20:51 UTC
Message-ID
<CABPp-BHYTBq=ExL=wx5oPH+DmWpSxsTqMbiOHgMM8uF1czs49g@mail.gmail.com>
In-Reply-To
<CY4P132MB00883DC8ABC72790A15C9630855E9@CY4P132MB0088.NAMP132.PROD.OUTLOOK.COM>
On Fri, Apr 30, 2021 at 2:57 AM Jeremy Faith <jeremy.faith@jci.com> wrote:
Show 99 quoted lines
>
> On 30 April 2021 01:22 Junio C Hamano <gitster@pobox.com> wrote:
>
> >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".
>
> >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;
>
> > (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.
>
> >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).

I'm inclined to agree with Jeremy here. Commit 7ec8125fba96 (check-ignore: fix documentation and implementation to match, 2020-02-18) demonstrated pretty clearly that check-ignore was just flat-out broken since the beginning in regards to negated patterns, in multiple ways: wrong exit status, documentation that was self-contradictory, and non-matching documentation and implementation. Among other things, that commit from last year fixed the exit status without -v. Unfortunately, when I (or the users I was interacting with) use -v, I ignore the exit status and am just looking for the output. So I missed the exit status inconsistency for -v when I created that commit and only fixed the exit status without -v.

Previous: Jeremy FaithNext: Junio C Hamano
Message 8 of 9 in “bug:git-check-ignore exit status is wrong for negative patterns when -v option used”
  1. Jeremy FaithApr 27, 2021
  2. Bagas SanjayaApr 28, 2021
  3. Jeremy FaithApr 29, 2021
  4. Junio C HamanoApr 28, 2021
  5. Jeremy FaithApr 29, 2021
  6. Junio C HamanoApr 30, 2021
  7. Jeremy FaithApr 30, 2021
  8. Elijah NewrenApr 30, 2021
  9. Junio C HamanoMay 3, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.