git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Git List <git@vger.kernel.org>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"brian m . carlson" <sandals@crustytoothpaste.net>
Subject: [PATCH 2/2] diff: use mks_tempfile_dt()
Date: Wed, 20 Apr 2022 22:30:10 +0200	[thread overview]
Message-ID: <2ad1dd80-e79f-7304-219c-db24bb269c4d@web.de> (raw)
In-Reply-To: <1c07dad1-3d3d-c52b-4440-631b46520254@web.de>

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 <l.s.r@web.de>
---
 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- <out >actual &&
-	git update-index --force-remove file.ext &&
-	rm file.ext
+	basename $(cut -d" " -f2 <out) >actual &&
+	test_cmp expect actual
 '

 echo "#!$SHELL_PATH" >fake-diff.sh
--
2.35.3

      reply	other threads:[~2022-04-20 20:31 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-20 20:26 [PATCH 1/2] tempfile: add mks_tempfile_dt() René Scharfe
2022-04-20 20:30 ` René Scharfe [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2ad1dd80-e79f-7304-219c-db24bb269c4d@web.de \
    --to=l.s.r@web.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).