Re: Git 1.8.4.2: 'git-rev-parse --is-inside-git-dir' wrong output!

From: Philip Oakley <philipoakley@iee.org>
Date: Nov 2, 2013, 16:11 UTC
Message-ID: <CE13C82CA4C24CA8B56380DA00A700A5@PhilipOakley>
In-Reply-To: <20131102140656.GE24023@serenity.lan>

From: "John Keeping" <john@keeping.me.uk>
Sent: Saturday, November 02, 2013 2:06 PM

Show 56 quoted lines

> On Sat, Nov 02, 2013 at 01:47:02PM -0000, Philip Oakley wrote:
>> From: "John Keeping" <john@keeping.me.uk>
>> Sent: Saturday, November 02, 2013 10:58 AM
>> > On Fri, Nov 01, 2013 at 06:19:51PM -0500, Ville Walveranta wrote:
>> >> "git-rev-parse --is-inside-git-dir" outputs "fatal: Not a git
>> >> repository (or any of the parent directories): .git", instead of
>> >> "false" when outside of a git directory.  "--is-inside-work-tree"
>> >> behaves the same way. Both commands work correctly (i.e. output
>> >> "true") when inside a git directory, or inside a work tree,
>> >> respectively.
>> >
>> > I think that's intentional - and it looks like the behaviour has
>> > not
>> > changed since these options were added.  With the current behaviour
>> > you
>> > get three possible outcomes from "git
>> > rev-parse --is-inside-work-tree":
>> >
>> >    if worktree=$(git rev-parse --is-inside-work-tree 2>/dev/null)
>> >    then
>> >        if test "$worktree" = true
>> >        then
>> >            echo 'inside work tree'
>> >        else
>> >            echo 'in repository, but not in work tree'
>> >        fi
>> >    else
>> >        echo 'not in repository'
>> >    fi
>> > --
>>
>>
>> Shouldn't this case which produces "fatal:..." need to be documented
>> in
>> the man page?
>> https://www.kernel.org/pub/software/scm/git/docs/git-rev-parse.html
>> doesn't mention it.
>
> I'm not sure where it should go in there.  The documentation for
> --git-dir says:
>
>   If $GIT_DIR is not defined and the current directory is not detected
>   to lie in a Git repository or work tree print a message to stderr
> and
>   exit with nonzero status.
>
> but there reality is that if you do not specify --parseopt
> or --sq-quote
> then the command expects to be run in a Git repository [1], so perhaps
> it would be better to say something under "Operation Modes" or in the
> description.
>
>
> [1] After taking account of $GIT_DIR, $GIT_WORK_TREE, and arguments to
>    the base "git" driver that affect these variables.
>

Yes, but given Ville's surprise and the need for special prior knowledge of the points you raised, I still think that some short note is needed.

It can/could be read that you need to invoke --git-dir as an option before the mentioned die() exit is taken, rather than it applying to all(?) the path relevant options.

Either the --git-dir condition needs to say it also applies to --is-inside-git-dir and --is-inside-work-tree (and --is-bare-repository?), or add a "see --git-dir preconditions." to each of those options. It's easy to be wise after the event hence my preference for a suitable note.

regards

Philip