From 2c2db194bd9e2f2dda7f1a47d26a1f86b3dd635a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 20 Apr 2022 22:26:09 +0200 Subject: [PATCH 1/2] tempfile: add mks_tempfile_dt() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a function to create a temporary file with a certain name in a temporary directory created using mkdtemp(3). Its result is more sightly than the paths created by mks_tempfile_ts(), which include a random prefix. That's useful for files passed to a program that displays their name, e.g. an external diff tool. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- tempfile.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tempfile.h | 13 +++++++++++ 2 files changed, 76 insertions(+) diff --git a/tempfile.c b/tempfile.c index 94aa18f3f7..2024c82691 100644 --- a/tempfile.c +++ b/tempfile.c @@ -56,6 +56,20 @@ static VOLATILE_LIST_HEAD(tempfile_list); +static void remove_template_directory(struct tempfile *tempfile, + int in_signal_handler) +{ + if (tempfile->directorylen > 0 && + tempfile->directorylen < tempfile->filename.len && + tempfile->filename.buf[tempfile->directorylen] == '/') { + strbuf_setlen(&tempfile->filename, tempfile->directorylen); + if (in_signal_handler) + rmdir(tempfile->filename.buf); + else + rmdir_or_warn(tempfile->filename.buf); + } +} + static void remove_tempfiles(int in_signal_handler) { pid_t me = getpid(); @@ -74,6 +88,7 @@ static void remove_tempfiles(int in_signal_handler) unlink(p->filename.buf); else unlink_or_warn(p->filename.buf); + remove_template_directory(p, in_signal_handler); p->active = 0; } @@ -100,6 +115,7 @@ static struct tempfile *new_tempfile(void) tempfile->owner = 0; INIT_LIST_HEAD(&tempfile->list); strbuf_init(&tempfile->filename, 0); + tempfile->directorylen = 0; return tempfile; } @@ -198,6 +214,52 @@ struct tempfile *mks_tempfile_tsm(const char *filename_template, int suffixlen, return tempfile; } +struct tempfile *mks_tempfile_dt(const char *directory_template, + const char *filename) +{ + struct tempfile *tempfile; + const char *tmpdir; + struct strbuf sb = STRBUF_INIT; + int fd; + size_t directorylen; + + if (!ends_with(directory_template, "XXXXXX")) { + errno = EINVAL; + return NULL; + } + + tmpdir = getenv("TMPDIR"); + if (!tmpdir) + tmpdir = "/tmp"; + + strbuf_addf(&sb, "%s/%s", tmpdir, directory_template); + directorylen = sb.len; + if (!mkdtemp(sb.buf)) { + int orig_errno = errno; + strbuf_release(&sb); + errno = orig_errno; + return NULL; + } + + strbuf_addf(&sb, "/%s", filename); + fd = open(sb.buf, O_CREAT | O_EXCL | O_RDWR, 0600); + if (fd < 0) { + int orig_errno = errno; + strbuf_setlen(&sb, directorylen); + rmdir(sb.buf); + strbuf_release(&sb); + errno = orig_errno; + return NULL; + } + + tempfile = new_tempfile(); + strbuf_swap(&tempfile->filename, &sb); + tempfile->directorylen = directorylen; + tempfile->fd = fd; + activate_tempfile(tempfile); + return tempfile; +} + struct tempfile *xmks_tempfile_m(const char *filename_template, int mode) { struct tempfile *tempfile; @@ -316,6 +378,7 @@ void delete_tempfile(struct tempfile **tempfile_p) close_tempfile_gently(tempfile); unlink_or_warn(tempfile->filename.buf); + remove_template_directory(tempfile, 0); deactivate_tempfile(tempfile); *tempfile_p = NULL; } diff --git a/tempfile.h b/tempfile.h index 4de3bc77d2..d7804a214a 100644 --- a/tempfile.h +++ b/tempfile.h @@ -82,6 +82,7 @@ struct tempfile { FILE *volatile fp; volatile pid_t owner; struct strbuf filename; + size_t directorylen; }; /* @@ -198,6 +199,18 @@ static inline struct tempfile *xmks_tempfile(const char *filename_template) return xmks_tempfile_m(filename_template, 0600); } +/* + * Attempt to create a temporary directory in $TMPDIR and to create and + * open a file in that new directory. Derive the directory name from the + * template in the manner of mkdtemp(). Arrange for directory and file + * to be deleted if the program exits before they are deleted + * explicitly. On success return a tempfile whose "filename" member + * contains the full path of the file and its "fd" member is open for + * writing the file. On error return NULL and set errno appropriately. + */ +struct tempfile *mks_tempfile_dt(const char *directory_template, + const char *filename); + /* * Associate a stdio stream with the temporary file (which must still * be open). Return `NULL` (*without* deleting the file) on error. The From 8af759374ec174372b841d2f8bc24edcba385418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Wed, 20 Apr 2022 22:30:10 +0200 Subject: [PATCH 2/2] diff: use mks_tempfile_dt() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Git uses temporary files to pass the contents of blobs to external diff programs and textconv filters. It calls mks_tempfile_ts() to create them, which puts them all in the same directory. This requires adding a random name prefix. Use mks_tempfile_dt() instead, which allows the files to have arbitrary names, each in their own separate temporary directory. This way they can have the same basename as the original blob, which looks nicer in graphical diff programs. The test in t4020 to check the prettiness of the temporary paths was neutered by 5476bdf0e8 (diff tests: don't ignore "git diff" exit code in "read" loop, 2022-03-07), which removed its grep check without replacing it with an equivalent test_cmp check. Add one that only checks the basename of the temporary file and nothing else. And make the test more robust while at it, by using test_when_finished to get rid of the added file even if the test fails. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- diff.c | 8 +------- t/t4020-diff-external.sh | 8 ++++---- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/diff.c b/diff.c index ef7159968b..e71cf75886 100644 --- a/diff.c +++ b/diff.c @@ -4136,18 +4136,13 @@ static void prep_temp_blob(struct index_state *istate, int mode) { struct strbuf buf = STRBUF_INIT; - struct strbuf tempfile = STRBUF_INIT; char *path_dup = xstrdup(path); const char *base = basename(path_dup); struct checkout_metadata meta; init_checkout_metadata(&meta, NULL, NULL, oid); - /* Generate "XXXXXX_basename.ext" */ - strbuf_addstr(&tempfile, "XXXXXX_"); - strbuf_addstr(&tempfile, base); - - temp->tempfile = mks_tempfile_ts(tempfile.buf, strlen(base) + 1); + temp->tempfile = mks_tempfile_dt("git-blob-XXXXXX", base); if (!temp->tempfile) die_errno("unable to create temp-file"); if (convert_to_working_tree(istate, path, @@ -4162,7 +4157,6 @@ static void prep_temp_blob(struct index_state *istate, oid_to_hex_r(temp->hex, oid); xsnprintf(temp->mode, sizeof(temp->mode), "%06o", mode); strbuf_release(&buf); - strbuf_release(&tempfile); free(path_dup); } diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh index 1219f8bd4c..858a5522f9 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -206,17 +206,17 @@ test_expect_success 'GIT_EXTERNAL_DIFF path counter/total' ' ' test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' ' + test_when_finished "git rm -f file.ext" && touch file.ext && git add file.ext && echo with extension > file.ext && cat >expect <<-EOF && - file.ext file $(git rev-parse --verify HEAD:file) 100644 file.ext $(test_oid zero) 100644 + file.ext EOF GIT_EXTERNAL_DIFF=echo git diff file.ext >out && - cut -d" " -f1,3- actual && - git update-index --force-remove file.ext && - rm file.ext + basename $(cut -d" " -f2 actual && + test_cmp expect actual ' echo "#!$SHELL_PATH" >fake-diff.sh