Re: [PATCH v3 2/5] commit test: Change $PWD to $(pwd)

[Johannes Sixt <j.sixt@viscovery.net>]

Please do not cull the Cc list.

Am 5/27/2014 8:14, schrieb Jeremiah Mahler:
> On Tue, May 27, 2014 at 07:46:59AM +0200, Johannes Sixt wrote:
>> Am 5/26/2014 20:56, schrieb Caleb Thompson:
>>> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>
>>> ---
>>>  t/t7507-commit-verbose.sh | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
>>> index 6d778ed..3b06d73 100755
>>> --- a/t/t7507-commit-verbose.sh
>>> +++ b/t/t7507-commit-verbose.sh
>>> @@ -8,7 +8,7 @@ cat >check-for-diff <<EOF
>>>  exec grep '^diff --git' "\$1"
>>>  EOF
>>>  chmod +x check-for-diff
>>> -test_set_editor "$PWD/check-for-diff"
>>> +test_set_editor "$(pwd)/check-for-diff"
>>>  
>>>  cat >message <<'EOF'
>>>  subject
>>
>> Why? I see no benefit. Both $PWD and $(pwd) work fine everywhere,
>> including Windows, and the former is faster, particularly on Windows.
> 
> I don't know the technical details of why this change is needed.
> But someone felt it was important enough to put in t/README.
> 
>   - When a test checks for an absolute path that a git command generated,
>     construct the expected value using $(pwd) rather than $PWD,
>     $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference on
>     Windows, where the shell (MSYS bash) mangles absolute path names.
>     For details, see the commit message of 4114156ae9.

That someone was I. I appreciate that people study t/README and do not
ignore the sentence.

However, it does not apply to the situation because the path to the editor
is not "generated by a git command and checked for by a test".

That said, it is not wrong to use $(pwd) with test_set_editor, it's just
unnecessarily slow.

-- Hannes