{"thread":{"id":"38632","subject":"zip files created with git archive flags text files as binaries","startedAt":"2015-02-23T13:58:42Z","lastAt":"2015-03-05T19:06:02Z","messageCount":8,"participants":["Ulrike Fischer","René Scharfe","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"256507","messageId":"12g5ss8uqwflv.dlg@nililand.de","threadId":"38632","inReplyTo":null,"subject":"zip files created with git archive flags text files as binaries","fromName":"Ulrike Fischer","fromEmail":"luatex@nililand.de","sentAt":"2015-02-23T13:58:42Z","receivedAt":"2015-02-23T13:58:42Z","isPatch":false,"sender":{"key":"luatex@nililand.de","avatar":null},"body":"I'm using git on windows 7.\n\n$ git --version\ngit version 1.9.4.msysgit.0\n\nSome days ago I uploaded a latex package to CTAN (www.ctan.org). \nI created the zip-file with \n\ngit archive --format=zip --prefix=citeall/\n--output=zip/citeall_2015-02-20.zip HEAD \n\nThe zip contained four text files and a pdf. \n\nThe CTAN maintainers informed me that all files in the zip are\nflagged as binaries and this makes it difficult for them to process\nthem further (they want to correct line feeds of the text files:\nhttp://mirror.ctan.org/tex-archive/help/ctan/CTAN-upload-addendum.html#crlf) \n\nunzip -Z reports for my zip:\n\n$ unzip -Z citeall_2015_02_20.zip\nArchive:  citeall_2015_02_20.zip\nZip file size: 105509 bytes, number of entries: 6\ndrwx---     0.0 fat        0 bx stor 15-Feb-20 17:07 citeall/\n-rw----     0.0 fat      458 bx defN 15-Feb-20 17:07 citeall/README\n-rw----     0.0 fat   102244 bx defN 15-Feb-20 17:07\nciteall/citeall.pdf\n-rw----     0.0 fat     3431 bx defN 15-Feb-20 17:07\nciteall/citeall.sty\n-rw----     0.0 fat     3971 bx defN 15-Feb-20 17:07\nciteall/citeall.tex\n-rw----     0.0 fat      557 bx defN 15-Feb-20 17:07\nciteall/examples-citeall.bib\n6 files, 110661 bytes uncompressed, 104669 bytes compressed:  5.4%\n\nThe problem are all the \"bx\" entries. \n\nWhen I zip all the files with the standard windows zip-tool I get\nthis:\n\n$ unzip -Z citeall-win.zip\nArchive:  citeall-win.zip\nZip file size: 105275 bytes, number of entries: 5\n-rw----     2.0 fat   102244 b- defN 15-Feb-20 17:07\nciteall/citeall.pdf\n-rw----     2.0 fat     3431 t- defN 15-Feb-20 17:07\nciteall/citeall.sty\n-rw----     2.0 fat     3971 t- defN 15-Feb-20 17:07\nciteall/citeall.tex\n-rw----     2.0 fat      557 t- defN 15-Feb-20 17:07\nciteall/examples-citeall.bib\n-rw----     2.0 fat      458 t- defN 15-Feb-20 17:07 citeall/README\n5 files, 110661 bytes uncompressed, 104675 bytes compressed:  5.4%\n\nHere the text files have a correct t flag. \n\nI don't know if it the problem exists also with zips created with\ngit archive on non-windows OS.\n\nWould it be possible to correct the zip-backend so that it flags\ntext files correctly? Or alternativly could one configure git\narchive to use another zip programm? \n\n(I tried to sent this already some hours ago, but the message seems\nto be lost ...)\n\n-- \nUlrike Fischer \nhttp://www.troubleshooting-tex.de/\n"},{"id":"256537","messageId":"54EB7FE4.7070804@web.de","threadId":"38632","inReplyTo":"12g5ss8uqwflv.dlg@nililand.de","subject":"Re: zip files created with git archive flags text files as binaries","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-02-23T19:30:44Z","receivedAt":"2015-02-23T19:30:44Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.02.2015 um 14:58 schrieb Ulrike Fischer:\n> I'm using git on windows 7.\n>\n> $ git --version\n> git version 1.9.4.msysgit.0\n\nGit's code for ZIP file creation hasn't changed since then.\n\n> Some days ago I uploaded a latex package to CTAN (www.ctan.org).\n> I created the zip-file with\n>\n> git archive --format=zip --prefix=citeall/\n> --output=zip/citeall_2015-02-20.zip HEAD\n>\n> The zip contained four text files and a pdf.\n>\n> The CTAN maintainers informed me that all files in the zip are\n> flagged as binaries and this makes it difficult for them to process\n> them further (they want to correct line feeds of the text files:\n> http://mirror.ctan.org/tex-archive/help/ctan/CTAN-upload-addendum.html#crlf)\n\nI wouldn't have expected that this ZIP file attribute is actually used \nin the wild.\n\n> unzip -Z reports for my zip:\n>\n> $ unzip -Z citeall_2015_02_20.zip\n> Archive:  citeall_2015_02_20.zip\n> Zip file size: 105509 bytes, number of entries: 6\n> drwx---     0.0 fat        0 bx stor 15-Feb-20 17:07 citeall/\n> -rw----     0.0 fat      458 bx defN 15-Feb-20 17:07 citeall/README\n> -rw----     0.0 fat   102244 bx defN 15-Feb-20 17:07\n> citeall/citeall.pdf\n> -rw----     0.0 fat     3431 bx defN 15-Feb-20 17:07\n> citeall/citeall.sty\n> -rw----     0.0 fat     3971 bx defN 15-Feb-20 17:07\n> citeall/citeall.tex\n> -rw----     0.0 fat      557 bx defN 15-Feb-20 17:07\n> citeall/examples-citeall.bib\n> 6 files, 110661 bytes uncompressed, 104669 bytes compressed:  5.4%\n>\n> The problem are all the \"bx\" entries.\n\nThe \"x\" is due to the extra mtime header and unrelated to your issue.\n\n\"b\" (binary) is set unconditionally to ensure that line endings are kept \nintact by unpackers on all platforms.\n\n\"fat\" is used as lowest common denominator for regular files and \ndirectories; \"unx\" is used for symbolic links.\n\n> When I zip all the files with the standard windows zip-tool I get\n> this:\n>\n> $ unzip -Z citeall-win.zip\n> Archive:  citeall-win.zip\n> Zip file size: 105275 bytes, number of entries: 5\n> -rw----     2.0 fat   102244 b- defN 15-Feb-20 17:07\n> citeall/citeall.pdf\n> -rw----     2.0 fat     3431 t- defN 15-Feb-20 17:07\n> citeall/citeall.sty\n> -rw----     2.0 fat     3971 t- defN 15-Feb-20 17:07\n> citeall/citeall.tex\n> -rw----     2.0 fat      557 t- defN 15-Feb-20 17:07\n> citeall/examples-citeall.bib\n> -rw----     2.0 fat      458 t- defN 15-Feb-20 17:07 citeall/README\n> 5 files, 110661 bytes uncompressed, 104675 bytes compressed:  5.4%\n>\n> Here the text files have a correct t flag.\n>\n> I don't know if it the problem exists also with zips created with\n> git archive on non-windows OS.\n\nYes, git archive creates the same ZIP files everywhere.\n\n> Would it be possible to correct the zip-backend so that it flags\n> text files correctly? Or alternativly could one configure git\n> archive to use another zip programm?\n\nWe would have to detect the line ending format (DOS, Unix, Macintosh, \netc.) of each file, then set the attribute \"t\" (text) and the host \nsystem.  The detection would slow down archive creation a bit and the \nresulting files would be different, of course, so this feature should \nonly be enabled by a new command line option.  I'll take a look.\n\nRené\n"},{"id":"257019","messageId":"54F7756D.6080907@web.de","threadId":"38632","inReplyTo":"54EB7FE4.7070804@web.de","subject":"Re: zip files created with git archive flags text files as binaries","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-04T21:13:17Z","receivedAt":"2015-03-04T21:13:17Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.02.2015 um 20:30 schrieb René Scharfe:\n> Am 23.02.2015 um 14:58 schrieb Ulrike Fischer:\n>> The zip contained four text files and a pdf.\n>>\n>> The CTAN maintainers informed me that all files in the zip are\n>> flagged as binaries and this makes it difficult for them to process\n>> them further (they want to correct line feeds of the text files:\n>> http://mirror.ctan.org/tex-archive/help/ctan/CTAN-upload-addendum.html#crlf)\n\nBy the way, a workaround for CTAN could be to extract the files using \nunzip and zip them again using Info-ZIP zip (one of the \"good zip \nprograms\" mentioned on that website).  The result will be a ZIP file \nwith text files flagged appropriately.  Just saying.\n\n>> Would it be possible to correct the zip-backend so that it flags\n>> text files correctly? Or alternativly could one configure git\n>> archive to use another zip programm?\n>\n> We would have to detect the line ending format (DOS, Unix, Macintosh,\n> etc.) of each file, then set the attribute \"t\" (text) and the host\n> system.  The detection would slow down archive creation a bit and the\n> resulting files would be different, of course, so this feature should\n> only be enabled by a new command line option.  I'll take a look.\n\nActually its easier than that.  unzip -a (with end-of-line conversion) \ndoesn't care for the actual format, it simply converts all occurrences \nof CR, CRLF and LF to the appropriate newline chars for the platform it \nis running on if the text flag is set.  Files with mixed line endings \nare normalized, i.e. they have a consistent end-of-line style afterward.\n\nI'll send a first patch shortly.\n\nRené\n"},{"id":"257028","messageId":"54F77573.80109@web.de","threadId":"38632","inReplyTo":"12g5ss8uqwflv.dlg@nililand.de","subject":"[PATCH] archive-zip: add --text parameter","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-04T21:13:23Z","receivedAt":"2015-03-04T21:13:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Entries in a ZIP file can be marked as text files.  Extractors can use\nthat flag to apply end-of-line conversions.  An example is unzip -a.\n\ngit archive currently marks all ZIP file entries as binary files.  This\npatch adds the new option --text that can be used to mark non-binary\nfiles or all files as text files, thus enabling the use of unzip -a.\n\nNo sign-off, yet, because I'm not sure we really need another option.\nE.g. --text=all doesn't seem to be actually useful, but it was easy to\nimplement.  Info-ZIP's zip always creates archives like --text=auto\ndoes, so perhaps we should make that our default behavior as well?\n\nChanging the default behavior would cause newer versions of git archive\nto create different ZIP files than older ones, of course.  This can\nbreak caching and signature checking.\n\nThe last time we did that was in 2012 when we added an extended mtime\nfield (227bf5980), I think.  I don't remember any fallout from that\nchange, but there was a recent discussion about the stability of\ngenerated tar files, so I'm a bit cautious:\n\n    http://thread.gmane.org/gmane.comp.version-control.git/258516\n\n---\n Documentation/git-archive.txt |  5 ++++\n archive-zip.c                 | 23 ++++++++++++++----\n archive.c                     | 18 ++++++++++++++\n archive.h                     |  7 ++++++\n t/t5003-archive-zip.sh        | 56 +++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 105 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex cfa1e4e..684ca36 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -93,6 +93,11 @@ zip\n \tHighest and slowest compression level.  You can specify any\n \tnumber from 1 to 9 to adjust compression speed and ratio.\n \n+--text=<which>::\n+\tMark the specfied entries as text files so that `unzip -a`\n+\tconverts end-of-line characters while extracting. The value\n+\tmust be either 'all', 'auto', or 'none' (the default).\n+\n \n CONFIGURATION\n -------------\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 4bde019..3767940 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -5,6 +5,7 @@\n #include \"archive.h\"\n #include \"streaming.h\"\n #include \"utf8.h\"\n+#include \"xdiff-interface.h\"\n \n static int zip_date;\n static int zip_time;\n@@ -210,6 +211,7 @@ static int write_zip_entry(struct archiver_args *args,\n \tstruct git_istream *stream = NULL;\n \tunsigned long flags = 0;\n \tunsigned long size;\n+\tint is_binary = -1;\n \n \tcrc = crc32(0, NULL, 0);\n \n@@ -238,8 +240,14 @@ static int write_zip_entry(struct archiver_args *args,\n \t\tmethod = 0;\n \t\tattr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) :\n \t\t\t(mode & 0111) ? ((mode) << 16) : 0;\n-\t\tif (S_ISREG(mode) && args->compression_level != 0 && size > 0)\n-\t\t\tmethod = 8;\n+\t\tif (S_ISREG(mode)) {\n+\t\t\tif (args->compression_level != 0 && size > 0)\n+\t\t\t\tmethod = 8;\n+\t\t\tif (args->text == ARCHIVE_TEXT_ALL)\n+\t\t\t\tis_binary = 0;\n+\t\t\telse if (args->text == ARCHIVE_TEXT_NONE)\n+\t\t\t\tis_binary = 1;\n+\t\t}\n \n \t\tif (S_ISREG(mode) && type == OBJ_BLOB && !args->convert &&\n \t\t    size > big_file_threshold) {\n@@ -256,6 +264,8 @@ static int write_zip_entry(struct archiver_args *args,\n \t\t\t\treturn error(\"cannot read %s\",\n \t\t\t\t\t     sha1_to_hex(sha1));\n \t\t\tcrc = crc32(crc, buffer, size);\n+\t\t\tif (is_binary < 0)\n+\t\t\t\tis_binary = buffer_is_binary(buffer, size);\n \t\t\tout = buffer;\n \t\t}\n \t\tcompressed_size = (method == 0) ? size : 0;\n@@ -300,7 +310,6 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(dirent.extra_length, ZIP_EXTRA_MTIME_SIZE);\n \tcopy_le16(dirent.comment_length, 0);\n \tcopy_le16(dirent.disk, 0);\n-\tcopy_le16(dirent.attr1, 0);\n \tcopy_le32(dirent.attr2, attr2);\n \tcopy_le32(dirent.offset, zip_offset);\n \n@@ -328,6 +337,8 @@ static int write_zip_entry(struct archiver_args *args,\n \t\t\tif (readlen <= 0)\n \t\t\t\tbreak;\n \t\t\tcrc = crc32(crc, buf, readlen);\n+\t\t\tif (is_binary < 0)\n+\t\t\t\tis_binary = buffer_is_binary(buffer, size);\n \t\t\twrite_or_die(1, buf, readlen);\n \t\t}\n \t\tclose_istream(stream);\n@@ -361,6 +372,8 @@ static int write_zip_entry(struct archiver_args *args,\n \t\t\tif (readlen <= 0)\n \t\t\t\tbreak;\n \t\t\tcrc = crc32(crc, buf, readlen);\n+\t\t\tif (is_binary < 0)\n+\t\t\t\tis_binary = buffer_is_binary(buffer, size);\n \n \t\t\tzstream.next_in = buf;\n \t\t\tzstream.avail_in = readlen;\n@@ -405,6 +418,8 @@ static int write_zip_entry(struct archiver_args *args,\n \tfree(deflated);\n \tfree(buffer);\n \n+\tcopy_le16(dirent.attr1, !is_binary);\n+\n \tmemcpy(zip_dir + zip_dir_offset, &dirent, ZIP_DIR_HEADER_SIZE);\n \tzip_dir_offset += ZIP_DIR_HEADER_SIZE;\n \tmemcpy(zip_dir + zip_dir_offset, path, pathlen);\n@@ -466,7 +481,7 @@ static int write_zip_archive(const struct archiver *ar,\n static struct archiver zip_archiver = {\n \t\"zip\",\n \twrite_zip_archive,\n-\tARCHIVER_WANT_COMPRESSION_LEVELS|ARCHIVER_REMOTE\n+\tARCHIVER_WANT_COMPRESSION_LEVELS|ARCHIVER_REMOTE|ARCHIVER_TEXT_ATTRIBUTE\n };\n \n void init_zip_archiver(void)\ndiff --git a/archive.c b/archive.c\nindex 96057ed..89bd23d 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -417,6 +417,7 @@ static int parse_archive_args(int argc, const char **argv,\n \tconst char *remote = NULL;\n \tconst char *exec = NULL;\n \tconst char *output = NULL;\n+\tconst char *text = NULL;\n \tint compression_level = -1;\n \tint verbose = 0;\n \tint i;\n@@ -442,6 +443,8 @@ static int parse_archive_args(int argc, const char **argv,\n \t\tOPT__COMPR_HIDDEN('7', &compression_level, 7),\n \t\tOPT__COMPR_HIDDEN('8', &compression_level, 8),\n \t\tOPT__COMPR('9', &compression_level, N_(\"compress better\"), 9),\n+\t\tOPT_STRING(0, \"text\", &text, N_(\"which\"),\n+\t\t\tN_(\"specify which files contain text\")),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL('l', \"list\", &list,\n \t\t\tN_(\"list supported archive formats\")),\n@@ -493,6 +496,21 @@ static int parse_archive_args(int argc, const char **argv,\n \t\t\t\t\tformat, compression_level);\n \t\t}\n \t}\n+\targs->text = ARCHIVE_TEXT_NONE;\n+\tif (text) {\n+\t\tif (!strcmp(text, \"auto\"))\n+\t\t\targs->text = ARCHIVE_TEXT_AUTO;\n+\t\telse if (!strcmp(text, \"all\"))\n+\t\t\targs->text = ARCHIVE_TEXT_ALL;\n+\t\telse if (!strcmp(text, \"none\"))\n+\t\t\targs->text = ARCHIVE_TEXT_NONE;\n+\t\telse\n+\t\t\tdie(\"Unknown argument: --text=%s\", text);\n+\t\tif (args->text != ARCHIVE_TEXT_NONE &&\n+\t\t    !((*ar)->flags & ARCHIVER_TEXT_ATTRIBUTE))\n+\t\t\tdie(\"Argument not supported for format '%s': --text=%s\",\n+\t\t\t    format, text);\n+\t}\n \targs->verbose = verbose;\n \targs->base = base;\n \targs->baselen = strlen(base);\ndiff --git a/archive.h b/archive.h\nindex 4a791e1..eabcd11 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -14,11 +14,18 @@ struct archiver_args {\n \tunsigned int verbose : 1;\n \tunsigned int worktree_attributes : 1;\n \tunsigned int convert : 1;\n+\tunsigned int text : 2;\n \tint compression_level;\n };\n \n #define ARCHIVER_WANT_COMPRESSION_LEVELS 1\n #define ARCHIVER_REMOTE 2\n+#define ARCHIVER_TEXT_ATTRIBUTE 4\n+\n+#define ARCHIVE_TEXT_NONE\t0\n+#define ARCHIVE_TEXT_ALL\t1\n+#define ARCHIVE_TEXT_AUTO\t2\n+\n struct archiver {\n \tconst char *name;\n \tint (*write_archive)(const struct archiver *, struct archiver_args *);\ndiff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\nindex c929db5..4e49aad 100755\n--- a/t/t5003-archive-zip.sh\n+++ b/t/t5003-archive-zip.sh\n@@ -35,12 +35,56 @@ check_zip() {\n \t\"\n }\n \n+zip_text() {\n+\toption=$1\n+\tzipfile=text_$1.zip\n+\tdir=text_$1\n+\n+\ttest_expect_success \"git archive --format-zip --text=$option\" \"\n+\t\tgit archive --format=zip --text=$option HEAD >$zipfile\n+\t\"\n+\n+\ttest_expect_success UNZIP \" extract ZIP archive with EOL conversion\" '\n+\t\t(mkdir $dir && cd $dir && \"$GIT_UNZIP\" -a ../$zipfile)\n+\t'\n+}\n+\n+check_text_converted() {\n+\tdir=text_$1\n+\tfiletype=$2\n+\textracted=$dir/a/$filetype\n+\n+\ttest_expect_success \" validate that $filetype files are converted\" \"\n+\t\ttest_cmp_bin $extracted.cr $extracted.crlf &&\n+\t\ttest_cmp_bin $extracted.cr $extracted.lf\n+\t\"\n+}\n+\n+check_text_verbatim() {\n+\tdir=text_$1\n+\tfiletype=$2\n+\textracted=$dir/a/$filetype\n+\toriginal=a/$filetype\n+\n+\ttest_expect_success \" validate that $filetype files are unchanged\" \"\n+\t\ttest_cmp_bin $original.cr   $extracted.cr &&\n+\t\ttest_cmp_bin $original.crlf $extracted.crlf &&\n+\t\ttest_cmp_bin $original.lf   $extracted.lf\n+\t\"\n+}\n+\n test_expect_success \\\n     'populate workdir' \\\n     'mkdir a &&\n      echo simple textfile >a/a &&\n      mkdir a/bin &&\n      cp /bin/sh a/bin &&\n+     printf \"text\\r\"   >a/text.cr &&\n+     printf \"text\\r\\n\" >a/text.crlf &&\n+     printf \"text\\n\"   >a/text.lf &&\n+     printf \"\\0\\r\"   >a/binary.cr &&\n+     printf \"\\0\\r\\n\" >a/binary.crlf &&\n+     printf \"\\0\\n\"   >a/binary.lf &&\n      printf \"A\\$Format:%s\\$O\" \"$SUBSTFORMAT\" >a/substfile1 &&\n      printf \"A not substituted O\" >a/substfile2 &&\n      (p=long_path_to_a_file && cd a &&\n@@ -124,4 +168,16 @@ test_expect_success 'git archive --format=zip on large files' '\n \n check_zip large-compressed\n \n+zip_text all\n+check_text_converted all text\n+check_text_converted all binary\n+\n+zip_text auto\n+check_text_converted auto text\n+check_text_verbatim auto binary\n+\n+zip_text none\n+check_text_verbatim none text\n+check_text_verbatim none binary\n+\n test_done\n-- \n2.3.1\n"},{"id":"257051","messageId":"xmqq7fuw5guc.fsf@gitster.dls.corp.google.com","threadId":"38632","inReplyTo":"54F77573.80109@web.de","subject":"Re: [PATCH] archive-zip: add --text parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-05T02:16:27Z","receivedAt":"2015-03-05T02:16:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> No sign-off, yet, because I'm not sure we really need another option.\n> E.g. --text=all doesn't seem to be actually useful, but it was easy to\n> implement.  Info-ZIP's zip always creates archives like --text=auto\n> does, so perhaps we should make that our default behavior as well?\n\nMy knee-jerk reaction is \"yeah, why not? what are the downsides,\nother than the result will not be bit-for-bit identical to the\noutput from older Git\".  I am sure I am missing something as I do\nnot regularly use this format.\n\n> @@ -256,6 +264,8 @@ static int write_zip_entry(struct archiver_args *args,\n>  \t\t\t\treturn error(\"cannot read %s\",\n>  \t\t\t\t\t     sha1_to_hex(sha1));\n>  \t\t\tcrc = crc32(crc, buffer, size);\n> +\t\t\tif (is_binary < 0)\n> +\t\t\t\tis_binary = buffer_is_binary(buffer, size);\n\nIn this codepath, do you have the path of the thing the buffer\ncontents came from?  I am wondering if consulting the attributes\nsystem is a better idea. Anything that is explicitly marked as\n\"binary\" or \"-diff\" is definitely binary, and anything that is not\nmarked as \"binary\" is text to us for all practical purposes, no?\n"},{"id":"257081","messageId":"54F875EC.2020600@web.de","threadId":"38632","inReplyTo":"xmqq7fuw5guc.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] archive-zip: add --text parameter","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-05T15:27:40Z","receivedAt":"2015-03-05T15:27:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 05.03.2015 um 03:16 schrieb Junio C Hamano:\n> René Scharfe <l.s.r@web.de> writes:\n>\n>> No sign-off, yet, because I'm not sure we really need another option.\n>> E.g. --text=all doesn't seem to be actually useful, but it was easy to\n>> implement.  Info-ZIP's zip always creates archives like --text=auto\n>> does, so perhaps we should make that our default behavior as well?\n>\n> My knee-jerk reaction is \"yeah, why not? what are the downsides,\n> other than the result will not be bit-for-bit identical to the\n> output from older Git\".  I am sure I am missing something as I do\n> not regularly use this format.\n\nAFAICS there won't be any other downsides.  And archive stability is \nharder to achieve for ZIP anyway because it depends on compression level \nand (more fundamentally) on libz version.\n\n>> @@ -256,6 +264,8 @@ static int write_zip_entry(struct archiver_args *args,\n>>   \t\t\t\treturn error(\"cannot read %s\",\n>>   \t\t\t\t\t     sha1_to_hex(sha1));\n>>   \t\t\tcrc = crc32(crc, buffer, size);\n>> +\t\t\tif (is_binary < 0)\n>> +\t\t\t\tis_binary = buffer_is_binary(buffer, size);\n>\n> In this codepath, do you have the path of the thing the buffer\n> contents came from?  I am wondering if consulting the attributes\n> system is a better idea. Anything that is explicitly marked as\n> \"binary\" or \"-diff\" is definitely binary, and anything that is not\n> marked as \"binary\" is text to us for all practical purposes, no?\n\nYes, attributes can help, especially to allow users to correct wrong \nguesses of the heuristic.  Offering automatic detection of binary files \nby default like git diff and git grep is still a good idea, I think. \nbuffer_is_binary() doesn't add a lot of overhead since it only looks at \nthe first few bytes of the buffer.\n\nRené\n"},{"id":"257080","messageId":"54F875F4.1080508@web.de","threadId":"38632","inReplyTo":"54F77573.80109@web.de","subject":"Re: [PATCH] archive-zip: add --text parameter","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-05T15:27:48Z","receivedAt":"2015-03-05T15:27:48Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 04.03.2015 um 22:13 schrieb René Scharfe:\n> diff --git a/archive-zip.c b/archive-zip.c\n> index 4bde019..3767940 100644\n> --- a/archive-zip.c\n> +++ b/archive-zip.c\n> @@ -5,6 +5,7 @@\n>   #include \"archive.h\"\n>   #include \"streaming.h\"\n>   #include \"utf8.h\"\n> +#include \"xdiff-interface.h\"\n>\n>   static int zip_date;\n>   static int zip_time;\n> @@ -210,6 +211,7 @@ static int write_zip_entry(struct archiver_args *args,\n>   \tstruct git_istream *stream = NULL;\n>   \tunsigned long flags = 0;\n>   \tunsigned long size;\n> +\tint is_binary = -1;\n>\n>   \tcrc = crc32(0, NULL, 0);\n>\n> @@ -238,8 +240,14 @@ static int write_zip_entry(struct archiver_args *args,\n>   \t\tmethod = 0;\n>   \t\tattr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) :\n>   \t\t\t(mode & 0111) ? ((mode) << 16) : 0;\n> -\t\tif (S_ISREG(mode) && args->compression_level != 0 && size > 0)\n> -\t\t\tmethod = 8;\n> +\t\tif (S_ISREG(mode)) {\n> +\t\t\tif (args->compression_level != 0 && size > 0)\n> +\t\t\t\tmethod = 8;\n> +\t\t\tif (args->text == ARCHIVE_TEXT_ALL)\n> +\t\t\t\tis_binary = 0;\n> +\t\t\telse if (args->text == ARCHIVE_TEXT_NONE)\n> +\t\t\t\tis_binary = 1;\n> +\t\t}\n>\n>   \t\tif (S_ISREG(mode) && type == OBJ_BLOB && !args->convert &&\n>   \t\t    size > big_file_threshold) {\n> @@ -256,6 +264,8 @@ static int write_zip_entry(struct archiver_args *args,\n>   \t\t\t\treturn error(\"cannot read %s\",\n>   \t\t\t\t\t     sha1_to_hex(sha1));\n>   \t\t\tcrc = crc32(crc, buffer, size);\n> +\t\t\tif (is_binary < 0)\n> +\t\t\t\tis_binary = buffer_is_binary(buffer, size);\n>   \t\t\tout = buffer;\n>   \t\t}\n>   \t\tcompressed_size = (method == 0) ? size : 0;\n> @@ -300,7 +310,6 @@ static int write_zip_entry(struct archiver_args *args,\n>   \tcopy_le16(dirent.extra_length, ZIP_EXTRA_MTIME_SIZE);\n>   \tcopy_le16(dirent.comment_length, 0);\n>   \tcopy_le16(dirent.disk, 0);\n> -\tcopy_le16(dirent.attr1, 0);\n>   \tcopy_le32(dirent.attr2, attr2);\n>   \tcopy_le32(dirent.offset, zip_offset);\n>\n> @@ -328,6 +337,8 @@ static int write_zip_entry(struct archiver_args *args,\n>   \t\t\tif (readlen <= 0)\n>   \t\t\t\tbreak;\n>   \t\t\tcrc = crc32(crc, buf, readlen);\n> +\t\t\tif (is_binary < 0)\n> +\t\t\t\tis_binary = buffer_is_binary(buffer, size);\n\nbuffer is NULL here, so this crashes.  buf and readlen have to be used \ninstead.  This code path is only used for entries that are too big to be \ncompressed in one go.\n\n>   \t\t\twrite_or_die(1, buf, readlen);\n>   \t\t}\n>   \t\tclose_istream(stream);\n> @@ -361,6 +372,8 @@ static int write_zip_entry(struct archiver_args *args,\n>   \t\t\tif (readlen <= 0)\n>   \t\t\t\tbreak;\n>   \t\t\tcrc = crc32(crc, buf, readlen);\n> +\t\t\tif (is_binary < 0)\n> +\t\t\t\tis_binary = buffer_is_binary(buffer, size);\n\nSame here.\n\n>   \t\t\tzstream.next_in = buf;\n>   \t\t\tzstream.avail_in = readlen;\n> @@ -405,6 +418,8 @@ static int write_zip_entry(struct archiver_args *args,\n>   \tfree(deflated);\n>   \tfree(buffer);\n>\n> +\tcopy_le16(dirent.attr1, !is_binary);\n> +\n>   \tmemcpy(zip_dir + zip_dir_offset, &dirent, ZIP_DIR_HEADER_SIZE);\n>   \tzip_dir_offset += ZIP_DIR_HEADER_SIZE;\n>   \tmemcpy(zip_dir + zip_dir_offset, path, pathlen);\n> @@ -466,7 +481,7 @@ static int write_zip_archive(const struct archiver *ar,\n>   static struct archiver zip_archiver = {\n>   \t\"zip\",\n>   \twrite_zip_archive,\n> -\tARCHIVER_WANT_COMPRESSION_LEVELS|ARCHIVER_REMOTE\n> +\tARCHIVER_WANT_COMPRESSION_LEVELS|ARCHIVER_REMOTE|ARCHIVER_TEXT_ATTRIBUTE\n>   };\n>\n>   void init_zip_archiver(void)\n"},{"id":"257098","messageId":"54F8A91A.2000003@web.de","threadId":"38632","inReplyTo":"54F77573.80109@web.de","subject":"[PATCH v2] archive-zip: mark text files in archives","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-05T19:06:02Z","receivedAt":"2015-03-05T19:06:02Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Set the text flag for ZIP archive entries that look like text files so\nthat unzip -a can be used to perform end-of-line conversions.  Info-ZIP\nzip does the same.\n\nDetect binary files the same way as git diff and git grep do, namely by\nchecking for the attribute \"diff\" and its negation \"-diff\", and if none\nis found by falling back to checking for the presence of NUL bytes in\nthe first few bytes of the file contents.\n\n7-Zip, Windows' built-in ZIP functionality and Info-ZIP unzip without\nthe switch -a are not affected by the change and still extract text\nfiles without doing any end-of-line conversions.\n\nNB: The actual end-of-line style used in the archive entries doesn't\nmatter to unzip -a, as it converts any CR, CRLF and LF to the line end\ncharacters appropriate for the platform it is running on.\n\nSuggested-by: Ulrike Fischer <luatex@nililand.de>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n archive-zip.c          | 25 ++++++++++++++++++++++++-\n t/t5003-archive-zip.sh | 47 ++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 70 insertions(+), 2 deletions(-)\n\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 4bde019..0f9e87f 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -5,6 +5,8 @@\n #include \"archive.h\"\n #include \"streaming.h\"\n #include \"utf8.h\"\n+#include \"userdiff.h\"\n+#include \"xdiff-interface.h\"\n \n static int zip_date;\n static int zip_time;\n@@ -189,6 +191,16 @@ static int has_only_ascii(const char *s)\n \t}\n }\n \n+static int entry_is_binary(const char *path, const void *buffer, size_t size)\n+{\n+\tstruct userdiff_driver *driver = userdiff_find_by_path(path);\n+\tif (!driver)\n+\t\tdriver = userdiff_find_by_name(\"default\");\n+\tif (driver->binary != -1)\n+\t\treturn driver->binary;\n+\treturn buffer_is_binary(buffer, size);\n+}\n+\n #define STREAM_BUFFER_SIZE (1024 * 16)\n \n static int write_zip_entry(struct archiver_args *args,\n@@ -210,6 +222,8 @@ static int write_zip_entry(struct archiver_args *args,\n \tstruct git_istream *stream = NULL;\n \tunsigned long flags = 0;\n \tunsigned long size;\n+\tint is_binary = -1;\n+\tconst char *path_without_prefix = path + args->baselen;\n \n \tcrc = crc32(0, NULL, 0);\n \n@@ -256,6 +270,8 @@ static int write_zip_entry(struct archiver_args *args,\n \t\t\t\treturn error(\"cannot read %s\",\n \t\t\t\t\t     sha1_to_hex(sha1));\n \t\t\tcrc = crc32(crc, buffer, size);\n+\t\t\tis_binary = entry_is_binary(path_without_prefix,\n+\t\t\t\t\t\t    buffer, size);\n \t\t\tout = buffer;\n \t\t}\n \t\tcompressed_size = (method == 0) ? size : 0;\n@@ -300,7 +316,6 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(dirent.extra_length, ZIP_EXTRA_MTIME_SIZE);\n \tcopy_le16(dirent.comment_length, 0);\n \tcopy_le16(dirent.disk, 0);\n-\tcopy_le16(dirent.attr1, 0);\n \tcopy_le32(dirent.attr2, attr2);\n \tcopy_le32(dirent.offset, zip_offset);\n \n@@ -328,6 +343,9 @@ static int write_zip_entry(struct archiver_args *args,\n \t\t\tif (readlen <= 0)\n \t\t\t\tbreak;\n \t\t\tcrc = crc32(crc, buf, readlen);\n+\t\t\tif (is_binary == -1)\n+\t\t\t\tis_binary = entry_is_binary(path_without_prefix,\n+\t\t\t\t\t\t\t    buf, readlen);\n \t\t\twrite_or_die(1, buf, readlen);\n \t\t}\n \t\tclose_istream(stream);\n@@ -361,6 +379,9 @@ static int write_zip_entry(struct archiver_args *args,\n \t\t\tif (readlen <= 0)\n \t\t\t\tbreak;\n \t\t\tcrc = crc32(crc, buf, readlen);\n+\t\t\tif (is_binary == -1)\n+\t\t\t\tis_binary = entry_is_binary(path_without_prefix,\n+\t\t\t\t\t\t\t    buf, readlen);\n \n \t\t\tzstream.next_in = buf;\n \t\t\tzstream.avail_in = readlen;\n@@ -405,6 +426,8 @@ static int write_zip_entry(struct archiver_args *args,\n \tfree(deflated);\n \tfree(buffer);\n \n+\tcopy_le16(dirent.attr1, !is_binary);\n+\n \tmemcpy(zip_dir + zip_dir_offset, &dirent, ZIP_DIR_HEADER_SIZE);\n \tzip_dir_offset += ZIP_DIR_HEADER_SIZE;\n \tmemcpy(zip_dir + zip_dir_offset, path, pathlen);\ndiff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\nindex c929db5..14744b2 100755\n--- a/t/t5003-archive-zip.sh\n+++ b/t/t5003-archive-zip.sh\n@@ -33,6 +33,37 @@ check_zip() {\n \ttest_expect_success UNZIP \" validate file contents\" \"\n \t\tdiff -r a ${dir_with_prefix}a\n \t\"\n+\n+\tdir=eol_$1\n+\tdir_with_prefix=$dir/$2\n+\textracted=${dir_with_prefix}a\n+\toriginal=a\n+\n+\ttest_expect_success UNZIP \" extract ZIP archive with EOL conversion\" '\n+\t\t(mkdir $dir && cd $dir && \"$GIT_UNZIP\" -a ../$zipfile)\n+\t'\n+\n+\ttest_expect_success UNZIP \" validate that text files are converted\" \"\n+\t\ttest_cmp_bin $extracted/text.cr $extracted/text.crlf &&\n+\t\ttest_cmp_bin $extracted/text.cr $extracted/text.lf\n+\t\"\n+\n+\ttest_expect_success UNZIP \" validate that binary files are unchanged\" \"\n+\t\ttest_cmp_bin $original/binary.cr   $extracted/binary.cr &&\n+\t\ttest_cmp_bin $original/binary.crlf $extracted/binary.crlf &&\n+\t\ttest_cmp_bin $original/binary.lf   $extracted/binary.lf\n+\t\"\n+\n+\ttest_expect_success UNZIP \" validate that diff files are converted\" \"\n+\t\ttest_cmp_bin $extracted/diff.cr $extracted/diff.crlf &&\n+\t\ttest_cmp_bin $extracted/diff.cr $extracted/diff.lf\n+\t\"\n+\n+\ttest_expect_success UNZIP \" validate that -diff files are unchanged\" \"\n+\t\ttest_cmp_bin $original/nodiff.cr   $extracted/nodiff.cr &&\n+\t\ttest_cmp_bin $original/nodiff.crlf $extracted/nodiff.crlf &&\n+\t\ttest_cmp_bin $original/nodiff.lf   $extracted/nodiff.lf\n+\t\"\n }\n \n test_expect_success \\\n@@ -41,6 +72,18 @@ test_expect_success \\\n      echo simple textfile >a/a &&\n      mkdir a/bin &&\n      cp /bin/sh a/bin &&\n+     printf \"text\\r\"\t>a/text.cr &&\n+     printf \"text\\r\\n\"\t>a/text.crlf &&\n+     printf \"text\\n\"\t>a/text.lf &&\n+     printf \"text\\r\"\t>a/nodiff.cr &&\n+     printf \"text\\r\\n\"\t>a/nodiff.crlf &&\n+     printf \"text\\n\"\t>a/nodiff.lf &&\n+     printf \"\\0\\r\"\t>a/binary.cr &&\n+     printf \"\\0\\r\\n\"\t>a/binary.crlf &&\n+     printf \"\\0\\n\"\t>a/binary.lf &&\n+     printf \"\\0\\r\"\t>a/diff.cr &&\n+     printf \"\\0\\r\\n\"\t>a/diff.crlf &&\n+     printf \"\\0\\n\"\t>a/diff.lf &&\n      printf \"A\\$Format:%s\\$O\" \"$SUBSTFORMAT\" >a/substfile1 &&\n      printf \"A not substituted O\" >a/substfile2 &&\n      (p=long_path_to_a_file && cd a &&\n@@ -66,7 +109,9 @@ test_expect_success 'add files to repository' '\n \tGIT_COMMITTER_DATE=\"2005-05-27 22:00\" git commit -m initial\n '\n \n-test_expect_success 'setup export-subst' '\n+test_expect_success 'setup export-subst and diff attributes' '\n+\techo \"a/nodiff.* -diff\" >>.git/info/attributes &&\n+\techo \"a/diff.* diff\" >>.git/info/attributes &&\n \techo \"substfile?\" export-subst >>.git/info/attributes &&\n \tgit log --max-count=1 \"--pretty=format:A${SUBSTFORMAT}O\" HEAD \\\n \t\t>a/substfile1\n-- \n2.3.1\n"}]}