Re: [PATCH] Add abbreviated commit hash to rebase conflict message

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Sverre Rabbelier <srabbelier@gmail.com> writes:
>
>> On Sun, Nov 6, 2011 at 21:27, Junio C Hamano <gitster@pobox.com> wrote:
>>> In what situation does it make sense to say "It came from _this_ commit"?
>>>
>>> I think there is a separate variable that allows any part of the script if
>>> we are being run as a backend of rebase or not, and that is the condition
>>> you are looking for.
>>
>> The closest I could find is:
>>
>>                 if test -f "$dotest/rebasing"
>>
>> Which is exactly the case when commit is set. Do you prefer the "-f
>> $dotest/rebasing" test or the "-n $commit" one?
>
> Given the variable scoping rules of vanilla shell script, relying on the
> variable $commit is a very bad idea to begin with.  I think the variable
> also is used to hold the final commit object name produced by patch
> application elsewhere in the script in the same loop, and I do not think
> existing code clears it before each iteration, as each part of the exiting
> code uses the variable only immediately after that part assigns to the
> variable for its own purpose, and they all know that nobody uses the
> variable as a way for long haul communication media between different
> parts of the script.  Unless your patch updated that aspect of the
> lifetime rule for the variable, which I doubt you did, using $commit would
> introduce yet another bug without solving anything, I would think.

I was looking at git-am today for a separate topic. Doesn't it appear to
you that $dotest/original-commit is what serves your purpose the best?

The file is removed before starting to process a new input (i.e. message
in the mbox), created only after we read the from line and determine it is
really the commit we are rebasing, and is left intact until we decide the
patch was applied correctly and write the result out as a tree.

It might be a clean-up to get rid of $dotest/original-commit file, rename
the variable to $original_commit and initialize it to an empty string
where we currently have 'rm -f "$dotest/original-commit"' (and replace the
check 'test -f "$dotest/original-commit"' later in the script with a check
'test -n "$original_commit"'), though.