Re: [PATCH 8/8] check-whitespace: detect if no base_commit is provided

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 10, 2024, 16:02 UTC
Message-ID: <xmqq8qy9z14t.fsf@gitster.g>
In-Reply-To: <CAOLa=ZQdPTnLp63eFiq1oOkgukBGsug==Cam0hGW_9koCefhGA@mail.gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 7 quoted lines

> Yeah, makes sense. I think I'll simply add in
>
>     if ! git rev-parse --quiet --verify "${baseCommit}"
>     then
>         echo "Invalid <BASE_COMMIT> '${baseCommit}'"
>         exit 1
>     fi

It would make the intent a lot clearer.  Good.

Show 25 quoted lines

>>> +if test $? -ne 0
>>> +then
>>> +	echo -n $gitLogOutput
>>
>> What is "-n" doing here?  Why are you squashing run of spaces in the
>> $gitLogOutput variable into a space by not "quoting" inside a dq-pair?
>>
>
> I actually didn't know about this. Thanks for informing.
>
>>> +	exit 1
>>> +fi
>>
>> Looking for "--check" in
>>
>> 	$ git log --help
>>
>> tells me that the command exits with non-zero status if problems are
>> found, so wouldn't that mean the cases with problems always exit
>> early, bypassing the while loop with full of bash-ism that comes
>> after this block?
>>
>
> It should exist with a non-zero code, but since we're capturing it in
> the while loop, it doesn't stop the slow.

Sorry, but I am confused. The control would not even reach a "while loop", I am afraid.

I was commenting on the exit status check done here:

    +gitLogOutput=$(git log --check --pretty=format:"---% h% s" "${baseCommit}"..)
    +if test $? -ne 0
    +then
    +	echo -n $gitLogOutput
    +	exit 1
    +fi

Even though the output is captured in a variable, the exit status of "git log --check" is still seen by the shell and "if test $? = 0" next line say "ah, the thing exited with non-zero status so lets echo the whole thing and exit with 1", before it gets to the while loop we have below the above piece of code, no?