Re: [PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder

From: David Aguilar <davvid@gmail.com>
Date: Dec 20, 2012, 08:03 UTC
Message-ID: <CAJDDKr63Yf0-3yTppGKAW+19wHY_OOF0iiKZ_6XR+Vae-7Bx7w@mail.gmail.com>
In-Reply-To: <7v623x6xvv.fsf@alter.siamese.dyndns.org>

On Wed, Dec 19, 2012 at 10:28 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

> David Aguilar <davvid@gmail.com> writes:
>
>> Use mktemp to create the /dev/null placeholder for p4merge.
>> This keeps it out of the current directory.
>>
>> Reported-by: Jeremy Morton <admin@game-point.net>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>> ---
>> I consider this a final finishing touch on a new 1.8.1 feature,
>> so hopefully we can get this in before 1.8.1.
>
> Does everybody have mktemp(1), which is not even in POSIX.1?
>
> I'm a bit hesitant to apply this to the upcoming release without
> cooking it in 'next' for sufficiently long time to give it a chance
> to be tried by wider audience.

True. I only tried Linux and Mac OS X, so I would not be surprised to find some exotic UNIX that does not have mktemp.

I meant to write "is this portable?" in the section after the double-dash; saying that it's polish for a fix is only true if it's portable.

The git-unpack thing looks interesting... is the SHA-1 in your example the special SHA-1 for an empty tree or blob?

The reason I ask is because in this code path we are comparing an unknown blob, basically a blob that does not exist in one of the trees, so I'm not sure if an 'unpack' command would help for this case because we would not have a blob SHA-1 to unpack.

As far as portability goes, the "UNIX" list for p4merge is here:

http://www.perforce.com/downloads/complete_list

I do not have AIX, Solaris, or FreeBSD to test, so I agree that this can wait.

Does msysgit have mktemp?

p4merge is available on Windows too so I would need to check there too unless someone happens to know off the top of their heads.

My other thought was to write a simple shell function that checks TMPDIR itself, and defaults to creating some file under /tmp when it is undefined. I can pursue this option if you think it's a safer choice.

-- 
David