{"thread":{"id":"42754","subject":"[PATCH v4 0/5] friendlier handling of overflows in archive-tar","startedAt":"2016-06-30T09:06:32Z","lastAt":"2016-07-16T06:28:41Z","messageCount":42,"participants":["Jeff King","Eric Sunshine","Junio C Hamano","Johannes Schindelin","Johannes Sixt","Torsten Bögershausen","Duy Nguyen"],"isPatch":true,"patchVersion":4,"patchTotal":5},"messages":[{"id":"290574","messageId":"20160630090614.GA16725@sigill.intra.peff.net","threadId":"42754","inReplyTo":null,"subject":"[PATCH v4 0/5] friendlier handling of overflows in archive-tar","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-30T09:06:14Z","receivedAt":"2016-06-30T09:06:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is a re-roll of the jk/big-and-future-archive-tar topic. It\naddresses all but one of the review comments, and I hope should be\npretty polished.\n\nThe changes are:\n\n - the dependency on bunzip2 is dropped; instead, we just provide a\n partial object for the 64GB blob. See the first commit message for\n details.\n\n - the portable \"head -c\" replacement from t9300 has been factored out,\n and we use it in the new tests\n\n - symbolic constants for the giant octal numbers (with a comment\n warning that the values are set by posix)\n\n - the comments for tar_info() and the lazy-prereq were split so the\n two aren't mashed together\n\n - uses awk in tar_info() instead of \"sed | cut\"\n\n - extra simplification in the final commit, as suggested by review\n\n - typo and awkwardness fixes in the commit messages\n\nThe one thing that isn't fixed is the use of \"141\" to test for sigpipe\ndeath. That should use test_match_signal, but that topic just got\nre-rolled, too.\n\n [1/5]: t9300: factor out portable \"head -c\" replacement\n [2/5]: t5000: test tar files that overflow ustar headers\n [3/5]: archive-tar: write extended headers for file sizes >= 8GB\n [4/5]: archive-tar: write extended headers for far-future mtime\n [5/5]: archive-tar: drop return value\n\n-Peff\n"},{"id":"290575","messageId":"20160630090926.GE17463@sigill.intra.peff.net","threadId":"42754","inReplyTo":"20160630090614.GA16725@sigill.intra.peff.net","subject":"[PATCH v4 5/5] archive-tar: drop return value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-30T09:09:26Z","receivedAt":"2016-06-30T09:10:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We never do any error checks, and so never return anything\nbut \"0\". Let's just drop this to simplify the code.\n\nSigned-off-by: Jeff King \n---\n archive-tar.c | 11 ++++-------\n 1 file changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex d671bc3..7ea4e90 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -319,13 +319,12 @@ static int write_tar_entry(struct archiver_args *args,\n \treturn err;\n }\n \n-static int write_global_extended_header(struct archiver_args *args)\n+static void write_global_extended_header(struct archiver_args *args)\n {\n \tconst unsigned char *sha1 = args->commit_sha1;\n \tstruct strbuf ext_header = STRBUF_INIT;\n \tstruct ustar_header header;\n \tunsigned int mode;\n-\tint err = 0;\n \n \tif (sha1)\n \t\tstrbuf_append_ext_header(&ext_header, \"comment\",\n@@ -337,7 +336,7 @@ static int write_global_extended_header(struct archiver_args *args)\n \t}\n \n \tif (!ext_header.len)\n-\t\treturn 0;\n+\t\treturn;\n \n \tmemset(&header, 0, sizeof(header));\n \t*header.typeflag = TYPEFLAG_GLOBAL_HEADER;\n@@ -347,7 +346,6 @@ static int write_global_extended_header(struct archiver_args *args)\n \twrite_blocked(&header, sizeof(header));\n \twrite_blocked(ext_header.buf, ext_header.len);\n \tstrbuf_release(&ext_header);\n-\treturn err;\n }\n \n static struct archiver **tar_filters;\n@@ -423,9 +421,8 @@ static int write_tar_archive(const struct archiver *ar,\n {\n \tint err = 0;\n \n-\terr = write_global_extended_header(args);\n-\tif (!err)\n-\t\terr = write_archive_entries(args, write_tar_entry);\n+\twrite_global_extended_header(args);\n+\terr = write_archive_entries(args, write_tar_entry);\n \tif (!err)\n \t\twrite_trailer();\n \treturn err;\n-- \n2.9.0.317.g65b4e7c\n"},{"id":"290576","messageId":"20160630090916.GC17463@sigill.intra.peff.net","threadId":"42754","inReplyTo":"20160630090614.GA16725@sigill.intra.peff.net","subject":"[PATCH v4 3/5] archive-tar: write extended headers for file sizes >= 8GB","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-30T09:09:16Z","receivedAt":"2016-06-30T09:11:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The ustar format has a fixed-length field for the size of\neach file entry which is supposed to contain up to 11 bytes\nof octal-formatted data plus a NUL or space terminator.\n\nThese means that the largest size we can represent is\n077777777777, or 1 byte short of 8GB. The correct solution\nfor a larger file, according to POSIX.1-2001, is to add an\nextended pax header, similar to how we handle long\nfilenames. This patch does that, and writes zero for the\nsize field in the ustar header (the last bit is not\nmentioned by POSIX, but it matches how GNU tar behaves with\n--format=pax).\n\nThis should be a strict improvement over the current\nbehavior, which is to die in xsnprintf with a \"BUG\".\nHowever, there's some interesting history here.\n\nPrior to f2f0267 (archive-tar: use xsnprintf for trivial\nformatting, 2015-09-24), we silently overflowed the \"size\"\nfield. The extra bytes ended up in the \"mtime\" field of the\nheader, which was then immediately written itself,\noverwriting our extra bytes. What that means depends on how\nmany bytes we wrote.\n\nIf the size was 64GB or greater, then we actually overflowed\ndigits into the mtime field, meaning our value was\neffectively right-shifted by those lost octal digits. And\nthis patch is again a strict improvement over that.\n\nBut if the size was between 8GB and 64GB, then our 12-byte\nfield held all of the actual digits, and only our NUL\nterminator overflowed. According to POSIX, there should be a\nNUL or space at the end of the field. However, GNU tar seems\nto be lenient here, and will correctly parse a size up 64GB\n(minus one) from the field. So sizes in this range might\nhave just worked, depending on the implementation reading\nthe tarfile.\n\nThis patch is mostly still an improvement there, as the 8GB\nlimit is specifically mentioned in POSIX as the correct\nlimit. But it's possible that it could be a regression\n(versus the pre-f2f0267 state) if all of the following are\ntrue:\n\n 1. You have a file between 8GB and 64GB.\n\n 2. Your tar implementation _doesn't_ know about pax\n extended headers.\n\n 3. Your tar implementation _does_ parse 12-byte sizes from\n the ustar header without a delimiter.\n\nIt's probably not worth worrying about such an obscure set\nof conditions, but I'm documenting it here just in case.\n\nHelped-by: René Scharfe \nSigned-off-by: Jeff King \n---\n archive-tar.c | 31 +++++++++++++++++++++++++++++--\n t/t5000-tar-tree.sh | 4 ++--\n 2 files changed, 31 insertions(+), 4 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex cb99df2..57a1540 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -18,6 +18,13 @@ static int tar_umask = 002;\n static int write_tar_filter_archive(const struct archiver *ar,\n \t\t\t\t struct archiver_args *args);\n \n+/*\n+ * This is the max value that a ustar size header can specify, as it is fixed\n+ * at 11 octal digits. POSIX specifies that we switch to extended headers at\n+ * this size.\n+ */\n+#define USTAR_MAX_SIZE 077777777777UL\n+\n /* writes out the whole block, but only if it is full */\n static void write_if_needed(void)\n {\n@@ -137,6 +144,20 @@ static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,\n \tstrbuf_addch(sb, '\\n');\n }\n \n+/*\n+ * Like strbuf_append_ext_header, but for numeric values.\n+ */\n+static void strbuf_append_ext_header_uint(struct strbuf *sb,\n+\t\t\t\t\t const char *keyword,\n+\t\t\t\t\t uintmax_t value)\n+{\n+\tchar buf[40]; /* big enough for 2^128 in decimal, plus NUL */\n+\tint len;\n+\n+\tlen = xsnprintf(buf, sizeof(buf), \"%\"PRIuMAX, value);\n+\tstrbuf_append_ext_header(sb, keyword, buf, len);\n+}\n+\n static unsigned int ustar_header_chksum(const struct ustar_header *header)\n {\n \tconst unsigned char *p = (const unsigned char *)header;\n@@ -208,7 +229,7 @@ static int write_tar_entry(struct archiver_args *args,\n \tstruct ustar_header header;\n \tstruct strbuf ext_header = STRBUF_INIT;\n \tunsigned int old_mode = mode;\n-\tunsigned long size;\n+\tunsigned long size, size_in_header;\n \tvoid *buffer;\n \tint err = 0;\n \n@@ -267,7 +288,13 @@ static int write_tar_entry(struct archiver_args *args,\n \t\t\tmemcpy(header.linkname, buffer, size);\n \t}\n \n-\tprepare_header(args, &header, mode, size);\n+\tsize_in_header = size;\n+\tif (S_ISREG(mode) && size > USTAR_MAX_SIZE) {\n+\t\tsize_in_header = 0;\n+\t\tstrbuf_append_ext_header_uint(&ext_header, \"size\", size);\n+\t}\n+\n+\tprepare_header(args, &header, mode, size_in_header);\n \n \tif (ext_header.len > 0) {\n \t\terr = write_extended_header(args, sha1, ext_header.buf,\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 950bdd3..93c2d34 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -360,7 +360,7 @@ test_expect_success 'set up repository with huge blob' '\n \n # We expect git to die with SIGPIPE here (otherwise we\n # would generate the whole 64GB).\n-test_expect_failure 'generate tar with huge size' '\n+test_expect_success 'generate tar with huge size' '\n \t{\n \t\tgit archive HEAD\n \t\techo $? >exit-code\n@@ -369,7 +369,7 @@ test_expect_failure 'generate tar with huge size' '\n \ttest_cmp expect exit-code\n '\n \n-test_expect_failure TAR_HUGE 'system tar can read our huge size' '\n+test_expect_success TAR_HUGE 'system tar can read our huge size' '\n \techo 68719476737 >expect &&\n \ttar_info huge.tar | cut -d\" \" -f1 >actual &&\n \ttest_cmp expect actual\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290577","messageId":"20160630090920.GD17463@sigill.intra.peff.net","threadId":"42754","inReplyTo":"20160630090614.GA16725@sigill.intra.peff.net","subject":"[PATCH v4 4/5] archive-tar: write extended headers for far-future mtime","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-30T09:09:20Z","receivedAt":"2016-06-30T09:12:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The ustar format represents timestamps as seconds since the\nepoch, but only has room to store 11 octal digits. To\nexpress anything larger, we need to use an extended header.\nThis is exactly the same case we fixed for the size field in\nthe previous commit, and the solution here follows the same\npattern.\n\nThis is even mentioned as an issue in f2f0267 (archive-tar:\nuse xsnprintf for trivial formatting, 2015-09-24), but since\nit only affected things far in the future, it wasn't deemed\nworth dealing with. But note that my calculations claiming\nthousands of years were off there; because our xsnprintf\nproduces a NUL byte, we only have until the year 2242 to fix\nthis.\n\nGiven that this is just around the corner (geologically\nspeaking, anyway), and because it's easy to fix, let's just\nmake it work. Unlike the previous fix for \"size\", where we\nhad to write an individual extended header for each file, we\ncan write one global header (since we have only one mtime\nfor the whole archive).\n\nThere's a slight bit of trickiness there. We may already be\nwriting a global header with a \"comment\" field for the\ncommit sha1. So we need to write our new field into the same\nheader. To do this, we push the decision of whether to write\nsuch a header down into write_global_extended_header(),\nwhich will now assemble the header as it sees fit, and will\nreturn early if we have nothing to write (in practice, we'll\nonly have a large mtime if it comes from a commit, but this\nmakes it also work if you set your system clock ahead such\nthat time() returns a huge value).\n\nNote that we don't (and never did) handle negative\ntimestamps (i.e., before 1970). This would probably not be\ntoo hard to support in the same way, but since git does not\nsupport negative timestamps at all, I didn't bother here.\n\nAfter writing the extended header, we munge the timestamp in\nthe ustar headers to the maximum-allowable size. This is\nwrong, but it's the least-wrong thing we can provide to a\ntar implementation that doesn't understand pax headers (it's\nalso what GNU tar does).\n\nHelped-by: René Scharfe \nSigned-off-by: Jeff King \n---\n archive-tar.c | 19 ++++++++++++++++---\n t/t5000-tar-tree.sh | 4 ++--\n 2 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 57a1540..d671bc3 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -22,8 +22,11 @@ static int write_tar_filter_archive(const struct archiver *ar,\n * This is the max value that a ustar size header can specify, as it is fixed\n * at 11 octal digits. POSIX specifies that we switch to extended headers at\n * this size.\n+ *\n+ * Likewise for the mtime (which happens to use a buffer of the same size).\n */\n #define USTAR_MAX_SIZE 077777777777UL\n+#define USTAR_MAX_MTIME 077777777777UL\n \n /* writes out the whole block, but only if it is full */\n static void write_if_needed(void)\n@@ -324,7 +327,18 @@ static int write_global_extended_header(struct archiver_args *args)\n \tunsigned int mode;\n \tint err = 0;\n \n-\tstrbuf_append_ext_header(&ext_header, \"comment\", sha1_to_hex(sha1), 40);\n+\tif (sha1)\n+\t\tstrbuf_append_ext_header(&ext_header, \"comment\",\n+\t\t\t\t\t sha1_to_hex(sha1), 40);\n+\tif (args->time > USTAR_MAX_MTIME) {\n+\t\tstrbuf_append_ext_header_uint(&ext_header, \"mtime\",\n+\t\t\t\t\t args->time);\n+\t\targs->time = USTAR_MAX_MTIME;\n+\t}\n+\n+\tif (!ext_header.len)\n+\t\treturn 0;\n+\n \tmemset(&header, 0, sizeof(header));\n \t*header.typeflag = TYPEFLAG_GLOBAL_HEADER;\n \tmode = 0100666;\n@@ -409,8 +423,7 @@ static int write_tar_archive(const struct archiver *ar,\n {\n \tint err = 0;\n \n-\tif (args->commit_sha1)\n-\t\terr = write_global_extended_header(args);\n+\terr = write_global_extended_header(args);\n \tif (!err)\n \t\terr = write_archive_entries(args, write_tar_entry);\n \tif (!err)\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 93c2d34..96d208d 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -383,11 +383,11 @@ test_expect_success 'set up repository with far-future commit' '\n \t\tgit commit -m \"tempori parendum\"\n '\n \n-test_expect_failure 'generate tar with future mtime' '\n+test_expect_success 'generate tar with future mtime' '\n \tgit archive HEAD >future.tar\n '\n \n-test_expect_failure TAR_HUGE 'system tar can read our future mtime' '\n+test_expect_success TAR_HUGE 'system tar can read our future mtime' '\n \techo 4147 >expect &&\n \ttar_info future.tar | cut -d\" \" -f2 >actual &&\n \ttest_cmp expect actual\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290578","messageId":"20160630090753.GA17463@sigill.intra.peff.net","threadId":"42754","inReplyTo":"20160630090614.GA16725@sigill.intra.peff.net","subject":"[PATCH v4 1/5] t9300: factor out portable \"head -c\" replacement","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-30T09:07:54Z","receivedAt":"2016-06-30T09:14:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In shell scripts it is sometimes useful to be able to read\nexactly N bytes from a pipe. Doing this portably turns out\nto be surprisingly difficult.\n\nWe want a solution that:\n\n - is portable\n\n - never reads more than N bytes due to buffering (which\n would mean those bytes are not available to the next\n program to read from the same pipe)\n\n - handles partial reads by looping until N bytes or read\n (or we see EOF)\n\n - is resilient to stray signals giving us EINTR while\n trying to read (even though we don't send them, things\n like SIGWINCH could cause apparently-random failures)\n\nSome possible solutions are:\n\n - \"head -c\" is not portable, and implementations may\n buffer (though GNU head does not)\n\n - \"read -N\" is a bash-ism, and thus not portable\n\n - \"dd bs=$n count=1\" does not handle partial reads. GNU dd\n has iflags=fullblock, but that is not portable\n\n - \"dd bs=1 count=$n\" fixes the partial read problem (all\n reads are 1-byte, so there can be no partial response).\n It does make a lot of write() calls, but for our tests\n that's unlikely to matter. It's fairly portable. We\n already use it in our tests, and it's unlikely that\n implementations would screw up any of our criteria. The\n most unknown one would be signal handling.\n\n - perl can do a sysread() loop pretty easily. On my Linux\n system, at least, it seems to restart the read() call\n automatically. If that turns out not to be portable,\n though, it would be easy for us to handle it.\n\nThat makes the perl solution the least bad (because we\nconveniently omitted \"length of code\" as a criterion).\nIt's also what t9300 is currently using, so we can just pull\nthe implementation from there.\n\nSigned-off-by: Jeff King \n---\n t/t9300-fast-import.sh | 23 +++--------------------\n t/test-lib-functions.sh | 14 ++++++++++++++\n 2 files changed, 17 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 74d740d..2e0ba3e 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -7,23 +7,6 @@ test_description='test git fast-import utility'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/diff-lib.sh ;# test-lib chdir's into trash\n \n-# Print $1 bytes from stdin to stdout.\n-#\n-# This could be written as \"head -c $1\", but IRIX \"head\" does not\n-# support the -c option.\n-head_c () {\n-\tperl -e '\n-\t\tmy $len = $ARGV[1];\n-\t\twhile ($len > 0) {\n-\t\t\tmy $s;\n-\t\t\tmy $nread = sysread(STDIN, $s, $len);\n-\t\t\tdie \"cannot read: $!\" unless defined($nread);\n-\t\t\tprint $s;\n-\t\t\t$len -= $nread;\n-\t\t}\n-\t' - \"$1\"\n-}\n-\n verify_packs () {\n \tfor p in .git/objects/pack/*.pack\n \tdo\n@@ -2481,7 +2464,7 @@ test_expect_success PIPE 'R: copy using cat-file' '\n \n \t\tread blob_id type size <&3 &&\n \t\techo \"$blob_id $type $size\" >response &&\n-\t\thead_c $size >blob <&3 &&\n+\t\ttest_copy_bytes $size >blob <&3 &&\n \t\tread newline <&3 &&\n \n \t\tcat <<-EOF &&\n@@ -2524,7 +2507,7 @@ test_expect_success PIPE 'R: print blob mid-commit' '\n \t\tEOF\n \n \t\tread blob_id type size <&3 &&\n-\t\thead_c $size >actual <&3 &&\n+\t\ttest_copy_bytes $size >actual <&3 &&\n \t\tread newline <&3 &&\n \n \t\techo\n@@ -2559,7 +2542,7 @@ test_expect_success PIPE 'R: print staged blob within commit' '\n \t\techo \"cat-blob $to_get\" &&\n \n \t\tread blob_id type size <&3 &&\n-\t\thead_c $size >actual <&3 &&\n+\t\ttest_copy_bytes $size >actual <&3 &&\n \t\tread newline <&3 &&\n \n \t\techo deleteall\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 48884d5..90856d6 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -961,3 +961,17 @@ test_env () {\n \t\tdone\n \t)\n }\n+\n+# Read up to \"$1\" bytes (or to EOF) from stdin and write them to stdout.\n+test_copy_bytes () {\n+\tperl -e '\n+\t\tmy $len = $ARGV[1];\n+\t\twhile ($len > 0) {\n+\t\t\tmy $s;\n+\t\t\tmy $nread = sysread(STDIN, $s, $len);\n+\t\t\tdie \"cannot read: $!\" unless defined($nread);\n+\t\t\tprint $s;\n+\t\t\t$len -= $nread;\n+\t\t}\n+\t' - \"$1\"\n+}\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290579","messageId":"20160630091451.GA24964@sigill.intra.peff.net","threadId":"42754","inReplyTo":"20160630090614.GA16725@sigill.intra.peff.net","subject":"[PATCH v4 6/5] t5000: use test_match_signal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-30T09:14:51Z","receivedAt":"2016-06-30T09:14:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 30, 2016 at 05:06:14AM -0400, Jeff King wrote:\n\n> The one thing that isn't fixed is the use of \"141\" to test for sigpipe\n> death. That should use test_match_signal, but that topic just got\n> re-rolled, too.\n\nAnd here's what the patch for that looks like (which can be applied if\nthis topic and jk/test-match-signal are merged).\n\n-- >8 --\nSubject: t5000: use test_match_signal\n\nWe are testing for sigpipe death from git, and doing so\nportably requires using our test helper.\n\nSigned-off-by: Jeff King \n---\n\nOf course this does not help Windows at all, because we removed the \"3\"\ncheck from jk/test-match-signal. So this new test will probably need to\nbe dealt with by Windows folks, one way or another (either extending\ntest_match_signal, or just skipping the exit code check under the MINGW\nprereq).\n\n t/t5000-tar-tree.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 96d208d..6950d7d 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -365,8 +365,7 @@ test_expect_success 'generate tar with huge size' '\n \t\tgit archive HEAD\n \t\techo $? >exit-code\n \t} | test_copy_bytes 4096 >huge.tar &&\n-\techo 141 >expect &&\n-\ttest_cmp expect exit-code\n+\ttest_match_signal 13 \"$(cat exit-code)\"\n '\n \n test_expect_success TAR_HUGE 'system tar can read our huge size' '\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290580","messageId":"20160630090857.GB17463@sigill.intra.peff.net","threadId":"42754","inReplyTo":"20160630090614.GA16725@sigill.intra.peff.net","subject":"[PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-30T09:08:57Z","receivedAt":"2016-06-30T09:15:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The ustar format only has room for 11 (or 12, depending on\nsome implementations) octal digits for the size and mtime of\neach file. For values larger than this, we have to add pax\nextended headers to specify the real data, and git does not\nyet know how to do so.\n\nBefore fixing that, let's start off with some test\ninfrastructure, as designing portable and efficient tests\nfor this is non-trivial.\n\nWe want to use the system tar to check our output (because\nwhat we really care about is interoperability), but we can't\nrely on it:\n\n 1. being able to read pax headers\n\n 2. being able to handle huge sizes or mtimes\n\n 3. supporting a \"t\" format we can parse\n\nSo as a prerequisite, we can feed the system tar a reference\ntarball to make sure it can handle these features. The\nreference tar here was created with:\n\n dd if=/dev/zero seek=64G bs=1 count=1 of=huge\n touch -d @68719476737 huge\n tar cf - --format=pax |\n head -c 2048\n\nusing GNU tar. Note that this is not a complete tarfile, but\nit's enough to contain the headers we want to examine.\n\nLikewise, we need to convince git that it has a 64GB blob to\noutput. Running \"git add\" on that 64GB file takes many\nminutes of CPU, and even compressed, the result is 64MB. So\nagain, I pre-generated that loose object, and then took only\nthe first 2k of it. That should be enough to generate 2MB of\ndata before hitting an inflate error, which is plenty for us\nto generate the tar header (and then die of SIGPIPE while\nstreaming the rest out).\n\nThe tests are split so that we test as much as we can even\nwith an uncooperative system tar. This actually catches the\ncurrent breakage (which is that we die(\"BUG\") trying to\nwrite the ustar header) on every system, and then on systems\nwhere we can, we go farther and actually verify the result.\n\nHelped-by: Robin H. Johnson \nSigned-off-by: Jeff King \n---\n t/t5000-tar-tree.sh | 74 +++++++++++++++++++++++\n t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a | Bin 0 -> 2048 bytes\n t/t5000/huge-and-future.tar | Bin 0 -> 2048 bytes\n 3 files changed, 74 insertions(+)\n create mode 100644 t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a\n create mode 100644 t/t5000/huge-and-future.tar\n\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 4b68bba..950bdd3 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -319,4 +319,78 @@ test_expect_success 'catch non-matching pathspec' '\n \ttest_must_fail git archive -v HEAD -- \"*.abc\" >/dev/null\n '\n \n+# Pull the size and date of each entry in a tarfile using the system tar.\n+#\n+# We'll pull out only the year from the date; that avoids any question of\n+# timezones impacting the result (as long as we keep our test times away from a\n+# year boundary; our reference times are all in August).\n+#\n+# The output of tar_info is expected to be \" \", both in decimal. It\n+# ignores the return value of tar. We have to do this, because some of our test\n+# input is only partial (the real data is 64GB in some cases).\n+tar_info () {\n+\t\"$TAR\" tvf \"$1\" |\n+\tawk '{\n+\t\tsplit($4, date, \"-\")\n+\t\tprint $3 \" \" date[1]\n+\t}'\n+}\n+\n+# See if our system tar can handle a tar file with huge sizes and dates far in\n+# the future, and that we can actually parse its output.\n+#\n+# The reference file was generated by GNU tar, and the magic time and size are\n+# both octal 01000000000001, which overflows normal ustar fields.\n+test_lazy_prereq TAR_HUGE '\n+\techo \"68719476737 4147\" >expect &&\n+\ttar_info \"$TEST_DIRECTORY\"/t5000/huge-and-future.tar >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'set up repository with huge blob' '\n+\tobj_d=19 &&\n+\tobj_f=f9c8273ec45a8938e6999cb59b3ff66739902a &&\n+\tobj=${obj_d}${obj_f} &&\n+\tmkdir -p .git/objects/$obj_d &&\n+\tcp \"$TEST_DIRECTORY\"/t5000/$obj .git/objects/$obj_d/$obj_f &&\n+\trm -f .git/index &&\n+\tgit update-index --add --cacheinfo 100644,$obj,huge &&\n+\tgit commit -m huge\n+'\n+\n+# We expect git to die with SIGPIPE here (otherwise we\n+# would generate the whole 64GB).\n+test_expect_failure 'generate tar with huge size' '\n+\t{\n+\t\tgit archive HEAD\n+\t\techo $? >exit-code\n+\t} | test_copy_bytes 4096 >huge.tar &&\n+\techo 141 >expect &&\n+\ttest_cmp expect exit-code\n+'\n+\n+test_expect_failure TAR_HUGE 'system tar can read our huge size' '\n+\techo 68719476737 >expect &&\n+\ttar_info huge.tar | cut -d\" \" -f1 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'set up repository with far-future commit' '\n+\trm -f .git/index &&\n+\techo content >file &&\n+\tgit add file &&\n+\tGIT_COMMITTER_DATE=\"@68719476737 +0000\" \\\n+\t\tgit commit -m \"tempori parendum\"\n+'\n+\n+test_expect_failure 'generate tar with future mtime' '\n+\tgit archive HEAD >future.tar\n+'\n+\n+test_expect_failure TAR_HUGE 'system tar can read our future mtime' '\n+\techo 4147 >expect &&\n+\ttar_info future.tar | cut -d\" \" -f2 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a b/t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..5cbe9ec312bfd7b7e0398ca281e9d42848743704\nGIT binary patch\nliteral 2048\nzcmb=p_2!@gk^{~&l;zIw((@uRaR^\n\nliteral 0\nHcmV?d00001\n\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290624","messageId":"CAPig+cS334=jN54Ab46VoNoD7uzEgLXwaNE1ePZwqgZYja33cw@mail.gmail.com","threadId":"42754","inReplyTo":"20160630090753.GA17463@sigill.intra.peff.net","subject":"Re: [PATCH v4 1/5] t9300: factor out portable \"head -c\" replacement","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-07-01T04:45:10Z","receivedAt":"2016-07-01T04:45:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thursday, June 30, 2016, Jeff King wrote:\n> In shell scripts it is sometimes useful to be able to read\n> exactly N bytes from a pipe. Doing this portably turns out\n> to be surprisingly difficult.\n>\n> We want a solution that:\n>\n> - is portable\n>\n> - never reads more than N bytes due to buffering (which\n> would mean those bytes are not available to the next\n> program to read from the same pipe)\n>\n> - handles partial reads by looping until N bytes or read\n\ns/or/are/\n\n> (or we see EOF)\n"},{"id":"290685","messageId":"xmqqd1mxz30m.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"20160630090753.GA17463@sigill.intra.peff.net","subject":"Re: [PATCH v4 1/5] t9300: factor out portable \"head -c\" replacement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-01T17:23:05Z","receivedAt":"2016-07-01T17:23:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> In shell scripts it is sometimes useful to be able to read\n> exactly N bytes from a pipe. Doing this portably turns out\n> to be surprisingly difficult.\n\nI'd rotate the above by three words ;-).\n\n It is sometimes useful to be able to read\n exactly N bytes from a pipe. Doing this portably turns out\n to be surprisingly difficult\n in shell scripts.\n\n> - \"dd bs=1 count=$n\" fixes the partial read problem (all\n> reads are 1-byte, so there can be no partial response).\n> It does make a lot of write() calls, but for our tests\n> that's unlikely to matter.\n\nIt makes me wonder if it helps to use different ibs and obs if many\nwrites bother you, but because this patch moves us away from dd,\nthat is a moot point.\n\n> That makes the perl solution the least bad (because we\n> conveniently omitted \"length of code\" as a criterion).\n> It's also what t9300 is currently using, so we can just pull\n> the implementation from there.\n\n;-).\n\nThe patch itself is good.\n"},{"id":"290691","messageId":"20160701180142.GC16235@sigill.intra.peff.net","threadId":"42754","inReplyTo":"xmqqd1mxz30m.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/5] t9300: factor out portable \"head -c\" replacement","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T18:01:43Z","receivedAt":"2016-07-01T18:08:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 01, 2016 at 10:23:05AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > In shell scripts it is sometimes useful to be able to read\n> > exactly N bytes from a pipe. Doing this portably turns out\n> > to be surprisingly difficult.\n> \n> I'd rotate the above by three words ;-).\n> \n> It is sometimes useful to be able to read\n> exactly N bytes from a pipe. Doing this portably turns out\n> to be surprisingly difficult\n> in shell scripts.\n\nYeah, I'd very much agree with that (feel free to mark it up as you\napply).\n\n> > - \"dd bs=1 count=$n\" fixes the partial read problem (all\n> > reads are 1-byte, so there can be no partial response).\n> > It does make a lot of write() calls, but for our tests\n> > that's unlikely to matter.\n> \n> It makes me wonder if it helps to use different ibs and obs if many\n> writes bother you, but because this patch moves us away from dd,\n> that is a moot point.\n\nActually, I just mis-spoke (mis-wrote?). I meant to say that it makes a\nlot of read() calls. (It probably also makes a lot of write() calls, but\nthat is beside the point).\n\n-Peff\n"},{"id":"291425","messageId":"alpine.DEB.2.20.1607141745420.6426@virtualbox","threadId":"42754","inReplyTo":"20160630090857.GB17463@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-14T15:47:41Z","receivedAt":"2016-07-14T15:48:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nsorry for the very, very late reply.\n\nOn Thu, 30 Jun 2016, Jeff King wrote:\n\n> The ustar format only has room for 11 (or 12, depending on\n> some implementations) octal digits for the size and mtime of\n> each file. For values larger than this, we have to add pax\n> extended headers to specify the real data, and git does not\n> yet know how to do so.\n>\n> [...]\n> t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a | Bin 0 -> 2048 bytes\n\nIt appears that this blob cannot be read when sizeof(unsigned long) == 4.\nThis happens to break the t5000 test on Windows, where that comparison\nholds true.\n\nI am sure that I missed some other discussion about this issue... could\nyou point me to it?\n\nCiao,\nDscho\n"},{"id":"291435","messageId":"19a2d02e-3918-3bc9-db34-66e12ab950e4@kdbg.org","threadId":"42754","inReplyTo":"alpine.DEB.2.20.1607141745420.6426@virtualbox","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-07-14T16:45:47Z","receivedAt":"2016-07-14T16:45:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.07.2016 um 17:47 schrieb Johannes Schindelin:\n> On Thu, 30 Jun 2016, Jeff King wrote:\n>> The ustar format only has room for 11 (or 12, depending on\n>> some implementations) octal digits for the size and mtime of\n>> each file. For values larger than this, we have to add pax\n>> extended headers to specify the real data, and git does not\n>> yet know how to do so.\n>>\n>> [...]\n>> t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a | Bin 0 -> 2048 bytes\n>\n> It appears that this blob cannot be read when sizeof(unsigned long) == 4.\n> This happens to break the t5000 test on Windows, where that comparison\n> holds true.\n\nThe problem occurs in parse_sha1_header_extended(), where the \ncalculation of the size in the object header overflows our 32-bit \nunsigned long.\n\n-- Hannes\n\n"},{"id":"291436","messageId":"ac016bb4-9f69-7e03-bc99-7a5bbc1a1a33@kdbg.org","threadId":"42754","inReplyTo":"20160630090916.GC17463@sigill.intra.peff.net","subject":"Re: [PATCH v4 3/5] archive-tar: write extended headers for file sizes >= 8GB","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-07-14T16:48:45Z","receivedAt":"2016-07-14T16:48:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 30.06.2016 um 11:09 schrieb Jeff King:\n> +/*\n> + * This is the max value that a ustar size header can specify, as it is fixed\n> + * at 11 octal digits. POSIX specifies that we switch to extended headers at\n> + * this size.\n> + */\n> +#define USTAR_MAX_SIZE 077777777777UL\n\nThis is too large by one bit for our 32-bit unsigned long on Windows:\n\narchive-tar.c: In function 'write_tar_entry':\narchive-tar.c:295: warning: integer constant is too large for 'unsigned \nlong' type\narchive-tar.c: In function 'write_global_extended_header':\narchive-tar.c:332: warning: integer constant is too large for 'unsigned \nlong' type\narchive-tar.c:335: warning: integer constant is too large for 'unsigned \nlong' type\narchive-tar.c:335: warning: overflow in implicit constant conversion\n\n-- Hannes\n\n"},{"id":"291439","messageId":"xmqqa8hkrvu5.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"19a2d02e-3918-3bc9-db34-66e12ab950e4@kdbg.org","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T17:08:34Z","receivedAt":"2016-07-14T17:08:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 14.07.2016 um 17:47 schrieb Johannes Schindelin:\n>> On Thu, 30 Jun 2016, Jeff King wrote:\n>>> The ustar format only has room for 11 (or 12, depending on\n>>> some implementations) octal digits for the size and mtime of\n>>> each file. For values larger than this, we have to add pax\n>>> extended headers to specify the real data, and git does not\n>>> yet know how to do so.\n>>>\n>>> [...]\n>>> t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a | Bin 0 -> 2048 bytes\n>>\n>> It appears that this blob cannot be read when sizeof(unsigned long) == 4.\n>> This happens to break the t5000 test on Windows, where that comparison\n>> holds true.\n>\n> The problem occurs in parse_sha1_header_extended(), where the\n> calculation of the size in the object header overflows our 32-bit\n> unsigned long.\n\nOK, then we'd want something that measures how big \"unsigned long\"\nis, and use it as a lazy prerequisite 64BIT_LONG, tweaking the other\npatch to t0006 the other Johannes sent yesterday.\n\nDscho? I'll revert the merge of 'js/t0006-for-v2.9.2' out of\n'next' so that we can replace it with your newer version, but it\nneeds to be massaged to lose the strong linkage with \"time\", as\nit is no longer \"is our time big enough\", I would think.\n\nThanks.\n"},{"id":"291440","messageId":"xmqq60s8rvpl.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"ac016bb4-9f69-7e03-bc99-7a5bbc1a1a33@kdbg.org","subject":"Re: [PATCH v4 3/5] archive-tar: write extended headers for file sizes >= 8GB","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T17:11:18Z","receivedAt":"2016-07-14T17:11:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 30.06.2016 um 11:09 schrieb Jeff King:\n>> +/*\n>> + * This is the max value that a ustar size header can specify, as it is fixed\n>> + * at 11 octal digits. POSIX specifies that we switch to extended headers at\n>> + * this size.\n>> + */\n>> +#define USTAR_MAX_SIZE 077777777777UL\n>\n> This is too large by one bit for our 32-bit unsigned long on Windows:\n>\n> archive-tar.c: In function 'write_tar_entry':\n> archive-tar.c:295: warning: integer constant is too large for\n> unsigned long' type\n> archive-tar.c: In function 'write_global_extended_header':\n> archive-tar.c:332: warning: integer constant is too large for\n> unsigned long' type\n> archive-tar.c:335: warning: integer constant is too large for\n> unsigned long' type\n> archive-tar.c:335: warning: overflow in implicit constant conversion\n\nYikes. I guess we need ULL here, and it cascade to all the\nvariables used to interact with this constant, but not everybody has\n\"long long\", so....\n\n"},{"id":"291447","messageId":"20160714181637.GA16497@sigill.intra.peff.net","threadId":"42754","inReplyTo":"xmqq60s8rvpl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 3/5] archive-tar: write extended headers for file sizes >= 8GB","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T18:16:38Z","receivedAt":"2016-07-14T18:16:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 10:11:18AM -0700, Junio C Hamano wrote:\n\n> Johannes Sixt writes:\n> \n> > Am 30.06.2016 um 11:09 schrieb Jeff King:\n> >> +/*\n> >> + * This is the max value that a ustar size header can specify, as it is fixed\n> >> + * at 11 octal digits. POSIX specifies that we switch to extended headers at\n> >> + * this size.\n> >> + */\n> >> +#define USTAR_MAX_SIZE 077777777777UL\n> >\n> > This is too large by one bit for our 32-bit unsigned long on Windows:\n> >\n> > archive-tar.c: In function 'write_tar_entry':\n> > archive-tar.c:295: warning: integer constant is too large for\n> > unsigned long' type\n> > archive-tar.c: In function 'write_global_extended_header':\n> > archive-tar.c:332: warning: integer constant is too large for\n> > unsigned long' type\n> > archive-tar.c:335: warning: integer constant is too large for\n> > unsigned long' type\n> > archive-tar.c:335: warning: overflow in implicit constant conversion\n> \n> Yikes. I guess we need ULL here, and it cascade to all the\n> variables used to interact with this constant, but not everybody has\n> \"long long\", so....\n\nOn 32-bit systems, I suspect the tar limits are a non-issue. For real\nfilesystem operations, tar on such a system would use off_t. But we use\n\"unsigned long\" internally for object sizes, so I imagine that objects\nlarger than 4G are simply impossible on such systems.\n\nSo one option would be to simply \"#if\"-out these checks on 32-bit\nsystems.\n\nI think it would also be OK to just set USTAR_MAX_SIZE to ULONG_MAX on\nsuch a system, too (which effectively eliminates the check, but keeps\nthe conditional bits contained).\n\n-Peff\n"},{"id":"291449","messageId":"20160714182108.GB16497@sigill.intra.peff.net","threadId":"42754","inReplyTo":"alpine.DEB.2.20.1607141745420.6426@virtualbox","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T18:21:08Z","receivedAt":"2016-07-14T18:21:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 05:47:41PM +0200, Johannes Schindelin wrote:\n\n> On Thu, 30 Jun 2016, Jeff King wrote:\n> \n> > The ustar format only has room for 11 (or 12, depending on\n> > some implementations) octal digits for the size and mtime of\n> > each file. For values larger than this, we have to add pax\n> > extended headers to specify the real data, and git does not\n> > yet know how to do so.\n> >\n> > [...]\n> > t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a | Bin 0 -> 2048 bytes\n> \n> It appears that this blob cannot be read when sizeof(unsigned long) == 4.\n> This happens to break the t5000 test on Windows, where that comparison\n> holds true.\n> \n> I am sure that I missed some other discussion about this issue... could\n> you point me to it?\n\nThere's tons of discussion in:\n\n http://thread.gmane.org/gmane.comp.version-control.git/297409\n\nbut frankly it is not worth your time to read it. These tests are about\noverflowing the tar limits, which can only happen with times and sizes\ngreater than 32-bits. The right thing to do is to skip the tests\nentirely on systems where sizeof(unsigned long) is less than 8 (the\nactual value is 64GB+1, so technically a 37-bit system would work, but I\nthink it is OK for the test-skipping to be less specific).\n\n-Peff\n"},{"id":"291450","messageId":"20160714182427.GC16497@sigill.intra.peff.net","threadId":"42754","inReplyTo":"19a2d02e-3918-3bc9-db34-66e12ab950e4@kdbg.org","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T18:24:27Z","receivedAt":"2016-07-14T18:24:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 06:45:47PM +0200, Johannes Sixt wrote:\n\n> Am 14.07.2016 um 17:47 schrieb Johannes Schindelin:\n> > On Thu, 30 Jun 2016, Jeff King wrote:\n> > > The ustar format only has room for 11 (or 12, depending on\n> > > some implementations) octal digits for the size and mtime of\n> > > each file. For values larger than this, we have to add pax\n> > > extended headers to specify the real data, and git does not\n> > > yet know how to do so.\n> > > \n> > > [...]\n> > > t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a | Bin 0 -> 2048 bytes\n> > \n> > It appears that this blob cannot be read when sizeof(unsigned long) == 4.\n> > This happens to break the t5000 test on Windows, where that comparison\n> > holds true.\n> \n> The problem occurs in parse_sha1_header_extended(), where the calculation of\n> the size in the object header overflows our 32-bit unsigned long.\n\nYep, unsurprising.\n\nI do think git is wrong to use \"unsigned long\" there. It really ought to\nbe \"size_t\". My understanding is that 64-bit Windows is LLP64, which\nmeans that you cannot currently have blobs greater than 4GB there, even\nthough it would work correctly with size_t.\n\nSwitching all of the things that look at blob sizes to size_t will be a\nbig job, though.\n\n-Peff\n"},{"id":"291467","messageId":"xmqqshvcourb.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"20160714182108.GB16497@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T20:00:08Z","receivedAt":"2016-07-14T20:00:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Jul 14, 2016 at 05:47:41PM +0200, Johannes Schindelin wrote:\n>\n>> On Thu, 30 Jun 2016, Jeff King wrote:\n>> \n>> > The ustar format only has room for 11 (or 12, depending on\n>> > some implementations) octal digits for the size and mtime of\n>> > each file. For values larger than this, we have to add pax\n>> > extended headers to specify the real data, and git does not\n>> > yet know how to do so.\n>> >\n>> > [...]\n>> > t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a | Bin 0 -> 2048 bytes\n>> \n>> It appears that this blob cannot be read when sizeof(unsigned long) == 4.\n>> This happens to break the t5000 test on Windows, where that comparison\n>> holds true.\n>> \n>> I am sure that I missed some other discussion about this issue... could\n>> you point me to it?\n>\n> There's tons of discussion in:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/297409\n>\n> but frankly it is not worth your time to read it. These tests are about\n> overflowing the tar limits, which can only happen with times and sizes\n> greater than 32-bits. The right thing to do is to skip the tests\n> entirely on systems where sizeof(unsigned long) is less than 8 (the\n> actual value is 64GB+1, so technically a 37-bit system would work, but I\n> think it is OK for the test-skipping to be less specific).\n\nOK, how about this on top of a replacement for js/t0006-for-v2.9.2\nthat I'll send out as a reply to this message?\n\n\n\n\n archive-tar.c | 5 +++++\n t/t5000-tar-tree.sh | 10 +++++-----\n 2 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 7ea4e90..4d2832c 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -25,8 +25,13 @@ static int write_tar_filter_archive(const struct archiver *ar,\n *\n * Likewise for the mtime (which happens to use a buffer of the same size).\n */\n+#if ULONG_MAX == 0x7FFFFFFF\n+#define USTAR_MAX_SIZE ULONG_MAX\n+#define USTAR_MAX_MTIME ULONG_MAX\n+#else\n #define USTAR_MAX_SIZE 077777777777UL\n #define USTAR_MAX_MTIME 077777777777UL\n+#endif\n \n /* writes out the whole block, but only if it is full */\n static void write_if_needed(void)\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 96d208d..9c97789 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -360,7 +360,7 @@ test_expect_success 'set up repository with huge blob' '\n \n # We expect git to die with SIGPIPE here (otherwise we\n # would generate the whole 64GB).\n-test_expect_success 'generate tar with huge size' '\n+test_expect_success 64BIT 'generate tar with huge size' '\n \t{\n \t\tgit archive HEAD\n \t\techo $? >exit-code\n@@ -369,13 +369,13 @@ test_expect_success 'generate tar with huge size' '\n \ttest_cmp expect exit-code\n '\n \n-test_expect_success TAR_HUGE 'system tar can read our huge size' '\n+test_expect_success TAR_HUGE,64BIT 'system tar can read our huge size' '\n \techo 68719476737 >expect &&\n \ttar_info huge.tar | cut -d\" \" -f1 >actual &&\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'set up repository with far-future commit' '\n+test_expect_success 64BIT 'set up repository with far-future commit' '\n \trm -f .git/index &&\n \techo content >file &&\n \tgit add file &&\n@@ -383,11 +383,11 @@ test_expect_success 'set up repository with far-future commit' '\n \t\tgit commit -m \"tempori parendum\"\n '\n \n-test_expect_success 'generate tar with future mtime' '\n+test_expect_success 64BIT 'generate tar with future mtime' '\n \tgit archive HEAD >future.tar\n '\n \n-test_expect_success TAR_HUGE 'system tar can read our future mtime' '\n+test_expect_success TAR_HUGE,64BIT 'system tar can read our future mtime' '\n \techo 4147 >expect &&\n \ttar_info future.tar | cut -d\" \" -f2 >actual &&\n \ttest_cmp expect actual\n"},{"id":"291468","messageId":"xmqqoa60oulk.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"xmqqshvcourb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T20:03:35Z","receivedAt":"2016-07-14T20:03:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> OK, how about this on top of a replacement for js/t0006-for-v2.9.2\n> that I'll send out as a reply to this message?\n\nwhich is this one, which is largely from your $gmane/299310.\n\n-- >8 --\nFrom: Jeff King \nDate: Mon, 11 Jul 2016 19:54:18 -0400\nSubject: [PATCH] t0006: skip \"far in the future\" test when unsigned long is\n not long enough\n\nGit's source code refers to timestamps as unsigned longs. On 32-bit\nplatforms, as well as on Windows, unsigned long is not large enough\nto capture dates that are \"absurdly far in the future\".\n\nWhile we can fix this issue properly by replacing unsigned long with\na larger type, we want to be a bit more conservative and just skip\nthose tests on the maint track.\n\nSigned-off-by: Jeff King \nHelped-by: Johannes Schindelin \nSigned-off-by: Junio C Hamano \n---\n help.c | 7 +++++++\n t/t0006-date.sh | 6 +++---\n t/test-lib.sh | 9 +++++++++\n 3 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex 19328ea..0cea240 100644\n--- a/help.c\n+++ b/help.c\n@@ -419,6 +419,13 @@ int cmd_version(int argc, const char **argv, const char *prefix)\n \t * with external projects that rely on the output of \"git version\".\n \t */\n \tprintf(\"git version %s\\n\", git_version_string);\n+\twhile (*++argv) {\n+\t\tif (!strcmp(*argv, \"--build-options\")) {\n+\t\t\tprintf(\"sizeof-unsigned-long: %d\",\n+\t\t\t (int)sizeof(unsigned long));\n+\t\t\t/* maybe also save and output GIT-BUILD_OPTIONS? */\n+\t\t}\n+\t}\n \treturn 0;\n }\n \ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 04ce535..a0b8497 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -31,7 +31,7 @@ check_show () {\n \tformat=$1\n \ttime=$2\n \texpect=$3\n-\ttest_expect_${4:-success} \"show date ($format:$time)\" '\n+\ttest_expect_success $4 \"show date ($format:$time)\" '\n \t\techo \"$time -> $expect\" >expect &&\n \t\ttest-date show:$format \"$time\" >actual &&\n \t\ttest_cmp expect actual\n@@ -50,8 +50,8 @@ check_show iso-local \"$TIME\" '2016-06-15 14:13:20 +0000'\n \n # arbitrary time absurdly far in the future\n FUTURE=\"5758122296 -0400\"\n-check_show iso \"$FUTURE\" \"2152-06-19 18:24:56 -0400\"\n-check_show iso-local \"$FUTURE\" \"2152-06-19 22:24:56 +0000\"\n+check_show iso \"$FUTURE\" \"2152-06-19 18:24:56 -0400\" 64BIT\n+check_show iso-local \"$FUTURE\" \"2152-06-19 22:24:56 +0000\" 64BIT\n \n check_parse() {\n \techo \"$1 -> $2\" >expect\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0055ebb..211f1a8 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1111,3 +1111,12 @@ run_with_limited_cmdline () {\n }\n \n test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'\n+\n+build_option () {\n+\tgit version --build-options |\n+\tsed -ne \"s/^$1: //p\"\n+}\n+\n+test_lazy_prereq 64BIT '\n+\ttest 8 -le \"$(build_option sizeof-unsigned-long)\"\n+'\n-- \n2.9.1-545-g8c0a069\n\n"},{"id":"291470","messageId":"xmqqfurcoubg.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"xmqqshvcourb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T20:09:39Z","receivedAt":"2016-07-14T20:09:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> OK, how about this on top of a replacement for js/t0006-for-v2.9.2\n> that I'll send out as a reply to this message?\n\nI won't repeat the patch body, but I had to write a log message, so \nhere is what I tentatively committed (not queued yet).\n\nSubject: archive-tar: huge offset and future timestamps would not work on 32-bit\n\nAs we are not yet moving everything to size_t but still using ulong\ninternally when talking about the size of object, platforms with\n32-bit long will not be able to produce tar archive with 4GB+ file,\nand cannot grok 077777777777UL as a constant. Disable the extended\nheader feature and do not test it on them.\n\nSigned-off-by: Junio C Hamano \n"},{"id":"291471","messageId":"20160714201018.GA18372@sigill.intra.peff.net","threadId":"42754","inReplyTo":"xmqqshvcourb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T20:10:18Z","receivedAt":"2016-07-14T20:10:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 01:00:08PM -0700, Junio C Hamano wrote:\n\n> > There's tons of discussion in:\n> >\n> > http://thread.gmane.org/gmane.comp.version-control.git/297409\n> >\n> > but frankly it is not worth your time to read it. These tests are about\n> > overflowing the tar limits, which can only happen with times and sizes\n> > greater than 32-bits. The right thing to do is to skip the tests\n> > entirely on systems where sizeof(unsigned long) is less than 8 (the\n> > actual value is 64GB+1, so technically a 37-bit system would work, but I\n> > think it is OK for the test-skipping to be less specific).\n> \n> OK, how about this on top of a replacement for js/t0006-for-v2.9.2\n> that I'll send out as a reply to this message?\n\nYeah, I think the patch here mostly makes sense. I tried to think what\ncould go wrong in this hunk:\n\n> diff --git a/archive-tar.c b/archive-tar.c\n> index 7ea4e90..4d2832c 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -25,8 +25,13 @@ static int write_tar_filter_archive(const struct archiver *ar,\n> *\n> * Likewise for the mtime (which happens to use a buffer of the same size).\n> */\n> +#if ULONG_MAX == 0x7FFFFFFF\n> +#define USTAR_MAX_SIZE ULONG_MAX\n> +#define USTAR_MAX_MTIME ULONG_MAX\n> +#else\n> #define USTAR_MAX_SIZE 077777777777UL\n> #define USTAR_MAX_MTIME 077777777777UL\n> +#endif\n> \n> /* writes out the whole block, but only if it is full */\n> static void write_if_needed(void)\n\nIf for some reason we are wrong that objects cannot be larger than\nULONG_MAX (e.g., later on we convert everything to size_t, and 64-bit\nLLP platforms handle large objects just fine), then we would prematurely\nswitch to extended headers on those platforms.\n\nI think that's OK. This would just need cleaned up as part of the\nconversion from \"unsigned long\" to \"size_t\" (the correct check would\nthen be against the max size_t).\n\nAlso, shouldn't it be checking against 0xFFFFFFFF?\n\nAn easier check would be \"sizeof()\", but I guess we can't use that in a\npreprocessor directive.\n\n> -test_expect_success TAR_HUGE 'system tar can read our huge size' '\n> +test_expect_success TAR_HUGE,64BIT 'system tar can read our huge size' '\n\nThe 64BIT prereq is really \"unsigned long is 64-bit\". I wonder if we\nshould call it UL64 or something like that to make it more clear.\n\nThat makes it unnecessarily tied-in with the implementation, but it does\nmake it more clear what we care about; the distinction matters for\nthings like LP64 vs LLP64.\n\n-Peff\n"},{"id":"291472","messageId":"20160714201423.GB18372@sigill.intra.peff.net","threadId":"42754","inReplyTo":"xmqqoa60oulk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T20:14:23Z","receivedAt":"2016-07-14T20:14:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 01:03:35PM -0700, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > OK, how about this on top of a replacement for js/t0006-for-v2.9.2\n> > that I'll send out as a reply to this message?\n> \n> which is this one, which is largely from your $gmane/299310.\n\nMostly OK, but two minor nits:\n\n> diff --git a/help.c b/help.c\n> index 19328ea..0cea240 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -419,6 +419,13 @@ int cmd_version(int argc, const char **argv, const char *prefix)\n> \t * with external projects that rely on the output of \"git version\".\n> \t */\n> \tprintf(\"git version %s\\n\", git_version_string);\n> +\twhile (*++argv) {\n> +\t\tif (!strcmp(*argv, \"--build-options\")) {\n> +\t\t\tprintf(\"sizeof-unsigned-long: %d\",\n> +\t\t\t (int)sizeof(unsigned long));\n> +\t\t\t/* maybe also save and output GIT-BUILD_OPTIONS? */\n> +\t\t}\n\nThat comment was mostly for explaining my mindset in the discussion. If\nwe're going to keep it, maybe mark it with a TODO or XXX or something?\n\n> +test_lazy_prereq 64BIT '\n> +\ttest 8 -le \"$(build_option sizeof-unsigned-long)\"\n> +'\n\nAs mentioned in my previous email, I wonder if this should be UL64 or\nsomething.\n\nAs noted earlier in the thread, t0006 actually cares most directly about\nLONG_MAX, but I think it's probably OK to assume it is directly related\nto \"unsigned long\". Also, even if we got past that initial \"strtol\", I\nsuspect \"unsigned long\" _would_ come into play anyway, as that is our\ninternal representation.\n\n-Peff\n"},{"id":"291474","messageId":"xmqqbn20otqu.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"20160714201018.GA18372@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T20:22:01Z","receivedAt":"2016-07-14T20:22:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> +#if ULONG_MAX == 0x7FFFFFFF\n>> +#define USTAR_MAX_SIZE ULONG_MAX\n>> +#define USTAR_MAX_MTIME ULONG_MAX\n>> +#else\n>> #define USTAR_MAX_SIZE 077777777777UL\n>> #define USTAR_MAX_MTIME 077777777777UL\n>> +#endif\n>> \n>> /* writes out the whole block, but only if it is full */\n>> static void write_if_needed(void)\n>\n> If for some reason we are wrong that objects cannot be larger than\n> ULONG_MAX (e.g., later on we convert everything to size_t, and 64-bit\n> LLP platforms handle large objects just fine), then we would prematurely\n> switch to extended headers on those platforms.\n>\n> I think that's OK. This would just need cleaned up as part of the\n> conversion from \"unsigned long\" to \"size_t\" (the correct check would\n> then be against the max size_t).\n\n> Also, shouldn't it be checking against 0xFFFFFFFF?\n\nCorrect. Somehow I thought I was checking with LONG_MAX. Will correct.\n\n> An easier check would be \"sizeof()\", but I guess we can't use that in a\n> preprocessor directive.\n\nYes, I tried it and failed ;-)\n\n>> -test_expect_success TAR_HUGE 'system tar can read our huge size' '\n>> +test_expect_success TAR_HUGE,64BIT 'system tar can read our huge size' '\n>\n> The 64BIT prereq is really \"unsigned long is 64-bit\". I wonder if we\n> should call it UL64 or something like that to make it more clear.\n>\n> That makes it unnecessarily tied-in with the implementation, but it does\n> make it more clear what we care about; the distinction matters for\n> things like LP64 vs LLP64.\n\nI do not think any platform is weird enough to have different sizes\nfor long and ulong, so I am not sure you need UL64.\n\nBut pointer size can legitimately be different, so it has a value to\ndifferentiate between LP64 and LLP64, if we start doing things like\n\"does this platform have large virtual address space?\"\n\nLONG_IS_64BIT perhaps to be more readable?\n"},{"id":"291475","messageId":"20160714202727.GA19101@sigill.intra.peff.net","threadId":"42754","inReplyTo":"xmqqbn20otqu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T20:27:27Z","receivedAt":"2016-07-14T20:27:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 01:22:01PM -0700, Junio C Hamano wrote:\n\n> > Also, shouldn't it be checking against 0xFFFFFFFF?\n> \n> Correct. Somehow I thought I was checking with LONG_MAX. Will correct.\n> \n> > An easier check would be \"sizeof()\", but I guess we can't use that in a\n> > preprocessor directive.\n> \n> Yes, I tried it and failed ;-)\n\nI also found it funny that you used \"==\" instead \"<=\", but I cannot\nreally think of a case where it would matter. If it is \"<= 0xFFFFFFFF\",\nthat basically covers 16-bit platforms. I really hope nobody is trying\nto run git on such a platform. Doing \"< 0xFFFFFFFFFFFFFFFF\" to check\nfor \"less than 64-bit\" would make more sense, but would probably choke\non a 32-bit preprocessor.\n\nSo that everybody is either 32- or 64-bit these days, I think it doesn't\nmatter in practice.\n\n> >> -test_expect_success TAR_HUGE 'system tar can read our huge size' '\n> >> +test_expect_success TAR_HUGE,64BIT 'system tar can read our huge size' '\n> >\n> > The 64BIT prereq is really \"unsigned long is 64-bit\". I wonder if we\n> > should call it UL64 or something like that to make it more clear.\n> >\n> > That makes it unnecessarily tied-in with the implementation, but it does\n> > make it more clear what we care about; the distinction matters for\n> > things like LP64 vs LLP64.\n> \n> I do not think any platform is weird enough to have different sizes\n> for long and ulong, so I am not sure you need UL64.\n> \n> But pointer size can legitimately be different, so it has a value to\n> differentiate between LP64 and LLP64, if we start doing things like\n> \"does this platform have large virtual address space?\"\n> \n> LONG_IS_64BIT perhaps to be more readable?\n\nYeah, that is what I was trying to get at, but you stated it much more\nclearly. LONG_IS_64BIT is good. I wonder if the \"git version\n--build-options\" should be \"sizeof-long\", too. It's shorter, and\nindicates our assumption that we are talking about all longs, not just\nunsigned ones.\n\n-Peff\n"},{"id":"291476","messageId":"xmqq7fcoot6t.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"20160714202727.GA19101@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T20:34:02Z","receivedAt":"2016-07-14T20:34:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yeah, that is what I was trying to get at, but you stated it much more\n> clearly. LONG_IS_64BIT is good. I wonder if the \"git version\n> --build-options\" should be \"sizeof-long\", too. It's shorter, and\n> indicates our assumption that we are talking about all longs, not just\n> unsigned ones.\n\nOK, I'll do a final reroll and then wait for Europeans ;-)\n"},{"id":"291477","messageId":"20160714204357.2628-3-gitster@pobox.com","threadId":"42754","inReplyTo":"20160714204357.2628-1-gitster@pobox.com","subject":"[PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T20:43:57Z","receivedAt":"2016-07-14T20:49:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As we are not yet moving everything to size_t but still using ulong\ninternally when talking about the size of object, platforms with\n32-bit long will not be able to produce tar archive with 4GB+ file,\nand cannot grok 077777777777UL as a constant. Disable the extended\nheader feature and do not test it on them.\n\nSigned-off-by: Junio C Hamano \n---\n archive-tar.c | 5 +++++\n t/t5000-tar-tree.sh | 10 +++++-----\n 2 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 7ea4e90..5568240 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -25,8 +25,13 @@ static int write_tar_filter_archive(const struct archiver *ar,\n *\n * Likewise for the mtime (which happens to use a buffer of the same size).\n */\n+#if ULONG_MAX == 0xFFFFFFFF\n+#define USTAR_MAX_SIZE ULONG_MAX\n+#define USTAR_MAX_MTIME ULONG_MAX\n+#else\n #define USTAR_MAX_SIZE 077777777777UL\n #define USTAR_MAX_MTIME 077777777777UL\n+#endif\n \n /* writes out the whole block, but only if it is full */\n static void write_if_needed(void)\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 96d208d..699355b 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -360,7 +360,7 @@ test_expect_success 'set up repository with huge blob' '\n \n # We expect git to die with SIGPIPE here (otherwise we\n # would generate the whole 64GB).\n-test_expect_success 'generate tar with huge size' '\n+test_expect_success LONG_IS_64BIT 'generate tar with huge size' '\n \t{\n \t\tgit archive HEAD\n \t\techo $? >exit-code\n@@ -369,13 +369,13 @@ test_expect_success 'generate tar with huge size' '\n \ttest_cmp expect exit-code\n '\n \n-test_expect_success TAR_HUGE 'system tar can read our huge size' '\n+test_expect_success TAR_HUGE,LONG_IS_64BIT 'system tar can read our huge size' '\n \techo 68719476737 >expect &&\n \ttar_info huge.tar | cut -d\" \" -f1 >actual &&\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'set up repository with far-future commit' '\n+test_expect_success LONG_IS_64BIT 'set up repository with far-future commit' '\n \trm -f .git/index &&\n \techo content >file &&\n \tgit add file &&\n@@ -383,11 +383,11 @@ test_expect_success 'set up repository with far-future commit' '\n \t\tgit commit -m \"tempori parendum\"\n '\n \n-test_expect_success 'generate tar with future mtime' '\n+test_expect_success LONG_IS_64BIT 'generate tar with future mtime' '\n \tgit archive HEAD >future.tar\n '\n \n-test_expect_success TAR_HUGE 'system tar can read our future mtime' '\n+test_expect_success TAR_HUGE,LONG_IS_64BIT 'system tar can read our future mtime' '\n \techo 4147 >expect &&\n \ttar_info future.tar | cut -d\" \" -f2 >actual &&\n \ttest_cmp expect actual\n-- \n2.9.1-545-g8c0a069\n\n"},{"id":"291478","messageId":"20160714204357.2628-1-gitster@pobox.com","threadId":"42754","inReplyTo":"xmqq7fcoot6t.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 0/2] ulong may only be 32-bit wide","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T20:43:55Z","receivedAt":"2016-07-14T20:50:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"So here is the final reroll from me for now that targets 'maint'\n(eventually).\n\nJeff King (1):\n t0006: skip \"far in the future\" test when unsigned long is not long\n enough\n\nJunio C Hamano (1):\n archive-tar: huge offset and future timestamps would not work on\n 32-bit\n\n archive-tar.c | 5 +++++\n help.c | 6 ++++++\n t/t0006-date.sh | 6 +++---\n t/t5000-tar-tree.sh | 10 +++++-----\n t/test-lib.sh | 9 +++++++++\n 5 files changed, 28 insertions(+), 8 deletions(-)\n\n-- \n2.9.1-545-g8c0a069\n\n"},{"id":"291479","messageId":"20160714204357.2628-2-gitster@pobox.com","threadId":"42754","inReplyTo":"20160714204357.2628-1-gitster@pobox.com","subject":"[PATCH v2 1/2] t0006: skip \"far in the future\" test when unsigned long is not long enough","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T20:43:56Z","receivedAt":"2016-07-14T20:50:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Jeff King \n\nGit's source code refers to timestamps as unsigned longs. On 32-bit\nplatforms, as well as on Windows, unsigned long is not large enough\nto capture dates that are \"absurdly far in the future\".\n\nWhile we can fix this issue properly by replacing unsigned long with\na larger type, we want to be a bit more conservative and just skip\nthose tests on the maint track.\n\nSigned-off-by: Jeff King \nHelped-by: Johannes Schindelin \nSigned-off-by: Junio C Hamano \n---\n help.c | 6 ++++++\n t/t0006-date.sh | 6 +++---\n t/test-lib.sh | 9 +++++++++\n 3 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex 19328ea..2ff3b5a 100644\n--- a/help.c\n+++ b/help.c\n@@ -419,6 +419,12 @@ int cmd_version(int argc, const char **argv, const char *prefix)\n \t * with external projects that rely on the output of \"git version\".\n \t */\n \tprintf(\"git version %s\\n\", git_version_string);\n+\twhile (*++argv) {\n+\t\tif (!strcmp(*argv, \"--build-options\")) {\n+\t\t\tprintf(\"sizeof-long: %d\\n\", (int)sizeof(long));\n+\t\t\t/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */\n+\t\t}\n+\t}\n \treturn 0;\n }\n \ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 04ce535..4c8cf58 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -31,7 +31,7 @@ check_show () {\n \tformat=$1\n \ttime=$2\n \texpect=$3\n-\ttest_expect_${4:-success} \"show date ($format:$time)\" '\n+\ttest_expect_success $4 \"show date ($format:$time)\" '\n \t\techo \"$time -> $expect\" >expect &&\n \t\ttest-date show:$format \"$time\" >actual &&\n \t\ttest_cmp expect actual\n@@ -50,8 +50,8 @@ check_show iso-local \"$TIME\" '2016-06-15 14:13:20 +0000'\n \n # arbitrary time absurdly far in the future\n FUTURE=\"5758122296 -0400\"\n-check_show iso \"$FUTURE\" \"2152-06-19 18:24:56 -0400\"\n-check_show iso-local \"$FUTURE\" \"2152-06-19 22:24:56 +0000\"\n+check_show iso \"$FUTURE\" \"2152-06-19 18:24:56 -0400\" LONG_IS_64BIT\n+check_show iso-local \"$FUTURE\" \"2152-06-19 22:24:56 +0000\" LONG_IS_64BIT\n \n check_parse() {\n \techo \"$1 -> $2\" >expect\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0055ebb..11201e9 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1111,3 +1111,12 @@ run_with_limited_cmdline () {\n }\n \n test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'\n+\n+build_option () {\n+\tgit version --build-options |\n+\tsed -ne \"s/^$1: //p\"\n+}\n+\n+test_lazy_prereq LONG_IS_64BIT '\n+\ttest 8 -le \"$(build_option sizeof-long)\"\n+'\n-- \n2.9.1-545-g8c0a069\n\n"},{"id":"291480","messageId":"3d71cf3a-44c7-0620-0375-fb7ecf2fac13@kdbg.org","threadId":"42754","inReplyTo":"xmqqa8hkrvu5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-07-14T20:52:55Z","receivedAt":"2016-07-14T20:53:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.07.2016 um 19:08 schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>\n>> Am 14.07.2016 um 17:47 schrieb Johannes Schindelin:\n>>> On Thu, 30 Jun 2016, Jeff King wrote:\n>>>> The ustar format only has room for 11 (or 12, depending on\n>>>> some implementations) octal digits for the size and mtime of\n>>>> each file. For values larger than this, we have to add pax\n>>>> extended headers to specify the real data, and git does not\n>>>> yet know how to do so.\n>>>>\n>>>> [...]\n>>>> t/t5000/19f9c8273ec45a8938e6999cb59b3ff66739902a | Bin 0 -> 2048 bytes\n>>>\n>>> It appears that this blob cannot be read when sizeof(unsigned long) == 4.\n>>> This happens to break the t5000 test on Windows, where that comparison\n>>> holds true.\n>>\n>> The problem occurs in parse_sha1_header_extended(), where the\n>> calculation of the size in the object header overflows our 32-bit\n>> unsigned long.\n>\n> OK, then we'd want something that measures how big \"unsigned long\"\n> is, and use it as a lazy prerequisite 64BIT_LONG, tweaking the other\n> patch to t0006 the other Johannes sent yesterday.\n>\n> Dscho? I'll revert the merge of 'js/t0006-for-v2.9.2' out of\n> 'next' so that we can replace it with your newer version, but it\n> needs to be massaged to lose the strong linkage with \"time\", as\n> it is no longer \"is our time big enough\", I would think.\n\nMy first thought was that this is not warranted because t0006 is about \ncommit time stamps, but the huge-tar breakage is file sizes, and the \ncases should be treated differently.\n\nBut on second thought, under the hood, both boil down to the size of \nunsigned long in our implementation. It may make sense to tie both cases \nto the same prerequisite.\n\nOn third thought, however, I think the two requirements could diverge in \nthe future. The file size case should depend on the size of size_t. The \ntimestamp case may become dependent on the size of time_t if we decide \nto move timestamp handling away from unsigned long: in modern(!) \nMicrosoft SDKs, time_t is 64 bits, but unsigned long is 32 bits, in both \nthe 32-bit and 64-bit environments!\n\n-- Hannes\n\n"},{"id":"291482","messageId":"20160714213222.GA20645@sigill.intra.peff.net","threadId":"42754","inReplyTo":"3d71cf3a-44c7-0620-0375-fb7ecf2fac13@kdbg.org","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T21:32:22Z","receivedAt":"2016-07-14T21:32:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 10:52:55PM +0200, Johannes Sixt wrote:\n\n> > Dscho? I'll revert the merge of 'js/t0006-for-v2.9.2' out of\n> > 'next' so that we can replace it with your newer version, but it\n> > needs to be massaged to lose the strong linkage with \"time\", as\n> > it is no longer \"is our time big enough\", I would think.\n> \n> My first thought was that this is not warranted because t0006 is about\n> commit time stamps, but the huge-tar breakage is file sizes, and the cases\n> should be treated differently.\n> \n> But on second thought, under the hood, both boil down to the size of\n> unsigned long in our implementation. It may make sense to tie both cases to\n> the same prerequisite.\n> \n> On third thought, however, I think the two requirements could diverge in the\n> future. The file size case should depend on the size of size_t. The\n> timestamp case may become dependent on the size of time_t if we decide to\n> move timestamp handling away from unsigned long: in modern(!) Microsoft\n> SDKs, time_t is 64 bits, but unsigned long is 32 bits, in both the 32-bit\n> and 64-bit environments!\n\nThe tar tests actually cover both: big files and big timestamps.\n\nI think as long as the prereq is labeled LONG_IS_64BIT, we can then\nadjust the appropriate tests if and when they become runnable on more\nsystems.\n\nIf we move to time_t everywhere, I think we'll need an extra\nTIME_T_IS_64BIT, but we can cross that bridge when we come to it.\n\nLikewise I think we'll need SIZE_T_IS_64BIT eventually (for real 32-bit\nsystems; LLP64 systems like Windows will then be able to run the test).\n\n-Peff\n"},{"id":"291491","messageId":"20160714222047.GA21868@sigill.intra.peff.net","threadId":"42754","inReplyTo":"20160714204357.2628-3-gitster@pobox.com","subject":"Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T22:20:47Z","receivedAt":"2016-07-14T22:20:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 01:43:57PM -0700, Junio C Hamano wrote:\n\n> As we are not yet moving everything to size_t but still using ulong\n> internally when talking about the size of object, platforms with\n> 32-bit long will not be able to produce tar archive with 4GB+ file,\n> and cannot grok 077777777777UL as a constant. Disable the extended\n> header feature and do not test it on them.\n\nI tried testing this in a VM with 32-bit Debian. It fixes the build\nproblems, but t5000 still fails.\n\nI think you need to add the prereq to one more test:\n\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 699355b..80b2387 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -347,7 +347,7 @@ test_lazy_prereq TAR_HUGE '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'set up repository with huge blob' '\n+test_expect_success LONG_IS_64BIT 'set up repository with huge blob' '\n \tobj_d=19 &&\n \tobj_f=f9c8273ec45a8938e6999cb59b3ff66739902a &&\n \tobj=${obj_d}${obj_f} &&\n\nWe shouldn't be accessing the blob in update-index, but I think \"git\ncommit\" does so for the diff (and then after seeing the size says\n\"whoops, that's binary\", but even the size check fails on 32-bit\nsystems).\n\nSo another solution would be to use \"commit -q\" at the end of that test.\nI don't think there's much point, though; it's just setting up a state\nfor other tests that need LONG_IS_64BIT.\n\nAs an aside, it is inadvertently testing that our diff code does not\nbother to read the whole blob in such a case. Which maybe argues for\nusing \"commit -q\", just because that is not a thing we are intending to\ntest here.\n\n-Peff\n"},{"id":"291492","messageId":"xmqqtwfronyk.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"3d71cf3a-44c7-0620-0375-fb7ecf2fac13@kdbg.org","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T22:26:59Z","receivedAt":"2016-07-14T22:27:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> My first thought was that this is not warranted because t0006 is about\n> commit time stamps, but the huge-tar breakage is file sizes, and the\n> cases should be treated differently.\n>\n> But on second thought, under the hood, both boil down to the size of\n> unsigned long in our implementation. It may make sense to tie both\n> cases to the same prerequisite.\n>\n> On third thought, however, I think the two requirements could diverge\n> in the future. The file size case should depend on the size of\n> size_t. The timestamp case may become dependent on the size of time_t\n> if we decide to move timestamp handling away from unsigned long: in\n> modern(!) Microsoft SDKs, time_t is 64 bits, but unsigned long is 32\n> bits, in both the 32-bit and 64-bit environments!\n\nI had the same three toughts, but this being a 'maint' material\nstopped me going too deep into them. Right now, \"long being 32-bit\"\nis the source of all of these issues, and we would solve them on the\ndevelopment track (not necessarily during this cycle) by deciding on\nmore appropriate types. Timestamps may become time_t, and object\nsizes may become off_t, such changes will come separately, and each\nof them would need to lift \"unless long is 64-bit, skip this test\"\nlimitation and swap it with something else.\n\n"},{"id":"291493","messageId":"xmqqpoqfonrx.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"20160714213222.GA20645@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T22:30:58Z","receivedAt":"2016-07-14T22:31:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If we move to time_t everywhere, I think we'll need an extra\n> TIME_T_IS_64BIT, but we can cross that bridge when we come to it.\n>\n> Likewise I think we'll need SIZE_T_IS_64BIT eventually (for real 32-bit\n> systems; LLP64 systems like Windows will then be able to run the test).\n\nI guess I wrote essentially the same thing before refreshing my\nInbox.\n\nI am a bit fuzzy between off_t and size_t; the former is for the\nsize of things you see on the filesystem, while the latter is for\nyou to give malloc(3). I would have thought that off_t is the type\nwe would want at the end of the raw object header, denoting the size\nof a blob object when deflated, which could be larger than the size\nof a region of memory we can get from malloc(3), in which case we\nwould use the streaming interface.\n\n\n"},{"id":"291494","messageId":"xmqqlh13onik.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"20160714222047.GA21868@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-14T22:36:35Z","receivedAt":"2016-07-14T22:36:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Jul 14, 2016 at 01:43:57PM -0700, Junio C Hamano wrote:\n>\n>> As we are not yet moving everything to size_t but still using ulong\n>> internally when talking about the size of object, platforms with\n>> 32-bit long will not be able to produce tar archive with 4GB+ file,\n>> and cannot grok 077777777777UL as a constant. Disable the extended\n>> header feature and do not test it on them.\n>\n> I tried testing this in a VM with 32-bit Debian. It fixes the build\n> problems, but t5000 still fails.\n\nThanks for testing. I need to find some time hunting for (or\nbuilding) an environment to do that myself.\n\n> I think you need to add the prereq to one more test:\n>\n> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\n> index 699355b..80b2387 100755\n> --- a/t/t5000-tar-tree.sh\n> +++ b/t/t5000-tar-tree.sh\n> @@ -347,7 +347,7 @@ test_lazy_prereq TAR_HUGE '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_success 'set up repository with huge blob' '\n> +test_expect_success LONG_IS_64BIT 'set up repository with huge blob' '\n> \tobj_d=19 &&\n> \tobj_f=f9c8273ec45a8938e6999cb59b3ff66739902a &&\n> \tobj=${obj_d}${obj_f} &&\n>\n> We shouldn't be accessing the blob in update-index, but I think \"git\n> commit\" does so for the diff (and then after seeing the size says\n> \"whoops, that's binary\", but even the size check fails on 32-bit\n> systems).\n>\n> So another solution would be to use \"commit -q\" at the end of that test.\n> I don't think there's much point, though; it's just setting up a state\n> for other tests that need LONG_IS_64BIT.\n>\n> As an aside, it is inadvertently testing that our diff code does not\n> bother to read the whole blob in such a case. Which maybe argues for\n> using \"commit -q\", just because that is not a thing we are intending to\n> test here.\n\nThanks. Let's add a prereq there.\n"},{"id":"291496","messageId":"20160714223843.GA22196@sigill.intra.peff.net","threadId":"42754","inReplyTo":"xmqqpoqfonrx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-14T22:38:43Z","receivedAt":"2016-07-14T22:38:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 14, 2016 at 03:30:58PM -0700, Junio C Hamano wrote:\n\n> > If we move to time_t everywhere, I think we'll need an extra\n> > TIME_T_IS_64BIT, but we can cross that bridge when we come to it.\n> >\n> > Likewise I think we'll need SIZE_T_IS_64BIT eventually (for real 32-bit\n> > systems; LLP64 systems like Windows will then be able to run the test).\n> \n> I guess I wrote essentially the same thing before refreshing my\n> Inbox.\n> \n> I am a bit fuzzy between off_t and size_t; the former is for the\n> size of things you see on the filesystem, while the latter is for\n> you to give malloc(3). I would have thought that off_t is the type\n> we would want at the end of the raw object header, denoting the size\n> of a blob object when deflated, which could be larger than the size\n> of a region of memory we can get from malloc(3), in which case we\n> would use the streaming interface.\n\nYeah, your understanding is right (s/deflated/inflated/). I agree that\noff_t is probably a better size for blobs. Traditionally git assumed any\nobject could fit in memory. The streaming interface helps that somewhat,\nbut I think there are cases where we still must load a blob (e.g., if it\nis stored as a delta). In theory that never happens because of\ncore.bigfilethreshold, but you may get a packfile from somebody with a\nhigher threshold than you.\n\nI wouldn't be surprised if there are other cases that aren't smart\nenough to use the streaming interface yet, but the solution there is to\nmake them smarter. :)\n\nSo off_t is probably better. We do need to be careful, though, when\nallocating objects. E.g., this:\n\n off_t size;\n struct git_istream *stream;\n void *buf;\n\n stream = open_istream(sha1, &type, &size, NULL);\n buf = xmalloc(size);\n while (1) {\n\t/* read stream into buf */\n }\n\nis a security hole when size_t is less than off_t (it gets truncated in\nthe call to xmalloc, which allocates too few bytes). This is a toy\nexample, obviously, but it's something to watch out for.\n\n-Peff\n"},{"id":"291503","messageId":"7ad7705a-40a1-cc2c-e7ea-47d83e1627a3@web.de","threadId":"42754","inReplyTo":"ac016bb4-9f69-7e03-bc99-7a5bbc1a1a33@kdbg.org","subject":"Re: [PATCH v4 3/5] archive-tar: write extended headers for file sizes >= 8GB","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-07-15T02:59:11Z","receivedAt":"2016-07-15T03:00:32Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\nOn 07/14/2016 06:48 PM, Johannes Sixt wrote:\n> Am 30.06.2016 um 11:09 schrieb Jeff King:\n>> +/*\n>> + * This is the max value that a ustar size header can specify, as it\n>> is fixed\n>> + * at 11 octal digits. POSIX specifies that we switch to extended\n>> headers at\n>> + * this size.\n>> + */\n>> +#define USTAR_MAX_SIZE 077777777777UL\n>\n> This is too large by one bit for our 32-bit unsigned long on Windows:\n>\n> archive-tar.c: In function 'write_tar_entry':\n> archive-tar.c:295: warning: integer constant is too large for 'unsigned\n> long' type\n> archive-tar.c: In function 'write_global_extended_header':\n> archive-tar.c:332: warning: integer constant is too large for 'unsigned\n> long' type\n> archive-tar.c:335: warning: integer constant is too large for 'unsigned\n> long' type\n> archive-tar.c:335: warning: overflow in implicit constant conversion\n>\n> -- Hannes\nSimilar problem on 32 Bit Linux:\n In function ‘write_global_extended_header’:\narchive-tar.c:29:25: warning: overflow in implicit constant conversion \n[-Woverflow]\n #define USTAR_MAX_MTIME 077777777777UL\n ^\narchive-tar.c:335:16: note: in expansion of macro ‘USTAR_MAX_MTIME’\n args->time = USTAR_MAX_MTIME;\n\n----------------------------------\nI want to volunteer to do more tests on 32 bit Linux ;-)\nDoes this fix it and look as a good thing to change ?\n\n\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -332,7 +332,7 @@ static void write_global_extended_header(struct \narchiver_args *args)\n if (args->time > USTAR_MAX_MTIME) {\n strbuf_append_ext_header_uint(&ext_header, \"mtime\",\n args->time);\n- args->time = USTAR_MAX_MTIME;\n+ args->time = (time_t)USTAR_MAX_MTIME;\n }\n"},{"id":"291537","messageId":"5b99a4bb-9b8e-e8c6-e214-e041209cb6e6@web.de","threadId":"42754","inReplyTo":"20160714223843.GA22196@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-07-15T13:37:32Z","receivedAt":"2016-07-15T13:38:27Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\nOn 07/15/2016 12:38 AM, Jeff King wrote:\n> On Thu, Jul 14, 2016 at 03:30:58PM -0700, Junio C Hamano wrote:\n>\n>>> If we move to time_t everywhere, I think we'll need an extra\n>>> TIME_T_IS_64BIT, but we can cross that bridge when we come to it.\n>>>\n>>> Likewise I think we'll need SIZE_T_IS_64BIT eventually (for real 32-bit\n>>> systems; LLP64 systems like Windows will then be able to run the test).\n>>\n>> I guess I wrote essentially the same thing before refreshing my\n>> Inbox.\n>>\n>> I am a bit fuzzy between off_t and size_t; the former is for the\n>> size of things you see on the filesystem, while the latter is for\n>> you to give malloc(3). I would have thought that off_t is the type\n>> we would want at the end of the raw object header, denoting the size\n>> of a blob object when deflated, which could be larger than the size\n>> of a region of memory we can get from malloc(3), in which case we\n>> would use the streaming interface.\n>\n> Yeah, your understanding is right (s/deflated/inflated/). I agree that\n> off_t is probably a better size for blobs. Traditionally git assumed any\n> object could fit in memory. The streaming interface helps that somewhat,\n> but I think there are cases where we still must load a blob (e.g., if it\n> is stored as a delta). In theory that never happens because of\n> core.bigfilethreshold, but you may get a packfile from somebody with a\n> higher threshold than you.\n>\n> I wouldn't be surprised if there are other cases that aren't smart\n> enough to use the streaming interface yet, but the solution there is to\n> make them smarter. :)\n>\n> So off_t is probably better. We do need to be careful, though, when\n> allocating objects. E.g., this:\n>\n> off_t size;\n> struct git_istream *stream;\n> void *buf;\n>\n> stream = open_istream(sha1, &type, &size, NULL);\n> buf = xmalloc(size);\n> while (1) {\n> \t/* read stream into buf */\n> }\n>\n> is a security hole when size_t is less than off_t (it gets truncated in\n> the call to xmalloc, which allocates too few bytes). This is a toy\n> example, obviously, but it's something to watch out for.\n>\n> -Peff\nThat code is \"illegal\", it should be\n buf = xmalloc(xsize_t(size));\n\nAnd the transition from off_t into size_t\nshould always got via xsize_t():\n\nstatic inline size_t xsize_t(off_t len)\n{\n\tif (len > (size_t) len)\n\t\tdie(\"Cannot handle files this big\");\n\treturn (size_t)len;\n}\n\nThere are some more things to be done, on the long run:\n- convert \"unsigned long\" into either off_t of size_t in e.g. convert.c\n- Use the streaming interface to analyze if blobs are binary\n (That is already on my list, the old \"stream and early out\"\n from the olc 10/10, gmane/$293010 or so can be reused)\n"},{"id":"291538","messageId":"20160715134608.GA22802@sigill.intra.peff.net","threadId":"42754","inReplyTo":"5b99a4bb-9b8e-e8c6-e214-e041209cb6e6@web.de","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-15T13:46:09Z","receivedAt":"2016-07-15T13:46:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 15, 2016 at 03:37:32PM +0200, Torsten Bögershausen wrote:\n\n> > So off_t is probably better. We do need to be careful, though, when\n> > allocating objects. E.g., this:\n> > \n> > off_t size;\n> > struct git_istream *stream;\n> > void *buf;\n> > \n> > stream = open_istream(sha1, &type, &size, NULL);\n> > buf = xmalloc(size);\n> > while (1) {\n> > \t/* read stream into buf */\n> > }\n> > \n> > is a security hole when size_t is less than off_t (it gets truncated in\n> > the call to xmalloc, which allocates too few bytes). This is a toy\n> > example, obviously, but it's something to watch out for.\n> > \n> That code is \"illegal\", it should be\n> buf = xmalloc(xsize_t(size));\n\nSure, I agree. The point is that it is easy to forget the extra\nwrapper/check, and we should be aware of it. I don't think the compiler\nwill warn you (probably some static analyzers would, though).\n\n> - Use the streaming interface to analyze if blobs are binary\n> (That is already on my list, the old \"stream and early out\"\n> from the olc 10/10, gmane/$293010 or so can be reused)\n\nYou might be interested in\n\n https://github.com/peff/git/commit/2fb07bc91f3ac6162c3dd5667d8167fc0bec6d99\n\nI don't remember if it produced good results or not (ISTR that the cost\nof setting up the streaming sometimes overwhelmed any benefit).\n\n-Peff\n"},{"id":"291540","messageId":"alpine.DEB.2.20.1607151707180.6426@virtualbox","threadId":"42754","inReplyTo":"xmqq7fcoot6t.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-15T15:10:13Z","receivedAt":"2016-07-15T15:10:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 14 Jul 2016, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Yeah, that is what I was trying to get at, but you stated it much more\n> > clearly. LONG_IS_64BIT is good. I wonder if the \"git version\n> > --build-options\" should be \"sizeof-long\", too. It's shorter, and\n> > indicates our assumption that we are talking about all longs, not just\n> > unsigned ones.\n> \n> OK, I'll do a final reroll and then wait for Europeans ;-)\n\nDon't let me stand in the way of progress.\n\nI would have preferred two separate prereqs, of course, for the same\nreason we use time_t and off_t: I find it better to describe what the\nprereq is about, and I do not mean implementation details here.\n\nBut that does not matter all that much. We will have to address the issues\nanyway, and at that point that wart will be removed and we will all go on\nwith our lives.\n\nSo what are your plans with 2.9.2? I ask because I do not want to engineer\na 2.9.1 release just to see that 2.9.2 is out and having to spend the same\namount of time for another release ;-)\n\nCiao,\nDscho\n"},{"id":"291548","messageId":"xmqq60s6ongx.fsf@gitster.mtv.corp.google.com","threadId":"42754","inReplyTo":"alpine.DEB.2.20.1607151707180.6426@virtualbox","subject":"Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-15T16:49:50Z","receivedAt":"2016-07-15T16:49:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> So what are your plans with 2.9.2? I ask because I do not want to engineer\n> a 2.9.1 release just to see that 2.9.2 is out and having to spend the same\n> amount of time for another release ;-)\n\nEssentially unchanged from what I said in $gmane/299446 a few days\nago. Merge only the t0006 workaround with lazy-prereq changes to\n'maint', so that we can skip tests that are unrunnable on platforms\nwithout 64-bit long to avoid unnecessary test failures, then tag\nthat to v2.9.2 soon enough so that you do not have to do two\nreleases in a row (i.e. skipping v2.9.1 saying \"Git for Windows\nskipped that one because it was not quite right; this release fixes\nthe issue\" in your v2.9.2 announcement).\n\nI couldn't do that while that t0006 fix was in flux. Now I\nhopefully can.\n\n\n\n\n\n\n\n"},{"id":"291589","messageId":"CACsJy8BMn=1SqU5zpALm1KAPwjFjnbm43aaf-FKH6vdAzHjS5g@mail.gmail.com","threadId":"42754","inReplyTo":"xmqqlh13onik.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-16T06:28:05Z","receivedAt":"2016-07-16T06:28:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jul 15, 2016 at 12:36 AM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> On Thu, Jul 14, 2016 at 01:43:57PM -0700, Junio C Hamano wrote:\n>>\n>>> As we are not yet moving everything to size_t but still using ulong\n>>> internally when talking about the size of object, platforms with\n>>> 32-bit long will not be able to produce tar archive with 4GB+ file,\n>>> and cannot grok 077777777777UL as a constant. Disable the extended\n>>> header feature and do not test it on them.\n>>\n>> I tried testing this in a VM with 32-bit Debian. It fixes the build\n>> problems, but t5000 still fails.\n>\n> Thanks for testing. I need to find some time hunting for (or\n> building) an environment to do that myself.\n\nIf you are on a distro with multilib, building git with \"CFLAGS=-m32\nLDFLAGS=-m32\" should do it. That's how i tested the 32-bit offset\ntruncation thing. I may pursue the docker thing soon. At least then\nyou only need to install docker and just build/test (zero docker\nconfiguration, assuming all the relevant kernel options are enabled).\n-- \nDuy\n"}]}