* Re: [PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
2012-12-20 6:28 ` Junio C Hamano
@ 2012-12-20 7:07 ` Junio C Hamano
2012-12-20 8:03 ` David Aguilar
2012-12-20 8:20 ` Joachim Schmitz
2 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2012-12-20 7:07 UTC (permalink / raw)
To: David Aguilar; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> 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.
One approach may be to do something like this as a preparation step
(current callers of unpack-file may want to do their temporary work
in TMPDIR as well), and then use
git unpack-file -t e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
to create your empty temporary file.
Documentation/git-unpack-file.txt | 10 +++++++---
builtin/unpack-file.c | 28 +++++++++++++++++++++-------
2 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-unpack-file.txt b/Documentation/git-unpack-file.txt
index e9f148a..56af328 100644
--- a/Documentation/git-unpack-file.txt
+++ b/Documentation/git-unpack-file.txt
@@ -10,16 +10,20 @@ git-unpack-file - Creates a temporary file with a blob's contents
SYNOPSIS
--------
[verse]
-'git unpack-file' <blob>
+'git unpack-file' [-t] <blob>
DESCRIPTION
-----------
Creates a file holding the contents of the blob specified by sha1. It
-returns the name of the temporary file in the following format:
- .merge_file_XXXXX
+returns the name of the temporary file (by default `.merge_file_XXXXX`).
+
OPTIONS
-------
+-t::
+ The temporary file is created in `$TMPDIR` directory,
+ instead of the current directory.
+
<blob>::
Must be a blob id
diff --git a/builtin/unpack-file.c b/builtin/unpack-file.c
index 1920029..de1f845 100644
--- a/builtin/unpack-file.c
+++ b/builtin/unpack-file.c
@@ -1,8 +1,9 @@
#include "builtin.h"
-static char *create_temp_file(unsigned char *sha1)
+static char *create_temp_file(unsigned char *sha1, int in_tempdir)
{
- static char path[50];
+ static char path[1024];
+ static const char template[] = ".merge_file_XXXXXX";
void *buf;
enum object_type type;
unsigned long size;
@@ -12,8 +13,12 @@ static char *create_temp_file(unsigned char *sha1)
if (!buf || type != OBJ_BLOB)
die("unable to read blob object %s", sha1_to_hex(sha1));
- strcpy(path, ".merge_file_XXXXXX");
- fd = xmkstemp(path);
+ if (in_tempdir) {
+ fd = git_mkstemp(path, sizeof(path) - 1, template);
+ } else {
+ strcpy(path, template);
+ fd = xmkstemp(path);
+ }
if (write_in_full(fd, buf, size) != size)
die_errno("unable to write temp-file");
close(fd);
@@ -23,14 +28,23 @@ static char *create_temp_file(unsigned char *sha1)
int cmd_unpack_file(int argc, const char **argv, const char *prefix)
{
unsigned char sha1[20];
+ int in_tempdir = 0;
+
+ if (argc < 2 || 3 < argc || !strcmp(argv[1], "-h"))
+ usage("git unpack-file [-t] <sha1>");
+ if (argc == 3) {
+ if (strcmp(argv[1], "-t"))
+ usage("git unpack-file [-t] <sha1>");
+ in_tempdir = 1;
+ argc--;
+ argv++;
+ }
- if (argc != 2 || !strcmp(argv[1], "-h"))
- usage("git unpack-file <sha1>");
if (get_sha1(argv[1], sha1))
die("Not a valid object name %s", argv[1]);
git_config(git_default_config, NULL);
- puts(create_temp_file(sha1));
+ puts(create_temp_file(sha1, in_tempdir));
return 0;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
2012-12-20 6:28 ` Junio C Hamano
2012-12-20 7:07 ` Junio C Hamano
@ 2012-12-20 8:03 ` David Aguilar
2012-12-20 8:20 ` Joachim Schmitz
2 siblings, 0 replies; 5+ messages in thread
From: David Aguilar @ 2012-12-20 8:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Wed, Dec 19, 2012 at 10:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread