Re: [PATCH v2] grep: die gracefully when outside repository

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 20, 2023, 18:06 UTC
Message-ID: <xmqqcyx9qid0.fsf@gitster.g>
In-Reply-To: <CAPig+cTBYw9=Wo=TR8MD5xX9hgurnfR2Xzc_wHSYnL1R00=xpw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 22 quoted lines

> On Fri, Oct 20, 2023 at 12:40 PM Kristoffer Haugsbakk
> <code@khaugsbakk.name> wrote:
>> Die gracefully when `git grep --no-index` is run outside of a Git
>> repository and the path is outside the directory tree.
>>
>> If you are not in a Git repository and say:
>>
>>     git grep --no-index search ..
>>
>> You trigger a `BUG`:
>>
>>     BUG: environment.c:213: git environment hasn't been setup
>>     Aborted (core dumped)
>>
>> Because `..` is a valid path which is treated as a pathspec. Then
>> `pathspec` figures out that it is not in the current directory tree. The
>> `BUG` is triggered when `pathspec` tries to advice the user about how the
>> path is not in the current (non-existing) repository.
>
> s/advice/advise/
>
> (probably not worth a reroll)

The only remaining niggle I have is that the effect of this change would be much wider than just "grep", but in "git shortlog" output it may appear that this is specific to it, making later developers' life a bit harder when they are hunting for the cause of a behaviour change that is outside "grep", but still caused by this patch.

But I think I am worried too much in this particular case. Once this codepath is entered, the code will die no matter what, and we are merely making it die a bit more nicely.

There still is the "we say different things depending on the path outside the hierarchy exists or not" raised by Peff remaining, but for now, let's declare a victory and merge it to 'next'.

Thanks.