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é