git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeremy Faith <jeremy.faith@jci.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: bug:git-check-ignore exit status is wrong for negative patterns when -v option used
Date: Fri, 30 Apr 2021 09:56:11 +0000	[thread overview]
Message-ID: <CY4P132MB00883DC8ABC72790A15C9630855E9@CY4P132MB0088.NAMP132.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <xmqq5z04fwoz.fsf@gitster.g>

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

  reply	other threads:[~2021-04-30  9:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-27 16:30 bug:git-check-ignore exit status is wrong for negative patterns when -v option used Jeremy Faith
2021-04-28  6:15 ` Bagas Sanjaya
2021-04-29 14:26   ` Jeremy Faith
2021-04-28  7:13 ` Junio C Hamano
2021-04-29 14:10   ` Jeremy Faith
2021-04-30  0:22     ` Junio C Hamano
2021-04-30  9:56       ` Jeremy Faith [this message]
2021-04-30 20:51         ` Elijah Newren
2021-05-03  3:09         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CY4P132MB00883DC8ABC72790A15C9630855E9@CY4P132MB0088.NAMP132.PROD.OUTLOOK.COM \
    --to=jeremy.faith@jci.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).