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

Re: [PATCH] archive-zip: load userdiff config

From
René Scharfe <l.s.r@web.de>
Date
Jan 3, 2017, 17:24 UTC
Message-ID
<7710c564-6b53-1908-7205-210d80eda59b@web.de>
In-Reply-To
<20170102222509.ho7motscnffrtnfh@sigill.intra.peff.net>
Am 02.01.2017 um 23:25 schrieb Jeff King:
Show 8 quoted lines
> 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.
Ah, didn't think of that, obviously.

Would it make sense for userdiff_find_by_path() to die if userdiff_config() hasn't been called, yet?

Show 5 quoted lines
> 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.

That reminds me of an old request to allow users better control over the meta-data written into archives. Being able to specify a time zone offset could be a start.

Show 24 quoted lines
>  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);
> +

I briefly thought about moving this call to archive.c with the rest of the config-related stuff, but I agree it's better kept here.

Show 56 quoted lines
>  	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' \
>
Looks good, thanks!
René
Previous: Jeff KingNext: Jeff King
Message 2 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.