git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] archive-zip: load userdiff config

From
Jeff King <peff@peff.net>
Date
Jan 2, 2017, 22:25 UTC
Message-ID
<20170102222509.ho7motscnffrtnfh@sigill.intra.peff.net>

Since 4aff646d17 (archive-zip: mark text files in archives, 2015-03-05), the zip archiver will look at the userdiff driver to decide whether a file is text or binary. This usually doesn't need to look any further than the attributes themselves (e.g., "-diff", etc). But if the user defines a custom driver like "diff=foo", we need to look at "diff.foo.binary" in the config. Prior to this patch, we didn't actually load it.

Signed-off-by: Jeff King <peff@peff.net>
---
I'd be surprised if anybody actually triggered this in practice. I don't
think any of the custom-driver fields except "binary" matter, and using
direct attributes is almost always easier than setting up a custom
driver. Though you could also do trickery with:
  git -c diff.default.binary=true archive ...
if you wanted to be really clever.

I ran across this while investigating a case where somebody's zipfile was all marked as binary (it turned out not to be related; the issue was just that their Git was pre-4aff646d17).

I also happened to notice that zipfiles are created using the local timezone (because they have no notion of the timezone, so we have to pick _something_). That's probably the least-terrible option, but it was certainly surprising to me when I tried to bit-for-bit reproduce a zipfile from GitHub on my local machine.

 archive-zip.c          |  7 +++++++
 t/t5003-archive-zip.sh | 22 ++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/archive-zip.c b/archive-zip.c
index 9db47357b0..b429a8d974 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -554,11 +554,18 @@ static void dos_time(time_t *time, int *dos_date, int *dos_time)
 	*dos_time = t->tm_sec / 2 + t->tm_min * 32 + t->tm_hour * 2048;
 }
 
+static int archive_zip_config(const char *var, const char *value, void *data)
+{
+	return userdiff_config(var, value);
+}
+
 static int write_zip_archive(const struct archiver *ar,
 			     struct archiver_args *args)
 {
 	int err;
 
+	git_config(archive_zip_config, NULL);
+
 	dos_time(&args->time, &zip_date, &zip_time);
 
 	zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
index 14744b2a4b..55c7870997 100755
--- a/t/t5003-archive-zip.sh
+++ b/t/t5003-archive-zip.sh
@@ -64,6 +64,12 @@ check_zip() {
 		test_cmp_bin $original/nodiff.crlf $extracted/nodiff.crlf &&
 		test_cmp_bin $original/nodiff.lf   $extracted/nodiff.lf
 	"
+
+	test_expect_success UNZIP " validate that custom diff is unchanged " "
+		test_cmp_bin $original/custom.cr   $extracted/custom.cr &&
+		test_cmp_bin $original/custom.crlf $extracted/custom.crlf &&
+		test_cmp_bin $original/custom.lf   $extracted/custom.lf
+	"
 }
 
 test_expect_success \
@@ -78,6 +84,9 @@ test_expect_success \
      printf "text\r"	>a/nodiff.cr &&
      printf "text\r\n"	>a/nodiff.crlf &&
      printf "text\n"	>a/nodiff.lf &&
+     printf "text\r"	>a/custom.cr &&
+     printf "text\r\n"	>a/custom.crlf &&
+     printf "text\n"	>a/custom.lf &&
      printf "\0\r"	>a/binary.cr &&
      printf "\0\r\n"	>a/binary.crlf &&
      printf "\0\n"	>a/binary.lf &&
@@ -112,15 +121,20 @@ test_expect_success 'add files to repository' '
 test_expect_success 'setup export-subst and diff attributes' '
 	echo "a/nodiff.* -diff" >>.git/info/attributes &&
 	echo "a/diff.* diff" >>.git/info/attributes &&
+	echo "a/custom.* diff=custom" >>.git/info/attributes &&
+	git config diff.custom.binary true &&
 	echo "substfile?" export-subst >>.git/info/attributes &&
 	git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
 		>a/substfile1
 '
 
-test_expect_success \
-    'create bare clone' \
-    'git clone --bare . bare.git &&
-     cp .git/info/attributes bare.git/info/attributes'
+test_expect_success 'create bare clone' '
+	git clone --bare . bare.git &&
+	cp .git/info/attributes bare.git/info/attributes &&
+	# Recreate our changes to .git/config rather than just copying it, as
+	# we do not want to clobber core.bare or other settings.
+	git -C bare.git config diff.custom.binary true
+'
 
 test_expect_success \
     'remove ignored file' \
-- 
2.11.0.519.g31435224cf
Next: René Scharfe
Message 1 of 3 in “archive-zip: load userdiff config”
  1. archive-zip: load userdiff configJeff King, Jan 2, 2017
  2. René ScharfeJan 3, 2017
  3. Jeff KingJan 4, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.