Re: [PATCH] difftool: honor --trust-exit-code for builtin tools

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 16, 2014, 18:11 UTC
Message-ID: <xmqq4mtz10ef.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAHYJk3Q9tcS+o0hDnDz24ysSKkL6m16OmhyHuj=W88VQjTximw@mail.gmail.com>

Mikael Magnusson <mikachu@gmail.com> writes:

Show 19 quoted lines

>>> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
>>> index a40d3df..2b66351 100644
>>> --- a/git-mergetool--lib.sh
>>> +++ b/git-mergetool--lib.sh
>>> @@ -221,6 +221,7 @@ run_merge_tool () {
>>>       else
>>>               run_diff_cmd "$1"
>>>       fi
>>> +     status=$?
>>>       return $status
>>>  }
>>
>> Thanks for a quick turn-around.  As a hot-fix for what is already in
>> -rc I am fine with this fix but the patch makes me wonder if $status
>> as a global shell variable has any significance.
>
> $status is an alias for $? in zsh, and so cannot be assigned to. But
> other than that I don't think it holds any meaning and should be fine
> in a .sh script.

That is not what I meant by "global ... significance".

The question was if the codepath in the caller depends on this setting the global variable here, or nobody looks at and depends on the global variable we are setting here after this function returns.

It does not have any significance that a random shell implementation is not POSIX compliant. That would merely mean that such a shell cannot be used to run POSIX shell scripts like our Porcelain. I would suspect that zsh has more "posixly correct" mode, with which it _can_ run POSIX shell scripts, and I would imagine that this "$status is an alias $?" business is disabled in that mode?

My quick glance across the codepaths in the callers of this funciton indicated that it should be safe not using this global variable, so my answer to my original question was "no there is no significance". I think we can safely remove any mention of status from this shell function, i.e. if we remove initial assignment to 0, remove this new assignment and then remove the "return $status" at the end, the caller would still be happy.