{"thread":{"id":"4840","subject":"Revisiting large binary files issue.","startedAt":"2006-07-10T23:01:32Z","lastAt":"2006-07-12T16:29:09Z","messageCount":37,"participants":["Carl Baldwin","Junio C Hamano","Linus Torvalds","sf","Peter Baumann","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"23593","messageId":"20060710230132.GA11132@hpsvcnb.fc.hp.com","threadId":"4840","inReplyTo":null,"subject":"Revisiting large binary files issue.","fromName":"Carl Baldwin","fromEmail":"cnb@fc.hp.com","sentAt":"2006-07-10T23:01:32Z","receivedAt":"2006-07-10T23:01:32Z","isPatch":false,"sender":{"key":"cnb@fc.hp.com","avatar":null},"body":"Hi,\n\nSome of this stuff has been discussed before but I thought I'd bring it\nup again.\n\nI am using git in a way for which, admittedly, it was not intended. I\nhave repositories in which I'm storing some source code mixed with some\nlarge binary files (from 20MB up to 1GB in the worst case). The large\nfiles easily dominate the space in the repository and the time that it\ntakes to perform network operations between repositories.\n\nJust to refresh your memory a sample of these files led to work that\nNicolas did to improve the performance of packing large binary files.\nThank you Nicolas, I think your work improved the speed of git in the\nface of largish files.\n\nAttempting to delta compress these blobs is a frivolous effort. The\nnature of the blobs is such that if given two blobs: 'A', and the next\nrevision, 'B' it is just as good --- from a pack-size standpoint --- to\ncompress the entire contents of 'B' than try to find a delta from A ->\nB. It is also much faster than trying to find deltas. In git, I can\naccomplish this by setting the pack window (I think this is right) to 0.\n\nWhen I set the window to 0 I one more issue. Even though the blobs are\nalready compressed on disk I still seem to pay the penalty of inflating\nthem into memory and then deflating them into the pack. When the window\nsize is 0 this is just wasted cycles. With large binary files these\nwasted cycles slow down the push/fetch operation considerably. Couldn't\nthe compressed blobs be copied into the pack without first deflating\nthem in this 0 window case?\n\nMy 'porcelain' on top of git works around these issues by first rsyncing\nthe object directories and then running the git push/fetch command\nafterward. The push/fetch command sees that the remote is up-to-date\nwith all the necessary objects and skips packing. This works and is\nfast (much faster than even packing with window of 0 because of the time\nto inflate/deflate the objects) but I'd like to remove this work-around\nin the long-term.\n\nIdeally, there are two things that I would like available with git to be\nable to remove my work-around.\n\nFirst, I would like to be able to set the packing window to 0 for all of\nthe git commands. It would be nice if I could set this in a\nper-repository config file so that any push/fetch operation would honor\nthis window. Is there currently a way to do this?\n\nSecond, I would like to not pay the penalty to inflate and then deflate\nthe objects into the pack when I use a window of 0. How hard would this\nbe? I am a capable programmer and wouldn't mind getting my hands dirty\nin the code to implement this if someone could point me in the right\ndirection.\n\nThanks for your time,\nCarl Baldwin\n\n-- \n- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -\n Carl Baldwin RADCAD (R&D CAD)\n Hewlett Packard Company\n MS 88 work: 970 898-1523\n 3404 E. Harmony Rd. work: Carl.N.Baldwin@hp.com\n Fort Collins, CO 80525 home: Carl@ecBaldwin.net\n- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -\n"},{"id":"23594","messageId":"7v7j2l833o.fsf@assigned-by-dhcp.cox.net","threadId":"4840","inReplyTo":"20060710230132.GA11132@hpsvcnb.fc.hp.com","subject":"Re: Revisiting large binary files issue.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-10T23:14:19Z","receivedAt":"2006-07-10T23:14:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carl Baldwin writes:\n\n> First, I would like to be able to set the packing window to 0 for all of\n> the git commands. It would be nice if I could set this in a\n> per-repository config file so that any push/fetch operation would honor\n> this window. Is there currently a way to do this?\n\nShould not be hard to add.\n\n> Second, I would like to not pay the penalty to inflate and then deflate\n> the objects into the pack when I use a window of 0. How hard would this\n> be? I am a capable programmer and wouldn't mind getting my hands dirty\n> in the code to implement this if someone could point me in the right\n> direction.\n\nThe problem is that unpacked objects have the single line header\n(type followed by its inflated size in decimal) which starts the\ndeflated stream, while in-pack representation of non-delta does\nnot.\n\nThere was an attempt to help doing this, but I haven't pursued it.\n\n\thttp://article.gmane.org/gmane.comp.version-control.git/17368\n"},{"id":"23596","messageId":"Pine.LNX.4.64.0607101623230.5623@g5.osdl.org","threadId":"4840","inReplyTo":"20060710230132.GA11132@hpsvcnb.fc.hp.com","subject":"Re: Revisiting large binary files issue.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-10T23:28:24Z","receivedAt":"2006-07-10T23:28:24Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 10 Jul 2006, Carl Baldwin wrote:\n> \n> When I set the window to 0 I one more issue. Even though the blobs are\n> already compressed on disk I still seem to pay the penalty of inflating\n> them into memory and then deflating them into the pack. When the window\n> size is 0 this is just wasted cycles. With large binary files these\n> wasted cycles slow down the push/fetch operation considerably. Couldn't\n> the compressed blobs be copied into the pack without first deflating\n> them in this 0 window case?\n\nThe problem is that the individual object disk format isn't actually the \nsame as the pack-file object format for one object. The header is \ndifferent: a pack-file uses a very dense bit packing, while the individual \nobject format is a bit less dense.\n\nSad, really, but it means that right now you can only re-use data that was \nalready packed (when the format matches).\n\n\t\tLinus\n"},{"id":"23640","messageId":"slrneb6gpf.gu9.Peter.B.Baumann@xp.machine.xx","threadId":"4840","inReplyTo":"7v7j2l833o.fsf@assigned-by-dhcp.cox.net","subject":"Re: Revisiting large binary files issue.","fromName":"Peter Baumann","fromEmail":"peter.b.baumann@stud.informatik.uni-erlangen.de","sentAt":"2006-07-11T06:20:31Z","receivedAt":"2006-07-11T06:20:31Z","isPatch":false,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On 2006-07-10, Junio C Hamano wrote:\n> Carl Baldwin writes:\n>\n>> First, I would like to be able to set the packing window to 0 for all of\n>> the git commands. It would be nice if I could set this in a\n>> per-repository config file so that any push/fetch operation would honor\n>> this window. Is there currently a way to do this?\n>\n> Should not be hard to add.\n>\n>> Second, I would like to not pay the penalty to inflate and then deflate\n>> the objects into the pack when I use a window of 0. How hard would this\n>> be? I am a capable programmer and wouldn't mind getting my hands dirty\n>> in the code to implement this if someone could point me in the right\n>> direction.\n>\n> The problem is that unpacked objects have the single line header\n> (type followed by its inflated size in decimal) which starts the\n> deflated stream, while in-pack representation of non-delta does\n> not.\n>\n> There was an attempt to help doing this, but I haven't pursued it.\n>\n> \thttp://article.gmane.org/gmane.comp.version-control.git/17368\n>\n>\n\nWouldn't it make more sense to convert the unpacked object reprasentation\nto the one which is used in the pack files?\n\nThis would make it really easy to just copy the object in the non-delta\ncase to the pack and avoid the inflate/deflate calls.\n\nPeter Baumann\n"},{"id":"23627","messageId":"44B371FB.2070800@b-i-t.de","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607101623230.5623@g5.osdl.org","subject":"[RFC]: Pack-file object format for individual objects (Was: Revisiting large binary files issue.)","fromName":"sf","fromEmail":"sf@b-i-t.de","sentAt":"2006-07-11T09:40:11Z","receivedAt":"2006-07-11T09:40:11Z","isPatch":false,"sender":{"key":"sf@b-i-t.de","avatar":null},"body":"Linus Torvalds wrote:\n...\n> The problem is that the individual object disk format isn't actually the \n> same as the pack-file object format for one object. The header is \n> different: a pack-file uses a very dense bit packing, while the individual \n> object format is a bit less dense.\n\nI just stumbled over the same fact and asked myself why there are still\ntwo formats. Wouldn't it make more sense to use the pack-file object\nformat for individual objects as well?\n\nAs it happens individual objects all start with nibble 7 (deflated with\ndefault _zlib_ window size of 32K) whereas in the pack-file object\nformat nibble 7 indicates delta entries which never occur as individual\nfiles.\n\nRoadmap for using pack-file format as individual object disk format:\n\nStep 1. When reading individual objects from disk check the first nibble\nand decode accordingly (see above).\n\nStep 2. When writing individual objects to disk write them in pack-file\nobject format. Make that optional (config-file parameter, command line\noption etc.)?\n\nStep 3. Remove code for (old) individual object disk format.\n\nPlease comment.\n\nRegards\n\tStephan\n"},{"id":"23641","messageId":"20060711145527.GA32468@hpsvcnb.fc.hp.com","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607101623230.5623@g5.osdl.org","subject":"Re: Revisiting large binary files issue.","fromName":"Carl Baldwin","fromEmail":"cnb@fc.hp.com","sentAt":"2006-07-11T14:55:27Z","receivedAt":"2006-07-11T14:55:27Z","isPatch":false,"sender":{"key":"cnb@fc.hp.com","avatar":null},"body":"I'd like to get my hands dirty and see for myself where the issue lies.\nI hope to have some time next week to devote to this. Is it reasonable\nto hope for a solution that is at least a lot lighter weight than the\ncurrent status quo?\n\nCarl\n\nOn Mon, Jul 10, 2006 at 04:28:24PM -0700, Linus Torvalds wrote:\n> \n> \n> On Mon, 10 Jul 2006, Carl Baldwin wrote:\n> > \n> > When I set the window to 0 I one more issue. Even though the blobs are\n> > already compressed on disk I still seem to pay the penalty of inflating\n> > them into memory and then deflating them into the pack. When the window\n> > size is 0 this is just wasted cycles. With large binary files these\n> > wasted cycles slow down the push/fetch operation considerably. Couldn't\n> > the compressed blobs be copied into the pack without first deflating\n> > them in this 0 window case?\n> \n> The problem is that the individual object disk format isn't actually the \n> same as the pack-file object format for one object. The header is \n> different: a pack-file uses a very dense bit packing, while the individual \n> object format is a bit less dense.\n> \n> Sad, really, but it means that right now you can only re-use data that was \n> already packed (when the format matches).\n> \n> \t\tLinus\n> \n\n-- \n- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -\n Carl Baldwin RADCAD (R&D CAD)\n Hewlett Packard Company\n MS 88 work: 970 898-1523\n 3404 E. Harmony Rd. work: Carl.N.Baldwin@hp.com\n Fort Collins, CO 80525 home: Carl@ecBaldwin.net\n- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -\n"},{"id":"23643","messageId":"Pine.LNX.4.64.0607111004360.5623@g5.osdl.org","threadId":"4840","inReplyTo":"20060711145527.GA32468@hpsvcnb.fc.hp.com","subject":"Re: Revisiting large binary files issue.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T17:09:48Z","receivedAt":"2006-07-11T17:09:48Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, Carl Baldwin wrote:\n>\n> I'd like to get my hands dirty and see for myself where the issue lies.\n> I hope to have some time next week to devote to this. Is it reasonable\n> to hope for a solution that is at least a lot lighter weight than the\n> current status quo?\n\nOk, I decided to see how nasty an object database change would be.\n\nIt doesn't look too bad. The following three patches implement what looks \nlike a workable model.\n\nNOTE NOTE NOTE! It makes the new \"binary headers\" the default, and that \nwill mean that unless you have applied the first two patches, any \nrepository that has had objects added with the new git version WILL NOT BE \nREADABLE BY AN OLDER GIT VERSION!\n\nSo I think that the first two patches can be added to the main git branch \npretty much immediately (after people have tested this all a _bit_ more, \nof course), because the first two patches just add the capability to \n_read_ a mixed-format repository.\n\nThe third patch is the one that actually starts _writing_ new-format repo. \nIt should be applied with extreme care, although it can basically be \nde-fanged by setting the default initial value of the \"use_binary_headers\" \nto 0, which will make it not write the binary headers by default.\n\nI have _not_ verified that the actual object format is identical to the \npack-file one, but it should be. It's simple enough.\n\nThe three patches will be sent as replies to this email.\n\n\t\tLinus\n"},{"id":"23644","messageId":"Pine.LNX.4.64.0607111009550.5623@g5.osdl.org","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111004360.5623@g5.osdl.org","subject":"[PATCH 1/3] Make the unpacked object header functions static to sha1_file.c","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T17:10:29Z","receivedAt":"2006-07-11T17:10:29Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nNobody else uses them, and I'm going to start changing them.\n\nSigned-off-by: Linus Torvalds \n---\n cache.h | 2 --\n sha1_file.c | 4 ++--\n 2 files changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex b5e3f8f..d433d46 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -219,8 +219,6 @@ int safe_create_leading_directories(char\n char *enter_repo(char *path, int strict);\n \n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\n-extern int unpack_sha1_header(z_stream *stream, void *map, unsigned long mapsize, void *buffer, unsigned long size);\n-extern int parse_sha1_header(char *hdr, char *type, unsigned long *sizep);\n extern int sha1_object_info(const unsigned char *, char *, unsigned long *);\n extern void * unpack_sha1_file(void *map, unsigned long mapsize, char *type, unsigned long *size);\n extern void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 459430a..8734d50 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -684,7 +684,7 @@ static void *map_sha1_file_internal(cons\n \treturn map;\n }\n \n-int unpack_sha1_header(z_stream *stream, void *map, unsigned long mapsize, void *buffer, unsigned long size)\n+static int unpack_sha1_header(z_stream *stream, void *map, unsigned long mapsize, void *buffer, unsigned long size)\n {\n \t/* Get the data stream */\n \tmemset(stream, 0, sizeof(*stream));\n@@ -720,7 +720,7 @@ static void *unpack_sha1_rest(z_stream *\n * too permissive for what we want to check. So do an anal\n * object header parse by hand.\n */\n-int parse_sha1_header(char *hdr, char *type, unsigned long *sizep)\n+static int parse_sha1_header(char *hdr, char *type, unsigned long *sizep)\n {\n \tint i;\n \tunsigned long size;\n"},{"id":"23645","messageId":"Pine.LNX.4.64.0607111010320.5623@g5.osdl.org","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111004360.5623@g5.osdl.org","subject":"[PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T17:12:08Z","receivedAt":"2006-07-11T17:12:08Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThe pack-file format is slightly different from the traditional git\nobject format, in that it has a much denser binary header encoding.\n\nThe traditional format uses an ASCII string with type and length\ninformation, which is somewhat wasteful.\n\nSigned-off-by: Linus Torvalds \n---\n\nThis should probably be applied to the main tree asap if we think\nthis is at all a worthwhile exercise. But somebody should verify that I \ngot the format right first!\n\n sha1_file.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++--------\n 1 files changed, 57 insertions(+), 9 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8734d50..ca5f0c0 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -697,9 +697,9 @@ static int unpack_sha1_header(z_stream *\n \treturn inflate(stream, 0);\n }\n \n-static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size)\n+static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size, unsigned int hdrlen)\n {\n-\tint bytes = strlen(buffer) + 1;\n+\tint bytes = hdrlen;\n \tunsigned char *buf = xmalloc(1+size);\n \n \tmemcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);\n@@ -720,9 +720,9 @@ static void *unpack_sha1_rest(z_stream *\n * too permissive for what we want to check. So do an anal\n * object header parse by hand.\n */\n-static int parse_sha1_header(char *hdr, char *type, unsigned long *sizep)\n+static int parse_ascii_sha1_header(char *hdr, char *type, unsigned long *sizep)\n {\n-\tint i;\n+\tint i, bytes = 0;\n \tunsigned long size;\n \n \t/*\n@@ -733,6 +733,7 @@ static int parse_sha1_header(char *hdr, \n \ti = 10;\n \tfor (;;) {\n \t\tchar c = *hdr++;\n+\t\tbytes++;\n \t\tif (c == ' ')\n \t\t\tbreak;\n \t\tif (!--i)\n@@ -746,6 +747,7 @@ static int parse_sha1_header(char *hdr, \n \t * decimal format (ie \"010\" is not valid).\n \t */\n \tsize = *hdr++ - '0';\n+\tbytes++;\n \tif (size > 9)\n \t\treturn -1;\n \tif (size) {\n@@ -754,6 +756,7 @@ static int parse_sha1_header(char *hdr, \n \t\t\tif (c > 9)\n \t\t\t\tbreak;\n \t\t\thdr++;\n+\t\t\tbytes++;\n \t\t\tsize = size * 10 + c;\n \t\t}\n \t}\n@@ -762,20 +765,65 @@ static int parse_sha1_header(char *hdr, \n \t/*\n \t * The length must be followed by a zero byte\n \t */\n-\treturn *hdr ? -1 : 0;\n+\tbytes++;\n+\tif (*hdr)\n+\t\tbytes = -1;\n+\treturn bytes;\n+}\n+\n+static int parse_binary_sha1_header(char *hdr, char *type, unsigned long *sizep)\n+{\n+\tunsigned char c;\n+\tint bytes = 1;\n+\tunsigned long size;\n+\tunsigned object_type, bits;\n+\tstatic const char *typename[8] = {\n+\t\tNULL,\t/* OBJ_EXT */\n+\t\t\"commit\", \"tree\", \"blob\", \"tag\",\n+\t\tNULL, NULL, NULL \n+\t};\t\n+\n+\tc = *hdr++;\n+\tobject_type = (c >> 4) & 7;\n+\tif (!typename[object_type])\n+\t\treturn -1;\n+\tstrcpy(type, typename[object_type]);\n+\tsize = c & 15;\n+\tbits = 4;\n+\twhile (!(c & 0x80)) {\n+\t\tif (bits >= 8*sizeof(unsigned long))\n+\t\t\treturn -1;\n+\t\tc = *hdr++;\n+\t\tsize += (unsigned long) (c & 0x7f) << bits;\n+\t\tbytes++;\n+\t\tbits += 7;\n+\t}\n+\t*sizep = size;\n+\treturn bytes;\n+}\n+\n+static int parse_sha1_header(char *hdr, char *type, unsigned long *sizep)\n+{\n+\tint retval = parse_ascii_sha1_header(hdr, type, sizep);\n+\tif (retval < 0)\n+\t\tretval = parse_binary_sha1_header(hdr, type, sizep);\n+\treturn retval;\n }\n \n void * unpack_sha1_file(void *map, unsigned long mapsize, char *type, unsigned long *size)\n {\n-\tint ret;\n+\tint ret, hdrlen;\n \tz_stream stream;\n \tchar hdr[8192];\n \n \tret = unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr));\n-\tif (ret < Z_OK || parse_sha1_header(hdr, type, size) < 0)\n+\tif (ret < Z_OK)\n+\t\treturn NULL;\n+\thdrlen = parse_sha1_header(hdr, type, size);\n+\tif (hdrlen < 0)\n \t\treturn NULL;\n \n-\treturn unpack_sha1_rest(&stream, hdr, *size);\n+\treturn unpack_sha1_rest(&stream, hdr, *size, hdrlen);\n }\n \n /* forward declaration for a mutually recursive function */\n@@ -1192,7 +1240,7 @@ struct packed_git *find_sha1_pack(const \n \n int sha1_object_info(const unsigned char *sha1, char *type, unsigned long *sizep)\n {\n-\tint status;\n+\tint status, hdrlen;\n \tunsigned long mapsize, size;\n \tvoid *map;\n \tz_stream stream;\n"},{"id":"23646","messageId":"Pine.LNX.4.64.0607111012110.5623@g5.osdl.org","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111004360.5623@g5.osdl.org","subject":"[PATCH 3/3] Enable the new binary header format for unpacked objects","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T17:16:23Z","receivedAt":"2006-07-11T17:16:23Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nEnable the new binary header format for unpacked objects\n\nThis makes unpacked objects use the same header encoding as the packed\nobjects do, which should eventually allow us to be able to re-use the\nobject data directly when creating pack-files (rather than having to\ndecode and re-encode the data when inserting it in a pack).\n\nIt's enabled by default, but can be disabled with\n\n\t[core]\n\t\tBinaryHeaders = false\n\nin the config file.\n\nWe can read both formats, of course, so you can have a mixed archive.\n\nSigned-off-by: Linus Torvalds \n---\n\nThis should _not_ be applied to the main git sources, at least not\nwith the default being \"use_binary_headers = 1\".\n\nBut if you change that initial assignment to 0, this should be reasonably \ngood.\n\nNot extensively tested, of course. It fails t9102-git-svn-deep-rmdir.sh \nfor me for some reason, I didn't really look at it yet, since this whole \nthing is more for Carl Baldwin to play with right now.\n\n Documentation/config.txt | 5 ++++\n cache.h | 1 +\n config.c | 5 ++++\n environment.c | 1 +\n sha1_file.c | 65 ++++++++++++++++++++++++++++++++++++++++------\n 5 files changed, 69 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 0b434c1..bc95416 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -97,6 +97,11 @@ core.compression::\n \tcompression, and 1..9 are various speed/size tradeoffs, 9 being\n \tslowest.\n \n+core.binaryheaders::\n+\tA boolean, that if set to false, disables the use of the\n+\tnew-style binary objects headers that share the same format with\n+\tthe headers in a pack file.\n+\n alias.*::\n \tCommand aliases for the gitlink:git[1] command wrapper - e.g.\n \tafter defining \"alias.last = cat-file commit HEAD\", the invocation\ndiff --git a/cache.h b/cache.h\nindex d433d46..756d89f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -176,6 +176,7 @@ extern int commit_lock_file(struct lock_\n extern void rollback_lock_file(struct lock_file *);\n \n /* Environment bits from configuration mechanism */\n+extern int use_binary_headers;\n extern int trust_executable_bit;\n extern int assume_unchanged;\n extern int prefer_symlink_refs;\ndiff --git a/config.c b/config.c\nindex 8445f7d..2497447 100644\n--- a/config.c\n+++ b/config.c\n@@ -289,6 +289,11 @@ int git_default_config(const char *var, \n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.binaryheaders\")) {\n+\t\tuse_binary_headers = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"user.name\")) {\n \t\tstrlcpy(git_default_name, value, sizeof(git_default_name));\n \t\treturn 0;\ndiff --git a/environment.c b/environment.c\nindex 43823ff..340214d 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -11,6 +11,7 @@ #include \"cache.h\"\n \n char git_default_email[MAX_GITNAME];\n char git_default_name[MAX_GITNAME];\n+int use_binary_headers = 1;\n int trust_executable_bit = 1;\n int assume_unchanged = 0;\n int prefer_symlink_refs = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex ca5f0c0..700f455 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -699,11 +699,14 @@ static int unpack_sha1_header(z_stream *\n \n static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size, unsigned int hdrlen)\n {\n-\tint bytes = hdrlen;\n+\tunsigned long bytes = hdrlen, n;\n \tunsigned char *buf = xmalloc(1+size);\n \n-\tmemcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);\n-\tbytes = stream->total_out - bytes;\n+\tn = stream->total_out - bytes;\n+\tif (n > size)\n+\t\tn = size;\n+\tmemcpy(buf, (char *) buffer + bytes, n);\n+\tbytes = n;\n \tif (bytes < size) {\n \t\tstream->next_out = buf + bytes;\n \t\tstream->avail_out = size - bytes;\n@@ -1240,7 +1243,7 @@ struct packed_git *find_sha1_pack(const \n \n int sha1_object_info(const unsigned char *sha1, char *type, unsigned long *sizep)\n {\n-\tint status, hdrlen;\n+\tint status;\n \tunsigned long mapsize, size;\n \tvoid *map;\n \tz_stream stream;\n@@ -1349,24 +1352,70 @@ void *read_object_with_reference(const u\n \t}\n }\n \n+static int write_binary_header(unsigned char *hdr, enum object_type type, unsigned long len)\n+{\n+\tint hdr_len;\n+\tunsigned char c;\n+\n+\tc = (type << 4) | (len & 15);\n+\tlen >>= 4;\n+\thdr_len = 1;\n+\twhile (len) {\n+\t\t*hdr++ = c;\n+\t\thdr_len++;\n+\t\tc = (len & 0x7f);\n+\t\tlen >>= 7;\n+\t}\n+\t*hdr = c | 0x80;\n+\treturn hdr_len;\n+}\n+\n+static int generate_binary_header(unsigned char *hdr, const char *type, unsigned long len)\n+{\n+\tint obj_type;\n+\n+\tif (!strcmp(type, blob_type))\n+\t\tobj_type = OBJ_BLOB;\n+\telse if (!strcmp(type, tree_type))\n+\t\tobj_type = OBJ_TREE;\n+\telse if (!strcmp(type, commit_type))\n+\t\tobj_type = OBJ_COMMIT;\n+\telse if (!strcmp(type, tag_type))\n+\t\tobj_type = OBJ_TAG;\n+\telse\n+\t\tdie(\"trying to generate bogus object of type '%s'\", type);\n+\treturn write_binary_header(hdr, obj_type, len);\n+}\n+\n char *write_sha1_file_prepare(void *buf,\n \t\t\t unsigned long len,\n \t\t\t const char *type,\n \t\t\t unsigned char *sha1,\n \t\t\t unsigned char *hdr,\n-\t\t\t int *hdrlen)\n+\t\t\t int *hdrlenp)\n {\n \tSHA_CTX c;\n+\tint hdr_len;\n \n-\t/* Generate the header */\n-\t*hdrlen = sprintf((char *)hdr, \"%s %lu\", type, len)+1;\n+\t/*\n+\t * Generate the header.\n+\t *\n+\t * NOTE! Regardless of whether we end up using the ASCII\n+\t * or binary header, we always generate the SHA1 of the\n+\t * file as if we had the ASCII header.\n+\t */\n+\thdr_len = sprintf((char *)hdr, \"%s %lu\", type, len)+1;\n \n \t/* Sha1.. */\n \tSHA1_Init(&c);\n-\tSHA1_Update(&c, hdr, *hdrlen);\n+\tSHA1_Update(&c, hdr, hdr_len);\n \tSHA1_Update(&c, buf, len);\n \tSHA1_Final(sha1, &c);\n \n+\tif (use_binary_headers)\n+\t\thdr_len = generate_binary_header(hdr, type, len);\n+\t*hdrlenp = hdr_len;\n+\n \treturn sha1_file_name(sha1);\n }\n \n"},{"id":"23647","messageId":"Pine.LNX.4.64.0607111053270.5623@g5.osdl.org","threadId":"4840","inReplyTo":"44B371FB.2070800@b-i-t.de","subject":"Re: [RFC]: Pack-file object format for individual objects (Was: Revisiting large binary files issue.)","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T18:00:32Z","receivedAt":"2006-07-11T18:00:32Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n[ I read my personal mailbox first, so I didn't see this one until after I \n had already written my version.. ]\n\nOn Tue, 11 Jul 2006, sf wrote:\n> \n> I just stumbled over the same fact and asked myself why there are still\n> two formats. Wouldn't it make more sense to use the pack-file object\n> format for individual objects as well?\n\nYes, see the git list for a series of patches that try to do this.\n\n> As it happens individual objects all start with nibble 7 (deflated with\n> default _zlib_ window size of 32K) whereas in the pack-file object\n> format nibble 7 indicates delta entries which never occur as individual\n> files.\n\nI didn't actually do it that way, but it would be better to make the \n\"parse_ascii_sha1_header()\" more strict, and only accept the old names. \n\nRight now my patch-series could in theory accept something that is _not_ \nan ASCII header (eg it would be a binary header that just happened to have \nthe format \"x n\\0\", where \"n\" was a valid number).\n\n> Step 1. When reading individual objects from disk check the first nibble\n> and decode accordingly (see above).\n\nCheck more than that, but yes, this should be tightened up in my \nseries.\n\n> Step 2. When writing individual objects to disk write them in pack-file\n> object format. Make that optional (config-file parameter, command line\n> option etc.)?\n\nDone.\n\n> Step 3. Remove code for (old) individual object disk format.\n\nWell, I'm not sure how necessary that even is. We actually do have to \ngenerate the old header regardless, if for no other reason than the fact \nthat we generate the SHA1 names based on it (even if we then write a \nnew-style dense binary header to disk and discard the ASCII header).\n\nHaving it there means that you can always just get a new version of git, \nand never worry about how old the archive you're working with is.\n\n(And then doing a \"git repack -a -d\" will make any archive also work with \nan old-style git, since the pack-file format didn't change, and a \"git \nrepack\" thus ends up always creating something that is readable by \nanybody, including old clients).\n\n\t\tLinus\n"},{"id":"23651","messageId":"Pine.LNX.4.63.0607112031150.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111010320.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-11T18:40:35Z","receivedAt":"2006-07-11T18:40:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Jul 2006, Linus Torvalds wrote:\n\n> @@ -762,20 +765,65 @@ static int parse_sha1_header(char *hdr, \n> \t/*\n> \t * The length must be followed by a zero byte\n> \t */\n> -\treturn *hdr ? -1 : 0;\n> +\tbytes++;\n> +\tif (*hdr)\n> +\t\tbytes = -1;\n> +\treturn bytes;\n\nWhy not just say\n\n\treturn *hdr ? -1 : bytes;\n\n> +}\n> +\n> +static int parse_binary_sha1_header(char *hdr, char *type, unsigned long *sizep)\n> +{\n> +\tunsigned char c;\n> +\tint bytes = 1;\n> +\tunsigned long size;\n> +\tunsigned object_type, bits;\n> +\tstatic const char *typename[8] = {\n> +\t\tNULL,\t/* OBJ_EXT */\n> +\t\t\"commit\", \"tree\", \"blob\", \"tag\",\n> +\t\tNULL, NULL, NULL \n> +\t};\t\n> +\n> +\tc = *hdr++;\n> +\tobject_type = (c >> 4) & 7;\n> +\tif (!typename[object_type])\n> +\t\treturn -1;\n\nYou might want to add a comment saying \"since the type is lowercase in \nascii format, object_type must be 6 or 7, which is an invalid object \ntype.\" It took me a little to figure that out...\n\n> +\tstrcpy(type, typename[object_type]);\n> +\tsize = c & 15;\n\nJust a nit here: I think 0xf is easier to read with boolean operations.\n\n> +\tbits = 4;\n> +\twhile (!(c & 0x80)) {\n> +\t\tif (bits >= 8*sizeof(unsigned long))\n> +\t\t\treturn -1;\n> +\t\tc = *hdr++;\n> +\t\tsize += (unsigned long) (c & 0x7f) << bits;\n> +\t\tbytes++;\n> +\t\tbits += 7;\n> +\t}\n\nAre you not losing the last byte by putting the \"while\" _before_ instead \nof _after_ the loop?\n\n> @@ -1192,7 +1240,7 @@ struct packed_git *find_sha1_pack(const \n> \n> int sha1_object_info(const unsigned char *sha1, char *type, unsigned long *sizep)\n> {\n> -\tint status;\n> +\tint status, hdrlen;\n> \tunsigned long mapsize, size;\n> \tvoid *map;\n> \tz_stream stream;\n> -\n\nThis hunk is unnecessary, right?\n\nCiao,\nDscho\n"},{"id":"23654","messageId":"Pine.LNX.4.64.0607111153170.5623@g5.osdl.org","threadId":"4840","inReplyTo":"Pine.LNX.4.63.0607112031150.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T18:58:24Z","receivedAt":"2006-07-11T18:58:24Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, Johannes Schindelin wrote:\n> \n> Why not just say\n> \n> \treturn *hdr ? -1 : bytes;\n\nHey, whatever works. I rewrote more, and edited some of my changes down \nagain..\n\n> You might want to add a comment saying \"since the type is lowercase in \n> ascii format, object_type must be 6 or 7, which is an invalid object \n> type.\" It took me a little to figure that out...\n\nIt's not even correct in my version - I check the ASCII header _first_, so \nby the time it looks at the binary one, it already knows it's not ascii.\n\nThe problematic case is actually the other way around: my \n\"parse_ascii_sha1_header()\" isn't strict enough.\n\nOr, more likely, the parse_sha1_header() function should just be changed \nto check the binary format first (and then add your comment about why that \nis safe).\n\n> > +\tbits = 4;\n> > +\twhile (!(c & 0x80)) {\n> > +\t\tif (bits >= 8*sizeof(unsigned long))\n> > +\t\t\treturn -1;\n> > +\t\tc = *hdr++;\n> > +\t\tsize += (unsigned long) (c & 0x7f) << bits;\n> > +\t\tbytes++;\n> > +\t\tbits += 7;\n> > +\t}\n> \n> Are you not losing the last byte by putting the \"while\" _before_ instead \n> of _after_ the loop?\n\nNo. The very first byte can have the 0x80 end marker, when the size was \nbetween 0..15.\n\n> > int sha1_object_info(const unsigned char *sha1, char *type, unsigned long *sizep)\n> > {\n> > -\tint status;\n> > +\tint status, hdrlen;\n> > \tunsigned long mapsize, size;\n> > \tvoid *map;\n> > \tz_stream stream;\n> \n> This hunk is unnecessary, right?\n\nYeah, never mind. That function didn't actually need the hdrlen, it only \ncared about the SHA1.\n\n\t\tLinus\n"},{"id":"23655","messageId":"Pine.LNX.4.63.0607112116270.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111153170.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-11T19:20:28Z","receivedAt":"2006-07-11T19:20:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Jul 2006, Linus Torvalds wrote:\n\n> On Tue, 11 Jul 2006, Johannes Schindelin wrote:\n> \n> > You might want to add a comment saying \"since the type is lowercase in \n> > ascii format, object_type must be 6 or 7, which is an invalid object \n> > type.\" It took me a little to figure that out...\n> \n> It's not even correct in my version - I check the ASCII header _first_, so \n> by the time it looks at the binary one, it already knows it's not ascii.\n\nJust realized it all by myself...\n\n> Or, more likely, the parse_sha1_header() function should just be changed \n> to check the binary format first (and then add your comment about why that \n> is safe).\n\nYes, exactly.\n\n> > > +\tbits = 4;\n> > > +\twhile (!(c & 0x80)) {\n> > > +\t\tif (bits >= 8*sizeof(unsigned long))\n> > > +\t\t\treturn -1;\n> > > +\t\tc = *hdr++;\n> > > +\t\tsize += (unsigned long) (c & 0x7f) << bits;\n> > > +\t\tbytes++;\n> > > +\t\tbits += 7;\n> > > +\t}\n> > \n> > Are you not losing the last byte by putting the \"while\" _before_ instead \n> > of _after_ the loop?\n> \n> No. The very first byte can have the 0x80 end marker, when the size was \n> between 0..15.\n\nYes, I understand now. I was a little confused by the way it is written...\n\nThanks for the clarification,\nDscho\n"},{"id":"23658","messageId":"Pine.LNX.4.64.0607111241460.5623@g5.osdl.org","threadId":"4840","inReplyTo":"Pine.LNX.4.63.0607112116270.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T19:48:08Z","receivedAt":"2006-07-11T19:48:08Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, Johannes Schindelin wrote:\n> \n> > Or, more likely, the parse_sha1_header() function should just be changed \n> > to check the binary format first (and then add your comment about why that \n> > is safe).\n> \n> Yes, exactly.\n\nHere's a newer verson of [2/3], with these issues fixed. It actually fixes \nthings twice: (a) by parsing the binary version first (which makes sense \nfor a totally independent reason - if that is going to be the \"default\" \nversion in the long run, we should just test it first anyway) and (b) by \nmaking the ASCII version parser stricter too.\n\nThis did, btw, also fix the test failure, so the fact that the ASCII \nheader parser wasn't careful enough was actually a problem in real life.\n\nSo please throw away the old version.\n\n\t\tLinus\n\n---\nFrom: Linus Torvalds \nSubject: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"\n\nThe pack-file format is slightly different from the traditional git\nobject format, in that it has a much denser binary header encoding.\n\nThe traditional format uses an ASCII string with type and length\ninformation, which is somewhat wasteful.\n\nSigned-off-by: Linus Torvalds \n---\n sha1_file.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------\n 1 files changed, 82 insertions(+), 11 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8734d50..15ccf5e 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -697,9 +697,9 @@ static int unpack_sha1_header(z_stream *\n \treturn inflate(stream, 0);\n }\n \n-static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size)\n+static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size, unsigned int hdrlen)\n {\n-\tint bytes = strlen(buffer) + 1;\n+\tint bytes = hdrlen;\n \tunsigned char *buf = xmalloc(1+size);\n \n \tmemcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);\n@@ -720,25 +720,40 @@ static void *unpack_sha1_rest(z_stream *\n * too permissive for what we want to check. So do an anal\n * object header parse by hand.\n */\n-static int parse_sha1_header(char *hdr, char *type, unsigned long *sizep)\n+static int parse_ascii_sha1_header(char *hdr, char *type, unsigned long *sizep)\n {\n-\tint i;\n+\tint bytes = 0;\n \tunsigned long size;\n \n \t/*\n \t * The type can be at most ten bytes (including the \n \t * terminating '\\0' that we add), and is followed by\n \t * a space. \n+\t *\n+\t * We want at least three characters, and they should\n+\t * all be normal lower-case letters.\n \t */\n-\ti = 10;\n \tfor (;;) {\n-\t\tchar c = *hdr++;\n+\t\tunsigned char c = *hdr++;\n+\t\tbytes++;\n \t\tif (c == ' ')\n \t\t\tbreak;\n-\t\tif (!--i)\n+\t\tif (bytes >= 10)\n \t\t\treturn -1;\n \t\t*type++ = c;\n+\n+\t\t/*\n+\t\t * The high nybble must be 6 of 7, see\n+\t\t * parse_binary_header(). This covers\n+\t\t * all ASCII lowercase characters.\n+\t\t */\n+\t\tif (c < 0x60 || c > 0x7f)\n+\t\t\treturn -1;\n \t}\n+\n+\t/* Minimum three letters and the space */\n+\tif (bytes < 4)\n+\t\treturn -1;\n \t*type = 0;\n \n \t/*\n@@ -746,6 +761,7 @@ static int parse_sha1_header(char *hdr, \n \t * decimal format (ie \"010\" is not valid).\n \t */\n \tsize = *hdr++ - '0';\n+\tbytes++;\n \tif (size > 9)\n \t\treturn -1;\n \tif (size) {\n@@ -754,6 +770,7 @@ static int parse_sha1_header(char *hdr, \n \t\t\tif (c > 9)\n \t\t\t\tbreak;\n \t\t\thdr++;\n+\t\t\tbytes++;\n \t\t\tsize = size * 10 + c;\n \t\t}\n \t}\n@@ -762,20 +779,74 @@ static int parse_sha1_header(char *hdr, \n \t/*\n \t * The length must be followed by a zero byte\n \t */\n-\treturn *hdr ? -1 : 0;\n+\tbytes++;\n+\treturn *hdr ? -1 : bytes;\n+}\n+\n+/*\n+ * We never confuse a binary header with an old ASCII one,\n+ * because the ASCII one will always start with a lower-case\n+ * letter, meaning that the first byte will be of the form\n+ * 0x6? or 0x7?.\n+ *\n+ * That in turn would be parsed as object type 6 or 7, neither\n+ * of which is valid for a unpacked object (object type 7 is\n+ * a delta, and can only exist in a pack-file, while object type\n+ * 6 is invalid).\n+ */\n+static int parse_binary_sha1_header(char *hdr, char *type, unsigned long *sizep)\n+{\n+\tunsigned char c;\n+\tint bytes = 1;\n+\tunsigned long size;\n+\tunsigned object_type, bits;\n+\tstatic const char *typename[8] = {\n+\t\tNULL,\t/* OBJ_EXT */\n+\t\t\"commit\", \"tree\", \"blob\", \"tag\",\n+\t\tNULL, NULL, NULL\n+\t};\n+\n+\tc = *hdr++;\n+\tobject_type = (c >> 4) & 7;\n+\tif (!typename[object_type])\n+\t\treturn -1;\n+\tstrcpy(type, typename[object_type]);\n+\tsize = c & 15;\n+\tbits = 4;\n+\twhile (!(c & 0x80)) {\n+\t\tif (bits >= 8*sizeof(unsigned long))\n+\t\t\treturn -1;\n+\t\tc = *hdr++;\n+\t\tsize += (unsigned long) (c & 0x7f) << bits;\n+\t\tbytes++;\n+\t\tbits += 7;\n+\t}\n+\t*sizep = size;\n+\treturn bytes;\n+}\n+\n+static int parse_sha1_header(char *hdr, char *type, unsigned long *sizep)\n+{\n+\tint retval = parse_binary_sha1_header(hdr, type, sizep);\n+\tif (retval < 0)\n+\t\tretval = parse_ascii_sha1_header(hdr, type, sizep);\n+\treturn retval;\n }\n \n void * unpack_sha1_file(void *map, unsigned long mapsize, char *type, unsigned long *size)\n {\n-\tint ret;\n+\tint ret, hdrlen;\n \tz_stream stream;\n \tchar hdr[8192];\n \n \tret = unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr));\n-\tif (ret < Z_OK || parse_sha1_header(hdr, type, size) < 0)\n+\tif (ret < Z_OK)\n+\t\treturn NULL;\n+\thdrlen = parse_sha1_header(hdr, type, size);\n+\tif (hdrlen < 0)\n \t\treturn NULL;\n \n-\treturn unpack_sha1_rest(&stream, hdr, *size);\n+\treturn unpack_sha1_rest(&stream, hdr, *size, hdrlen);\n }\n \n /* forward declaration for a mutually recursive function */\n"},{"id":"23664","messageId":"44B4172B.3070503@stephan-feder.de","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111010320.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"sf","fromEmail":"sf-gmane@stephan-feder.de","sentAt":"2006-07-11T21:24:59Z","receivedAt":"2006-07-11T21:24:59Z","isPatch":true,"sender":{"key":"sf-gmane@stephan-feder.de","avatar":null},"body":"Linus Torvalds wrote:\n> The pack-file format is slightly different from the traditional git\n> object format, in that it has a much denser binary header encoding.\n> \n> The traditional format uses an ASCII string with type and length\n> information, which is somewhat wasteful.\n\nAnd in the traditional format type and length are compressed whereas in\nthe pack-file format they are not.\n\n> This should probably be applied to the main tree asap if we think\n> this is at all a worthwhile exercise. But somebody should verify that I \n> got the format right first!\n\nSorry but see above.\n\nRegards\n\tStephan\n"},{"id":"23663","messageId":"Pine.LNX.4.63.0607112324330.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111241460.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-11T21:25:17Z","receivedAt":"2006-07-11T21:25:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Jul 2006, Linus Torvalds wrote:\n\n> On Tue, 11 Jul 2006, Johannes Schindelin wrote:\n> > \n> > > Or, more likely, the parse_sha1_header() function should just be changed \n> > > to check the binary format first (and then add your comment about why that \n> > > is safe).\n> > \n> > Yes, exactly.\n> \n> Here's a newer verson of [2/3], with these issues fixed. It actually fixes \n> things twice: (a) by parsing the binary version first (which makes sense \n> for a totally independent reason - if that is going to be the \"default\" \n> version in the long run, we should just test it first anyway) and (b) by \n> making the ASCII version parser stricter too.\n\nMelikey.\n\nCiao,\nDscho\n"},{"id":"23665","messageId":"44B41BFD.8010808@stephan-feder.de","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111053270.5623@g5.osdl.org","subject":"Re: [RFC]: Pack-file object format for individual objects (Was: Revisiting large binary files issue.)","fromName":"sf","fromEmail":"sf-gmane@stephan-feder.de","sentAt":"2006-07-11T21:45:33Z","receivedAt":"2006-07-11T21:45:33Z","isPatch":false,"sender":{"key":"sf-gmane@stephan-feder.de","avatar":null},"body":"Linus Torvalds wrote:\n...\n> On Tue, 11 Jul 2006, sf wrote:\n...\n>> Step 1. When reading individual objects from disk check the first nibble\n>> and decode accordingly (see above).\n> \n> Check more than that, but yes, this should be tightened up in my \n> series.\n\nJust look at the first byte of the object file _without doing any\ndecompression_. It is 0x78 _if and only if_ the object file is in the\ntraditional format.\n\n>> Step 3. Remove code for (old) individual object disk format.\n> \n> Well, I'm not sure how necessary that even is. We actually do have to \n> generate the old header regardless, if for no other reason than the fact \n> that we generate the SHA1 names based on it (even if we then write a \n> new-style dense binary header to disk and discard the ASCII header).\n> \n> Having it there means that you can always just get a new version of git, \n> and never worry about how old the archive you're working with is.\n> \n> (And then doing a \"git repack -a -d\" will make any archive also work with \n> an old-style git, since the pack-file format didn't change, and a \"git \n> repack\" thus ends up always creating something that is readable by \n> anybody, including old clients).\n\nAgreed.\n\nRegards\n\tStephan\n"},{"id":"23666","messageId":"7vlkqz3jb1.fsf@assigned-by-dhcp.cox.net","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111241460.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-11T21:47:46Z","receivedAt":"2006-07-11T21:47:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Here's a newer verson of [2/3], with these issues fixed. It actually fixes \n> things twice: (a) by parsing the binary version first (which makes sense \n> for a totally independent reason - if that is going to be the \"default\" \n> version in the long run, we should just test it first anyway) and (b) by \n> making the ASCII version parser stricter too.\n\nWait a minute.\n\n read-sha1-file maps sha1-file-internal (for unpacked one), and then\n calls unpack-sha1-file.\n\n unpack-sha1-file calls unpack-sha1-header to start inflation,\n lets parse-sha1-header to read the header in the inflated\n buffer, and calls unpack-sha1-rest to inflate the rest.\n\nBut in packs, we have binary header not deflated, followed by\ndeflated payload. If we want to copy things from loose objects\ninto pack without changing the packfile format, this change\nwould not help, I suspect.\n\nAt least, your updated unpack_sha1_file() needs to check for\nbinary header first (starting from \"map\"), and if that starts\nwith binary header, start inflating after the header to extract\nthe payload. Otherwise you would do the traditional.\n"},{"id":"23667","messageId":"Pine.LNX.4.64.0607111449190.5623@g5.osdl.org","threadId":"4840","inReplyTo":"44B4172B.3070503@stephan-feder.de","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T22:09:07Z","receivedAt":"2006-07-11T22:09:07Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, sf wrote:\n> \n> And in the traditional format type and length are compressed whereas in\n> the pack-file format they are not.\n\nAhh. Yes.\n\n> > This should probably be applied to the main tree asap if we think\n> > this is at all a worthwhile exercise. But somebody should verify that I \n> > got the format right first!\n> \n> Sorry but see above.\n\nGood catch, thanks indeed.\n\nDoing that for unpacked objects would in fact make a lot of things much \nsimpler, so it would be good to do. The _bad_ part is that this also makes \nit a lot harder to see the difference between a \"binary header\" and a \n\"compressed ASCII header\". The two are not \"obviously different\" any more.\n\nThe common byte sequence for a compressed stream is\n\n\t78 9c ...\n\nwhere the first byte is the CMF byte (compression info and method).\n\nBut it's not the only possible such sequence according to the zlib format.\n\n(The 16-bit hex number in MSB format, ie 0x789c above, is defined to have \na built-in checksum, so that it must be a multiple of 31 according to the \nstandard: 0x789c = 996 * 31).\n\nSo if we have a uncompressed header, we'd need to add a separate 2-byte \nfingerprint to it _before_ the real header that isn't divisible by 31, and \nuse that as the thing to test.\n\nHo humm. I'll see what I can come up with.\n\n\t\tLinus\n"},{"id":"23668","messageId":"Pine.LNX.4.64.0607111512420.5623@g5.osdl.org","threadId":"4840","inReplyTo":"44B41BFD.8010808@stephan-feder.de","subject":"Re: [RFC]: Pack-file object format for individual objects (Was: Revisiting large binary files issue.)","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T22:17:44Z","receivedAt":"2006-07-11T22:17:44Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, sf wrote:\n> \n> Just look at the first byte of the object file _without doing any\n> decompression_. It is 0x78 _if and only if_ the object file is in the\n> traditional format.\n\n0x78 isn't the only valid flag for a zlib stream, as far as I can tell.\n\nIt may be the only one _in_practice_, of course, but the zlib standard \ndefines the first byte as\n\n - for low bits: CM (compression method):\n\n \"This identifies the compression method used in the file. CM = 8\n denotes the \"deflate\" compression method with a window size up\n to 32K. This is the method used by gzip and PNG (see\n references [1] and [2] in Chapter 3, below, for the reference\n documents). CM = 15 is reserved. It might be used in a future\n version of this specification to indicate the presence of an\n extra field before the compressed data.\"\n\n - four high bits are CINFO: \n\n \"For CM = 8, CINFO is the base-2 logarithm of the LZ77 window\n size, minus eight (CINFO=7 indicates a 32K window size). Values\n of CINFO above 7 are not allowed in this version of the\n specification. CINFO is not defined in this specification for\n CM not equal to 8.\"\n\nso 0x78 means \"deflate with 32kB window size\", but I don't see anything \nguaranteeing that we might not see something else for an object that \ncannot be compressed, for example.\n\nAnyway, the good news is that _if_ 0x78 is indeed the only possible value, \nthen it is also an illegal value for an unpacked object in pack-file \nformat (type=7 being OBJ_DELTA) and we wouldn't need any other flag for \nthis.\n\nI just don't know if it's the only possible one..\n\n\t\tLinus\n"},{"id":"23671","messageId":"44B42567.9010307@stephan-feder.de","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111449190.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"sf","fromEmail":"sf-gmane@stephan-feder.de","sentAt":"2006-07-11T22:25:43Z","receivedAt":"2006-07-11T22:25:43Z","isPatch":true,"sender":{"key":"sf-gmane@stephan-feder.de","avatar":null},"body":"Linus Torvalds wrote:\n...\n> The common byte sequence for a compressed stream is\n> \n> \t78 9c ...\n> \n> where the first byte is the CMF byte (compression info and method).\n> \n> But it's not the only possible such sequence according to the zlib format.\n\nNo, but 0x78 is the only first byte ever produced by git; the files are\nalways deflated (second nibble is 8) with window size 32K (first nibble\nis 7).\n"},{"id":"23670","messageId":"Pine.LNX.4.64.0607111520020.5623@g5.osdl.org","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111512420.5623@g5.osdl.org","subject":"Re: [RFC]: Pack-file object format for individual objects (Was: Revisiting large binary files issue.)","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-11T22:26:49Z","receivedAt":"2006-07-11T22:26:49Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, Linus Torvalds wrote:\n> \n> - for low bits: CM (compression method):\n> \n> \"This identifies the compression method used in the file. CM = 8\n> denotes the \"deflate\" compression method with a window size up\n> to 32K. This is the method used by gzip and PNG (see\n> references [1] and [2] in Chapter 3, below, for the reference\n> documents). CM = 15 is reserved. It might be used in a future\n> version of this specification to indicate the presence of an\n> extra field before the compressed data.\"\n> \n> - four high bits are CINFO: \n> \n> \"For CM = 8, CINFO is the base-2 logarithm of the LZ77 window\n> size, minus eight (CINFO=7 indicates a 32K window size). Values\n> of CINFO above 7 are not allowed in this version of the\n> specification. CINFO is not defined in this specification for\n> CM not equal to 8.\"\n> \n> so 0x78 means \"deflate with 32kB window size\", but I don't see anything \n> guaranteeing that we might not see something else for an object that \n> cannot be compressed, for example.\n\nAhh. Looking at the zlib sources, I see\n\n /* Write the zlib header */\n if (s->status == INIT_STATE) {\n\n uInt header = (Z_DEFLATED + ((s->w_bits-8)<<4)) << 8;\n uInt level_flags = (s->level-1) >> 1;\n \n if (level_flags > 3) level_flags = 3;\n header |= (level_flags << 6);\n if (s->strstart != 0) header |= PRESET_DICT;\n header += 31 - (header % 31);\n\n s->status = BUSY_STATE;\n putShortMSB(s, header);\n\n(which is that first 16-bit word, MSB first). So we'll always have the \nZ-DEFLATED (8) there in the low four bits, but the high nybble will be \n\"s->w_bits-8\" where w_bits comes from windowBits, and I think we can \ndepend on it beign 15:\n\n \"The windowBits parameter is the base two logarithm of the window size\n (the size of the history buffer). It should be in the range 8..15 for this\n version of the library. Larger values of this parameter result in better\n compression at the expense of memory usage. The default value is 15 if\n deflateInit is used instead.\"\n\nso since we use deflateInit(), we know the window will be 15.\n\nSo I guess we _can_ depend on the first byte being 0x78 for our use.\n\nGoodie.\n\n\t\tLinus\n"},{"id":"23673","messageId":"7vejwr3ftl.fsf@assigned-by-dhcp.cox.net","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111449190.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-11T23:03:02Z","receivedAt":"2006-07-11T23:03:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> So if we have a uncompressed header, we'd need to add a separate 2-byte \n> fingerprint to it _before_ the real header that isn't divisible by 31, and \n> use that as the thing to test.\n>\n> Ho humm. I'll see what I can come up with.\n\nI do not like to rely too heavily on what zlib compression's\nbeginning of stream looks like.\n\nI think the new format can be deflated new header (fully\nsynched) followed by deflated payload.\n\nSo the sequence unpack-sha1-header followed by parse-sha1-header\nwould notice we are dealing with new format and reinitialize the\ndeflator at the point where the header deflator left off.\n\nWouldn't that work?\n"},{"id":"23677","messageId":"Pine.LNX.4.64.0607111656250.5623@g5.osdl.org","threadId":"4840","inReplyTo":"7vejwr3ftl.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-12T00:03:20Z","receivedAt":"2006-07-12T00:03:20Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, Junio C Hamano wrote:\n> \n> I do not like to rely too heavily on what zlib compression's\n> beginning of stream looks like.\n\nWell, I normally would agree with you if it was a \"oh, all our zlib \nobjects seem to start with 0x78\" thing, but after having dug into both the \nzlib standard (which is actually an RFC, not just some random thing), and \nlooked at the sources, it's definitely the case that the \"0x78\" byte isn't \njust an implementation detail.\n\nIt's actually really part of the specs, and not just happenstance.\n\n> I think the new format can be deflated new header (fully\n> synched) followed by deflated payload.\n\nThat would work, but on the other hand, one of the advantages of doing the \nnew format would be that the \"check size and type\" code wouldn't even need \nto call into the zlib code. \n\nAnyway, I think this following patch replaces the old 2/3 and 3/3 (it \nstill depends on the original [1/3] cleanup.\n\n(It also renames and reverses the meaning of the config file option: it's \nnow \"[core] LegacyHeaders = true\" for using legacy headers.)\n\nNot heavily tested, but seems ok.\n\nsf? Dscho? Can you check this thing out?\n\n\t\tLinus\n----\n Documentation/config.txt | 6 +++\n cache.h | 1 \n config.c | 5 ++\n environment.c | 1 \n sha1_file.c | 106 +++++++++++++++++++++++++++++++++++++++++++---\n 5 files changed, 111 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 0b434c1..9780c89 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -97,6 +97,12 @@ core.compression::\n \tcompression, and 1..9 are various speed/size tradeoffs, 9 being\n \tslowest.\n \n+core.legacyheaders::\n+\tA boolean which enables the legacy object header format in case\n+\tyou want to interoperate with old clients accessing the object\n+\tdatabase directly (where the \"http://\" and \"rsync://\" protocols\n+\tcount as direct access).\n+\n alias.*::\n \tCommand aliases for the gitlink:git[1] command wrapper - e.g.\n \tafter defining \"alias.last = cat-file commit HEAD\", the invocation\ndiff --git a/cache.h b/cache.h\nindex d433d46..eee5fc9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -176,6 +176,7 @@ extern int commit_lock_file(struct lock_\n extern void rollback_lock_file(struct lock_file *);\n \n /* Environment bits from configuration mechanism */\n+extern int use_legacy_headers;\n extern int trust_executable_bit;\n extern int assume_unchanged;\n extern int prefer_symlink_refs;\ndiff --git a/config.c b/config.c\nindex 8445f7d..0ac6aeb 100644\n--- a/config.c\n+++ b/config.c\n@@ -279,6 +279,11 @@ int git_default_config(const char *var, \n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.legacyheaders\")) {\n+\t\tuse_legacy_headers = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"core.compression\")) {\n \t\tint level = git_config_int(var, value);\n \t\tif (level == -1)\ndiff --git a/environment.c b/environment.c\nindex 97d42b1..d80a39a 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -11,6 +11,7 @@ #include \"cache.h\"\n \n char git_default_email[MAX_GITNAME];\n char git_default_name[MAX_GITNAME];\n+int use_legacy_headers = 0;\n int trust_executable_bit = 1;\n int assume_unchanged = 0;\n int prefer_symlink_refs = 0;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8734d50..475b23d 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -684,26 +684,74 @@ static void *map_sha1_file_internal(cons\n \treturn map;\n }\n \n-static int unpack_sha1_header(z_stream *stream, void *map, unsigned long mapsize, void *buffer, unsigned long size)\n+static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz)\n {\n+\tunsigned char c;\n+\tunsigned int word, bits;\n+\tunsigned long size;\n+\tstatic const char *typename[8] = {\n+\t\tNULL,\t/* OBJ_EXT */\n+\t\t\"commit\", \"tree\", \"blob\", \"tag\",\n+\t\tNULL, NULL, NULL\n+\t};\n+\tconst char *type;\n+\n \t/* Get the data stream */\n \tmemset(stream, 0, sizeof(*stream));\n \tstream->next_in = map;\n \tstream->avail_in = mapsize;\n \tstream->next_out = buffer;\n-\tstream->avail_out = size;\n+\tstream->avail_out = bufsiz;\n+\n+\t/*\n+\t * Is it a zlib-compressed buffer? If so, the first byte\n+\t * must be 0x78 (15-bit window size, deflated), and the\n+\t * first 16-bit word is evenly divisible by 31\n+\t */\n+\tword = (map[0] << 8) + map[1];\n+\tif (map[0] == 0x78 && !(word % 31)) {\n+\t\tinflateInit(stream);\n+\t\treturn inflate(stream, 0); \n+\t}\n+\n+\tc = *map++;\n+\tmapsize--;\n+\ttype = typename[(c >> 4) & 7];\n+\tif (!type)\n+\t\treturn -1;\n+\t\n+\tbits = 4;\n+\tsize = c & 0xf;\n+\twhile (!(c & 0x80)) {\n+\t\tif (bits >= 8*sizeof(long))\n+\t\t\treturn -1;\n+\t\tc = *map++;\n+\t\tsize += (c & 0x7f) << bits;\n+\t\tbits += 7;\n+\t\tmapsize--;\n+\t}\n \n+\t/* Set up the stream for the rest.. */\n+\tstream->next_in = map;\n+\tstream->avail_in = mapsize;\n \tinflateInit(stream);\n-\treturn inflate(stream, 0);\n+\n+\t/* And generate the fake traditional header */\n+\tstream->total_out = 1 + snprintf(buffer, bufsiz, \"%s %lu\", type, size);\n+\treturn 0;\n }\n \n static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size)\n {\n \tint bytes = strlen(buffer) + 1;\n \tunsigned char *buf = xmalloc(1+size);\n+\tunsigned long n;\n \n-\tmemcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);\n-\tbytes = stream->total_out - bytes;\n+\tn = stream->total_out - bytes;\n+\tif (n > size)\n+\t\tn = size;\n+\tmemcpy(buf, (char *) buffer + bytes, n);\n+\tbytes = n;\n \tif (bytes < size) {\n \t\tstream->next_out = buf + bytes;\n \t\tstream->avail_out = size - bytes;\n@@ -1414,6 +1462,49 @@ static int write_buffer(int fd, const vo\n \treturn 0;\n }\n \n+static int write_binary_header(unsigned char *hdr, enum object_type type, unsigned long len)\n+{\n+\tint hdr_len;\n+\tunsigned char c;\n+\n+\tc = (type << 4) | (len & 15);\n+\tlen >>= 4;\n+\thdr_len = 1;\n+\twhile (len) {\n+\t\t*hdr++ = c;\n+\t\thdr_len++;\n+\t\tc = (len & 0x7f);\n+\t\tlen >>= 7;\n+\t}\n+\t*hdr = c | 0x80;\n+\treturn hdr_len;\n+}\n+\n+static void setup_object_header(z_stream *stream, const char *type, unsigned long len)\n+{\n+\tint obj_type, hdr;\n+\n+\tif (use_legacy_headers) {\n+\t\twhile (deflate(stream, 0) == Z_OK)\n+\t\t\t/* nothing */;\n+\t\treturn;\n+\t}\n+\tif (!strcmp(type, blob_type))\n+\t\tobj_type = OBJ_BLOB;\n+\telse if (!strcmp(type, tree_type))\n+\t\tobj_type = OBJ_TREE;\n+\telse if (!strcmp(type, commit_type))\n+\t\tobj_type = OBJ_COMMIT;\n+\telse if (!strcmp(type, tag_type))\n+\t\tobj_type = OBJ_TAG;\n+\telse\n+\t\tdie(\"trying to generate bogus object of type '%s'\", type);\n+\thdr = write_binary_header(stream->next_out, obj_type, len);\n+\tstream->total_out = hdr;\n+\tstream->next_out += hdr;\n+\tstream->avail_out -= hdr;\n+}\n+\n int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)\n {\n \tint size;\n@@ -1459,7 +1550,7 @@ int write_sha1_file(void *buf, unsigned \n \t/* Set it up */\n \tmemset(&stream, 0, sizeof(stream));\n \tdeflateInit(&stream, zlib_compression_level);\n-\tsize = deflateBound(&stream, len+hdrlen);\n+\tsize = 8 + deflateBound(&stream, len+hdrlen);\n \tcompressed = xmalloc(size);\n \n \t/* Compress it */\n@@ -1469,8 +1560,7 @@ int write_sha1_file(void *buf, unsigned \n \t/* First header.. */\n \tstream.next_in = hdr;\n \tstream.avail_in = hdrlen;\n-\twhile (deflate(&stream, 0) == Z_OK)\n-\t\t/* nothing */;\n+\tsetup_object_header(&stream, type, len);\n \n \t/* Then the data itself.. */\n \tstream.next_in = buf;\n"},{"id":"23678","messageId":"Pine.LNX.4.63.0607120226210.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111656250.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-12T00:39:26Z","receivedAt":"2006-07-12T00:39:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 11 Jul 2006, Linus Torvalds wrote:\n\n> On Tue, 11 Jul 2006, Junio C Hamano wrote:\n> > \n> > I do not like to rely too heavily on what zlib compression's\n> > beginning of stream looks like.\n> \n> Well, I normally would agree with you if it was a \"oh, all our zlib \n> objects seem to start with 0x78\" thing, but after having dug into both the \n> zlib standard (which is actually an RFC, not just some random thing), and \n> looked at the sources, it's definitely the case that the \"0x78\" byte isn't \n> just an implementation detail.\n> \n> It's actually really part of the specs, and not just happenstance.\n\nGood.\n\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -684,26 +684,74 @@ static void *map_sha1_file_internal(cons\n> \treturn map;\n> }\n> \n> -static int unpack_sha1_header(z_stream *stream, void *map, unsigned long mapsize, void *buffer, unsigned long size)\n> +static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz)\n> {\n> +\tunsigned char c;\n> +\tunsigned int word, bits;\n> +\tunsigned long size;\n> +\tstatic const char *typename[8] = {\n> +\t\tNULL,\t/* OBJ_EXT */\n> +\t\t\"commit\", \"tree\", \"blob\", \"tag\",\n> +\t\tNULL, NULL, NULL\n> +\t};\n\nI completely forgot to mention that type_names[] is already declared in \nobject.h. Obviously, it is not really important, but maybe it would be \nbetter to obey the DRY principle (think addition of \"bind\" object type).\n\n> +\twhile (!(c & 0x80)) {\n> +\t\tif (bits >= 8*sizeof(long))\n\nAnother nit: while it is safe to assume that sizeof(long) == \nsizeof(unsigned long), it was nevertheless a little confusing to yours \ntruly (especially since you changed it since your last patch).\n\n> static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size)\n> {\n> \tint bytes = strlen(buffer) + 1;\n> \tunsigned char *buf = xmalloc(1+size);\n> +\tunsigned long n;\n> \n> -\tmemcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);\n> -\tbytes = stream->total_out - bytes;\n> +\tn = stream->total_out - bytes;\n> +\tif (n > size)\n> +\t\tn = size;\n\nJust out of curiosity: when can this happen? I mean, there is no error or \nsomething which could tell the caller that not the whole object was \ncopied.\n\n> \tmemset(&stream, 0, sizeof(stream));\n> \tdeflateInit(&stream, zlib_compression_level);\n> -\tsize = deflateBound(&stream, len+hdrlen);\n> +\tsize = 8 + deflateBound(&stream, len+hdrlen);\n\nAgain, I had to think why this is correct. I think it should be something \nlike 2 + sizeof(unsigned long) * 8 / 7, but I did not think all that hard.\n\nIt looks good to me, but I did not really test...\n\nCiao,\nDscho\n"},{"id":"23679","messageId":"7vveq31wgo.fsf@assigned-by-dhcp.cox.net","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111656250.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-12T00:46:31Z","receivedAt":"2006-07-12T00:46:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> It's actually really part of the specs, and not just happenstance.\n\n> Well, I normally would agree with you if it was a \"oh, all our zlib \n> objects seem to start with 0x78\" thing, but after having dug into both the \n> zlib standard (which is actually an RFC, not just some random thing), and \n> looked at the sources, it's definitely the case that the \"0x78\" byte isn't \n> just an implementation detail.\n\nOk, I do not think we would worry about casting use of deflate +\n32k windowsize in stone that much, and being able to check the\nsize and type without inflating certainly is attractive.\nValidating FCHECK bits is surely a nice touch. Thanks.\n\n> Anyway, I think this following patch replaces the old 2/3 and 3/3 (it \n> still depends on the original [1/3] cleanup.\n>\n> (It also renames and reverses the meaning of the config file option: it's \n> now \"[core] LegacyHeaders = true\" for using legacy headers.)\n>\n> Not heavily tested, but seems ok.\n\nI'd queue it in \"pu\" with reversed default and then move it to\n\"next\" later.\n\n> static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size)\n> {\n> \tint bytes = strlen(buffer) + 1;\n> \tunsigned char *buf = xmalloc(1+size);\n> +\tunsigned long n;\n> \n> -\tmemcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);\n> -\tbytes = stream->total_out - bytes;\n> +\tn = stream->total_out - bytes;\n> +\tif (n > size)\n> +\t\tn = size;\n> +\tmemcpy(buf, (char *) buffer + bytes, n);\n> +\tbytes = n;\n> \tif (bytes < size) {\n> \t\tstream->next_out = buf + bytes;\n> \t\tstream->avail_out = size - bytes;\n\nThis one looks like an independent fix for a well spotted bug.\n\n> int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)\n> {\n> \tint size;\n> @@ -1459,7 +1550,7 @@ int write_sha1_file(void *buf, unsigned \n> \t/* Set it up */\n> \tmemset(&stream, 0, sizeof(stream));\n> \tdeflateInit(&stream, zlib_compression_level);\n> -\tsize = deflateBound(&stream, len+hdrlen);\n> +\tsize = 8 + deflateBound(&stream, len+hdrlen);\n> \tcompressed = xmalloc(size);\n> \n> \t/* Compress it */\n\nI am wondring what this eight is. You would pack 7 7-bit length\nplus 4-bit totalling 49+4 = 53-bit length (plus 4-bit type). Is\nit an unwritten decision that the format would not deal with\nobjects larger than 2^53 (which is probably fine but looks\nmagic)?\n"},{"id":"23681","messageId":"Pine.LNX.4.64.0607112040050.5623@g5.osdl.org","threadId":"4840","inReplyTo":"7vveq31wgo.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-12T03:42:49Z","receivedAt":"2006-07-12T03:42:49Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, Junio C Hamano wrote:\n> > \tunsigned char *buf = xmalloc(1+size);\n> > +\tunsigned long n;\n> > \n> > -\tmemcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);\n> > -\tbytes = stream->total_out - bytes;\n> > +\tn = stream->total_out - bytes;\n> > +\tif (n > size)\n> > +\t\tn = size;\n> > +\tmemcpy(buf, (char *) buffer + bytes, n);\n> > +\tbytes = n;\n> > \tif (bytes < size) {\n> > \t\tstream->next_out = buf + bytes;\n> > \t\tstream->avail_out = size - bytes;\n> \n> This one looks like an independent fix for a well spotted bug.\n\nYeah, well, the \"bug\" only happens if you screw something up (which I \ntriggered both times I tried to rewrite this ;)\n\nOr possibly it the object is corrupt.\n\nBut yes, it's independent.\n\n> > -\tsize = deflateBound(&stream, len+hdrlen);\n> > +\tsize = 8 + deflateBound(&stream, len+hdrlen);\n> > \tcompressed = xmalloc(size);\n> > \n> > \t/* Compress it */\n> \n> I am wondring what this eight is. You would pack 7 7-bit length\n> plus 4-bit totalling 49+4 = 53-bit length (plus 4-bit type). Is\n> it an unwritten decision that the format would not deal with\n> objects larger than 2^53 (which is probably fine but looks\n> magic)?\n\n8 was just \"obviously enough\".\n\nThe \"hdrlen\" part should already give us _way_ more padding than we need \n(the old-fashioned header will deflate to something bigger than the new \nheader, so deflateBound() even _without_ the extra space should be \nplenty).\n\nBut I decided to add a few bytes just because it won't hurt.\n\n\t\tLinus\n"},{"id":"23684","messageId":"Pine.LNX.4.64.0607111755180.5623@g5.osdl.org","threadId":"4840","inReplyTo":"Pine.LNX.4.63.0607120226210.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-12T03:45:31Z","receivedAt":"2006-07-12T03:45:31Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 12 Jul 2006, Johannes Schindelin wrote:\n>> \n> I completely forgot to mention that type_names[] is already declared in \n> object.h. Obviously, it is not really important, but maybe it would be \n> better to obey the DRY principle (think addition of \"bind\" object type).\n\nActually, the type_names[] array in object.h is _totally_ different. That \none is indexed by TYPE_xxx, not OBJ_xxxx.\n\nAnd yeah, it's stupid to have two different numberings for the same thing, \nand we really really shouldn't. But the OBJ_xxx numbering was originally a \na totally internal implementation detail to just the pack-files. \n\nI shouldn't have introduced the new TYPE_xxx macros. I should just have \nused the same OBJ_xxx macros that we use in the pack-file.\n\nJunio: this patch is totally independent from the other patches, and is on \ntop of you current \"master\". It gets rid of TYPE_xxx in favor of the \nOBJ_xxx enums, which are moved from pack.h into object.h.\n\nThat way, we can't have people mixing up the two different kinds of type \nenumeration.\n\n(Eventually, I want to get rid of passing around the \"type strings\" \nentirely, and this will help - no confusion about two different integer \nenumeration).\n\n\t\tLinus\n\n----\ndiff --git a/blob.c b/blob.c\nindex 496f270..d1af2e6 100644\n--- a/blob.c\n+++ b/blob.c\n@@ -10,12 +10,12 @@ struct blob *lookup_blob(const unsigned \n \tif (!obj) {\n \t\tstruct blob *ret = alloc_blob_node();\n \t\tcreated_object(sha1, &ret->object);\n-\t\tret->object.type = TYPE_BLOB;\n+\t\tret->object.type = OBJ_BLOB;\n \t\treturn ret;\n \t}\n \tif (!obj->type)\n-\t\tobj->type = TYPE_BLOB;\n-\tif (obj->type != TYPE_BLOB) {\n+\t\tobj->type = OBJ_BLOB;\n+\tif (obj->type != OBJ_BLOB) {\n \t\terror(\"Object %s is a %s, not a blob\",\n \t\t sha1_to_hex(sha1), typename(obj->type));\n \t\treturn NULL;\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex ae901dd..cb38f44 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -285,9 +285,9 @@ int cmd_diff(int argc, const char **argv\n \t\tobj = deref_tag(obj, NULL, 0);\n \t\tif (!obj)\n \t\t\tdie(\"invalid object '%s' given.\", name);\n-\t\tif (obj->type == TYPE_COMMIT)\n+\t\tif (obj->type == OBJ_COMMIT)\n \t\t\tobj = &((struct commit *)obj)->tree->object;\n-\t\tif (obj->type == TYPE_TREE) {\n+\t\tif (obj->type == OBJ_TREE) {\n \t\t\tif (ARRAY_SIZE(ent) <= ents)\n \t\t\t\tdie(\"more than %d trees given: '%s'\",\n \t\t\t\t (int) ARRAY_SIZE(ent), name);\n@@ -297,7 +297,7 @@ int cmd_diff(int argc, const char **argv\n \t\t\tents++;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (obj->type == TYPE_BLOB) {\n+\t\tif (obj->type == OBJ_BLOB) {\n \t\t\tif (2 <= blobs)\n \t\t\t\tdie(\"more than two blobs given: '%s'\", name);\n \t\t\tmemcpy(blob[blobs].sha1, obj->sha1, 20);\ndiff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c\nindex 6527482..b3c4f98 100644\n--- a/builtin-fmt-merge-msg.c\n+++ b/builtin-fmt-merge-msg.c\n@@ -177,7 +177,7 @@ static void shortlog(const char *name, u\n \tint flags = UNINTERESTING | TREECHANGE | SEEN | SHOWN | ADDED;\n \n \tbranch = deref_tag(parse_object(sha1), sha1_to_hex(sha1), 40);\n-\tif (!branch || branch->type != TYPE_COMMIT)\n+\tif (!branch || branch->type != OBJ_COMMIT)\n \t\treturn;\n \n \tsetup_revisions(0, NULL, rev, NULL);\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 4c2f7df..a79bac3 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -891,9 +891,9 @@ static int grep_tree(struct grep_opt *op\n static int grep_object(struct grep_opt *opt, const char **paths,\n \t\t struct object *obj, const char *name)\n {\n-\tif (obj->type == TYPE_BLOB)\n+\tif (obj->type == OBJ_BLOB)\n \t\treturn grep_sha1(opt, obj->sha1, name);\n-\tif (obj->type == TYPE_COMMIT || obj->type == TYPE_TREE) {\n+\tif (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {\n \t\tstruct tree_desc tree;\n \t\tvoid *data;\n \t\tint hit;\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 63bad0e..8f32871 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -167,16 +167,16 @@ static void show_commit_list(struct rev_\n \t\tconst char *name = pending->name;\n \t\tif (obj->flags & (UNINTERESTING | SEEN))\n \t\t\tcontinue;\n-\t\tif (obj->type == TYPE_TAG) {\n+\t\tif (obj->type == OBJ_TAG) {\n \t\t\tobj->flags |= SEEN;\n \t\t\tadd_object_array(obj, name, &objects);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (obj->type == TYPE_TREE) {\n+\t\tif (obj->type == OBJ_TREE) {\n \t\t\tprocess_tree((struct tree *)obj, &objects, NULL, name);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (obj->type == TYPE_BLOB) {\n+\t\tif (obj->type == OBJ_BLOB) {\n \t\t\tprocess_blob((struct blob *)obj, &objects, NULL, name);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/commit.c b/commit.c\nindex c6bf10d..46d5867 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -56,7 +56,7 @@ static struct commit *check_commit(struc\n \t\t\t\t const unsigned char *sha1,\n \t\t\t\t int quiet)\n {\n-\tif (obj->type != TYPE_COMMIT) {\n+\tif (obj->type != OBJ_COMMIT) {\n \t\tif (!quiet)\n \t\t\terror(\"Object %s is a %s, not a commit\",\n \t\t\t sha1_to_hex(sha1), typename(obj->type));\n@@ -86,11 +86,11 @@ struct commit *lookup_commit(const unsig\n \tif (!obj) {\n \t\tstruct commit *ret = alloc_commit_node();\n \t\tcreated_object(sha1, &ret->object);\n-\t\tret->object.type = TYPE_COMMIT;\n+\t\tret->object.type = OBJ_COMMIT;\n \t\treturn ret;\n \t}\n \tif (!obj->type)\n-\t\tobj->type = TYPE_COMMIT;\n+\t\tobj->type = OBJ_COMMIT;\n \treturn check_commit(obj, sha1, 0);\n }\n \ndiff --git a/describe.c b/describe.c\nindex 8e68d5d..324ca89 100644\n--- a/describe.c\n+++ b/describe.c\n@@ -67,7 +67,7 @@ static int get_name(const char *path, co\n \t * Otherwise only annotated tags are used.\n \t */\n \tif (!strncmp(path, \"refs/tags/\", 10)) {\n-\t\tif (object->type == TYPE_TAG)\n+\t\tif (object->type == OBJ_TAG)\n \t\t\tprio = 2;\n \t\telse\n \t\t\tprio = 1;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex f2c51eb..b7824db 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -46,7 +46,7 @@ static int rev_list_insert_ref(const cha\n {\n \tstruct object *o = deref_tag(parse_object(sha1), path, 0);\n \n-\tif (o && o->type == TYPE_COMMIT)\n+\tif (o && o->type == OBJ_COMMIT)\n \t\trev_list_push((struct commit *)o, SEEN);\n \n \treturn 0;\n@@ -256,14 +256,14 @@ static int mark_complete(const char *pat\n {\n \tstruct object *o = parse_object(sha1);\n \n-\twhile (o && o->type == TYPE_TAG) {\n+\twhile (o && o->type == OBJ_TAG) {\n \t\tstruct tag *t = (struct tag *) o;\n \t\tif (!t->tagged)\n \t\t\tbreak; /* broken repository */\n \t\to->flags |= COMPLETE;\n \t\to = parse_object(t->tagged->sha1);\n \t}\n-\tif (o && o->type == TYPE_COMMIT) {\n+\tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n \t\tcommit->object.flags |= COMPLETE;\n \t\tinsert_by_date(commit, &complete);\n@@ -357,7 +357,7 @@ static int everything_local(struct ref *\n \t\t * in sync with the other side at some time after\n \t\t * that (it is OK if we guess wrong here).\n \t\t */\n-\t\tif (o->type == TYPE_COMMIT) {\n+\t\tif (o->type == OBJ_COMMIT) {\n \t\t\tstruct commit *commit = (struct commit *)o;\n \t\t\tif (!cutoff || cutoff < commit->date)\n \t\t\t\tcutoff = commit->date;\n@@ -376,7 +376,7 @@ static int everything_local(struct ref *\n \t\tstruct object *o = deref_tag(lookup_object(ref->old_sha1),\n \t\t\t\t\t NULL, 0);\n \n-\t\tif (!o || o->type != TYPE_COMMIT || !(o->flags & COMPLETE))\n+\t\tif (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE))\n \t\t\tcontinue;\n \n \t\tif (!(o->flags & SEEN)) {\ndiff --git a/fsck-objects.c b/fsck-objects.c\nindex ef54a8a..e167f41 100644\n--- a/fsck-objects.c\n+++ b/fsck-objects.c\n@@ -297,13 +297,13 @@ static int fsck_sha1(unsigned char *sha1\n \tif (obj->flags & SEEN)\n \t\treturn 0;\n \tobj->flags |= SEEN;\n-\tif (obj->type == TYPE_BLOB)\n+\tif (obj->type == OBJ_BLOB)\n \t\treturn 0;\n-\tif (obj->type == TYPE_TREE)\n+\tif (obj->type == OBJ_TREE)\n \t\treturn fsck_tree((struct tree *) obj);\n-\tif (obj->type == TYPE_COMMIT)\n+\tif (obj->type == OBJ_COMMIT)\n \t\treturn fsck_commit((struct commit *) obj);\n-\tif (obj->type == TYPE_TAG)\n+\tif (obj->type == OBJ_TAG)\n \t\treturn fsck_tag((struct tag *) obj);\n \t/* By now, parse_object() would've returned NULL instead. */\n \treturn objerror(obj, \"unknown type '%d' (internal fsck error)\", obj->type);\n@@ -472,7 +472,7 @@ static int fsck_cache_tree(struct cache_\n \t\t}\n \t\tmark_reachable(obj, REACHABLE);\n \t\tobj->used = 1;\n-\t\tif (obj->type != TYPE_TREE)\n+\t\tif (obj->type != OBJ_TREE)\n \t\t\terr |= objerror(obj, \"non-tree in cache-tree\");\n \t}\n \tfor (i = 0; i < it->subtree_nr; i++)\ndiff --git a/http-push.c b/http-push.c\nindex f761584..4768619 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1784,16 +1784,16 @@ static int get_delta(struct rev_info *re\n \n \t\tif (obj->flags & (UNINTERESTING | SEEN))\n \t\t\tcontinue;\n-\t\tif (obj->type == TYPE_TAG) {\n+\t\tif (obj->type == OBJ_TAG) {\n \t\t\tobj->flags |= SEEN;\n \t\t\tp = add_one_object(obj, p);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (obj->type == TYPE_TREE) {\n+\t\tif (obj->type == OBJ_TREE) {\n \t\t\tp = process_tree((struct tree *)obj, p, NULL, name);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (obj->type == TYPE_BLOB) {\n+\t\tif (obj->type == OBJ_BLOB) {\n \t\t\tp = process_blob((struct blob *)obj, p, NULL, name);\n \t\t\tcontinue;\n \t\t}\n@@ -1960,12 +1960,12 @@ static int ref_newer(const unsigned char\n \t * old. Otherwise we require --force.\n \t */\n \to = deref_tag(parse_object(old_sha1), NULL, 0);\n-\tif (!o || o->type != TYPE_COMMIT)\n+\tif (!o || o->type != OBJ_COMMIT)\n \t\treturn 0;\n \told = (struct commit *) o;\n \n \to = deref_tag(parse_object(new_sha1), NULL, 0);\n-\tif (!o || o->type != TYPE_COMMIT)\n+\tif (!o || o->type != OBJ_COMMIT)\n \t\treturn 0;\n \tnew = (struct commit *) o;\n \n@@ -2044,7 +2044,7 @@ static void add_remote_info_ref(struct r\n \tfwrite_buffer(ref_info, 1, len, buf);\n \tfree(ref_info);\n \n-\tif (o->type == TYPE_TAG) {\n+\tif (o->type == OBJ_TAG) {\n \t\to = deref_tag(o, ls->dentry_name, 0);\n \t\tif (o) {\n \t\t\tlen = strlen(ls->dentry_name) + 45;\ndiff --git a/name-rev.c b/name-rev.c\nindex 083d067..f92f14e 100644\n--- a/name-rev.c\n+++ b/name-rev.c\n@@ -84,14 +84,14 @@ static int name_ref(const char *path, co\n \tif (tags_only && strncmp(path, \"refs/tags/\", 10))\n \t\treturn 0;\n \n-\twhile (o && o->type == TYPE_TAG) {\n+\twhile (o && o->type == OBJ_TAG) {\n \t\tstruct tag *t = (struct tag *) o;\n \t\tif (!t->tagged)\n \t\t\tbreak; /* broken repository */\n \t\to = parse_object(t->tagged->sha1);\n \t\tderef = 1;\n \t}\n-\tif (o && o->type == TYPE_COMMIT) {\n+\tif (o && o->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)o;\n \n \t\tif (!strncmp(path, \"refs/heads/\", 11))\n@@ -111,7 +111,7 @@ static const char* get_rev_name(struct o\n \tstruct rev_name *n;\n \tstruct commit *c;\n \n-\tif (o->type != TYPE_COMMIT)\n+\tif (o->type != OBJ_COMMIT)\n \t\treturn \"undefined\";\n \tc = (struct commit *) o;\n \tn = c->util;\n@@ -172,7 +172,7 @@ int main(int argc, char **argv)\n \t\t}\n \n \t\to = deref_tag(parse_object(sha1), *argv, 0);\n-\t\tif (!o || o->type != TYPE_COMMIT) {\n+\t\tif (!o || o->type != OBJ_COMMIT) {\n \t\t\tfprintf(stderr, \"Could not get commit for %s. Skipping.\\n\",\n \t\t\t\t\t*argv);\n \t\t\tcontinue;\ndiff --git a/object.c b/object.c\nindex 37277f9..e7ca56e 100644\n--- a/object.c\n+++ b/object.c\n@@ -19,7 +19,8 @@ struct object *get_indexed_object(unsign\n }\n \n const char *type_names[] = {\n-\t\"none\", \"blob\", \"tree\", \"commit\", \"bad\"\n+\t\"none\", \"commit\", \"tree\", \"blob\", \"tag\", \n+\t\"bad type 5\", \"bad type 6\", \"delta\", \"bad\",\n };\n \n static unsigned int hash_obj(struct object *obj, unsigned int n)\n@@ -88,7 +89,7 @@ void created_object(const unsigned char \n {\n \tobj->parsed = 0;\n \tobj->used = 0;\n-\tobj->type = TYPE_NONE;\n+\tobj->type = OBJ_NONE;\n \tobj->flags = 0;\n \tmemcpy(obj->sha1, sha1, 20);\n \n@@ -131,7 +132,7 @@ struct object *lookup_unknown_object(con\n \tif (!obj) {\n \t\tunion any_object *ret = xcalloc(1, sizeof(*ret));\n \t\tcreated_object(sha1, &ret->object);\n-\t\tret->object.type = TYPE_NONE;\n+\t\tret->object.type = OBJ_NONE;\n \t\treturn &ret->object;\n \t}\n \treturn obj;\ndiff --git a/object.h b/object.h\nindex e0125e1..7893e94 100644\n--- a/object.h\n+++ b/object.h\n@@ -24,12 +24,19 @@ struct object_array {\n #define TYPE_BITS 3\n #define FLAG_BITS 27\n \n-#define TYPE_NONE 0\n-#define TYPE_BLOB 1\n-#define TYPE_TREE 2\n-#define TYPE_COMMIT 3\n-#define TYPE_TAG 4\n-#define TYPE_BAD 5\n+/*\n+ * The object type is stored in 3 bits.\n+ */\n+enum object_type {\n+\tOBJ_NONE = 0,\n+\tOBJ_COMMIT = 1,\n+\tOBJ_TREE = 2,\n+\tOBJ_BLOB = 3,\n+\tOBJ_TAG = 4,\n+\t/* 5/6 for future expansion */\n+\tOBJ_DELTA = 7,\n+\tOBJ_BAD,\n+};\n \n struct object {\n \tunsigned parsed : 1;\n@@ -40,14 +47,14 @@ struct object {\n };\n \n extern int track_object_refs;\n-extern const char *type_names[];\n+extern const char *type_names[9];\n \n extern unsigned int get_max_object_index(void);\n extern struct object *get_indexed_object(unsigned int);\n \n static inline const char *typename(unsigned int type)\n {\n-\treturn type_names[type > TYPE_TAG ? TYPE_BAD : type];\n+\treturn type_names[type > OBJ_TAG ? OBJ_BAD : type];\n }\n \n extern struct object_refs *lookup_object_refs(struct object *);\ndiff --git a/pack-check.c b/pack-check.c\ndiff --git a/pack.h b/pack.h\nindex 694e0c5..eb07b03 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -1,20 +1,7 @@\n #ifndef PACK_H\n #define PACK_H\n \n-/*\n- * The packed object type is stored in 3 bits.\n- * The type value 0 is a reserved prefix if ever there is more than 7\n- * object types, or any future format extensions.\n- */\n-enum object_type {\n-\tOBJ_EXT = 0,\n-\tOBJ_COMMIT = 1,\n-\tOBJ_TREE = 2,\n-\tOBJ_BLOB = 3,\n-\tOBJ_TAG = 4,\n-\t/* 5/6 for future expansion */\n-\tOBJ_DELTA = 7,\n-};\n+#include \"object.h\"\n \n /*\n * Packed object header\ndiff --git a/revision.c b/revision.c\nindex 7df9089..874e349 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -135,7 +135,7 @@ static struct commit *handle_commit(stru\n \t/*\n \t * Tag object? Look what it points to..\n \t */\n-\twhile (object->type == TYPE_TAG) {\n+\twhile (object->type == OBJ_TAG) {\n \t\tstruct tag *tag = (struct tag *) object;\n \t\tif (revs->tag_objects && !(flags & UNINTERESTING))\n \t\t\tadd_pending_object(revs, object, tag->tag);\n@@ -148,7 +148,7 @@ static struct commit *handle_commit(stru\n \t * Commit object? Just return it, we'll do all the complex\n \t * reachability crud.\n \t */\n-\tif (object->type == TYPE_COMMIT) {\n+\tif (object->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)object;\n \t\tif (parse_commit(commit) < 0)\n \t\t\tdie(\"unable to parse commit %s\", name);\n@@ -164,7 +164,7 @@ static struct commit *handle_commit(stru\n \t * Tree object? Either mark it uniniteresting, or add it\n \t * to the list of objects to look at later..\n \t */\n-\tif (object->type == TYPE_TREE) {\n+\tif (object->type == OBJ_TREE) {\n \t\tstruct tree *tree = (struct tree *)object;\n \t\tif (!revs->tree_objects)\n \t\t\treturn NULL;\n@@ -179,7 +179,7 @@ static struct commit *handle_commit(stru\n \t/*\n \t * Blob object? You know the drill by now..\n \t */\n-\tif (object->type == TYPE_BLOB) {\n+\tif (object->type == OBJ_BLOB) {\n \t\tstruct blob *blob = (struct blob *)object;\n \t\tif (!revs->blob_objects)\n \t\t\treturn NULL;\n@@ -494,11 +494,11 @@ static int add_parents_only(struct rev_i\n \t\treturn 0;\n \twhile (1) {\n \t\tit = get_reference(revs, arg, sha1, 0);\n-\t\tif (it->type != TYPE_TAG)\n+\t\tif (it->type != OBJ_TAG)\n \t\t\tbreak;\n \t\tmemcpy(sha1, ((struct tag*)it)->tagged->sha1, 20);\n \t}\n-\tif (it->type != TYPE_COMMIT)\n+\tif (it->type != OBJ_COMMIT)\n \t\treturn 0;\n \tcommit = (struct commit *)it;\n \tfor (parents = commit->parents; parents; parents = parents->next) {\ndiff --git a/send-pack.c b/send-pack.c\nindex 4019a4b..10bc8bc 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -151,12 +151,12 @@ static int ref_newer(const unsigned char\n \t * old. Otherwise we require --force.\n \t */\n \to = deref_tag(parse_object(old_sha1), NULL, 0);\n-\tif (!o || o->type != TYPE_COMMIT)\n+\tif (!o || o->type != OBJ_COMMIT)\n \t\treturn 0;\n \told = (struct commit *) o;\n \n \to = deref_tag(parse_object(new_sha1), NULL, 0);\n-\tif (!o || o->type != TYPE_COMMIT)\n+\tif (!o || o->type != OBJ_COMMIT)\n \t\treturn 0;\n \tnew = (struct commit *) o;\n \ndiff --git a/server-info.c b/server-info.c\nindex fdfe05a..7df628f 100644\n--- a/server-info.c\n+++ b/server-info.c\n@@ -12,7 +12,7 @@ static int add_info_ref(const char *path\n \tstruct object *o = parse_object(sha1);\n \n \tfprintf(info_ref_fp, \"%s\t%s\\n\", sha1_to_hex(sha1), path);\n-\tif (o->type == TYPE_TAG) {\n+\tif (o->type == OBJ_TAG) {\n \t\to = deref_tag(o, path, 0);\n \t\tif (o)\n \t\t\tfprintf(info_ref_fp, \"%s\t%s^{}\\n\",\ndiff --git a/sha1_name.c b/sha1_name.c\nindex f2cbafa..5fe8e5d 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -381,13 +381,13 @@ static int peel_onion(const char *name, \n \n \tsp++; /* beginning of type name, or closing brace for empty */\n \tif (!strncmp(commit_type, sp, 6) && sp[6] == '}')\n-\t\texpected_type = TYPE_COMMIT;\n+\t\texpected_type = OBJ_COMMIT;\n \telse if (!strncmp(tree_type, sp, 4) && sp[4] == '}')\n-\t\texpected_type = TYPE_TREE;\n+\t\texpected_type = OBJ_TREE;\n \telse if (!strncmp(blob_type, sp, 4) && sp[4] == '}')\n-\t\texpected_type = TYPE_BLOB;\n+\t\texpected_type = OBJ_BLOB;\n \telse if (sp[0] == '}')\n-\t\texpected_type = TYPE_NONE;\n+\t\texpected_type = OBJ_NONE;\n \telse\n \t\treturn -1;\n \n@@ -416,9 +416,9 @@ static int peel_onion(const char *name, \n \t\t\t\tmemcpy(sha1, o->sha1, 20);\n \t\t\t\treturn 0;\n \t\t\t}\n-\t\t\tif (o->type == TYPE_TAG)\n+\t\t\tif (o->type == OBJ_TAG)\n \t\t\t\to = ((struct tag*) o)->tagged;\n-\t\t\telse if (o->type == TYPE_COMMIT)\n+\t\t\telse if (o->type == OBJ_COMMIT)\n \t\t\t\to = &(((struct commit *) o)->tree->object);\n \t\t\telse\n \t\t\t\treturn error(\"%.*s: expected %s type, but the object dereferences to %s type\",\ndiff --git a/tag.c b/tag.c\nindex 74d0dab..864ac1b 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -5,7 +5,7 @@ const char *tag_type = \"tag\";\n \n struct object *deref_tag(struct object *o, const char *warn, int warnlen)\n {\n-\twhile (o && o->type == TYPE_TAG)\n+\twhile (o && o->type == OBJ_TAG)\n \t\to = parse_object(((struct tag *)o)->tagged->sha1);\n \tif (!o && warn) {\n \t\tif (!warnlen)\n@@ -21,12 +21,12 @@ struct tag *lookup_tag(const unsigned ch\n if (!obj) {\n struct tag *ret = alloc_tag_node();\n created_object(sha1, &ret->object);\n- ret->object.type = TYPE_TAG;\n+ ret->object.type = OBJ_TAG;\n return ret;\n }\n \tif (!obj->type)\n-\t\tobj->type = TYPE_TAG;\n- if (obj->type != TYPE_TAG) {\n+\t\tobj->type = OBJ_TAG;\n+ if (obj->type != OBJ_TAG) {\n error(\"Object %s is a %s, not a tree\",\n sha1_to_hex(sha1), typename(obj->type));\n return NULL;\ndiff --git a/tree.c b/tree.c\nindex 1023655..a6032e3 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -131,12 +131,12 @@ struct tree *lookup_tree(const unsigned \n \tif (!obj) {\n \t\tstruct tree *ret = alloc_tree_node();\n \t\tcreated_object(sha1, &ret->object);\n-\t\tret->object.type = TYPE_TREE;\n+\t\tret->object.type = OBJ_TREE;\n \t\treturn ret;\n \t}\n \tif (!obj->type)\n-\t\tobj->type = TYPE_TREE;\n-\tif (obj->type != TYPE_TREE) {\n+\t\tobj->type = OBJ_TREE;\n+\tif (obj->type != OBJ_TREE) {\n \t\terror(\"Object %s is a %s, not a tree\",\n \t\t sha1_to_hex(sha1), typename(obj->type));\n \t\treturn NULL;\n@@ -216,11 +216,11 @@ struct tree *parse_tree_indirect(const u\n \tdo {\n \t\tif (!obj)\n \t\t\treturn NULL;\n-\t\tif (obj->type == TYPE_TREE)\n+\t\tif (obj->type == OBJ_TREE)\n \t\t\treturn (struct tree *) obj;\n-\t\telse if (obj->type == TYPE_COMMIT)\n+\t\telse if (obj->type == OBJ_COMMIT)\n \t\t\tobj = &(((struct commit *) obj)->tree->object);\n-\t\telse if (obj->type == TYPE_TAG)\n+\t\telse if (obj->type == OBJ_TAG)\n \t\t\tobj = ((struct tag *) obj)->tagged;\n \t\telse\n \t\t\treturn NULL;\ndiff --git a/upload-pack.c b/upload-pack.c\nindex b18eb9b..2e820c9 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -326,7 +326,7 @@ static int got_sha1(char *hex, unsigned \n \t\t\to = parse_object(sha1);\n \t\tif (!o)\n \t\t\tdie(\"oops (%s)\", sha1_to_hex(sha1));\n-\t\tif (o->type == TYPE_COMMIT) {\n+\t\tif (o->type == OBJ_COMMIT) {\n \t\t\tstruct commit_list *parents;\n \t\t\tif (o->flags & THEY_HAVE)\n \t\t\t\treturn 0;\n@@ -457,7 +457,7 @@ static int send_ref(const char *refname,\n \t\to->flags |= OUR_REF;\n \t\tnr_our_refs++;\n \t}\n-\tif (o->type == TYPE_TAG) {\n+\tif (o->type == OBJ_TAG) {\n \t\to = deref_tag(o, refname, 0);\n \t\tpacket_write(1, \"%s %s^{}\\n\", sha1_to_hex(o->sha1), refname);\n \t}\n"},{"id":"23687","messageId":"Pine.LNX.4.64.0607112128270.5623@g5.osdl.org","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111755180.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-12T04:31:20Z","receivedAt":"2006-07-12T04:31:20Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, Linus Torvalds wrote:\n> \n> Junio: this patch is totally independent from the other patches, and is on \n> top of you current \"master\". It gets rid of TYPE_xxx in favor of the \n> OBJ_xxx enums, which are moved from pack.h into object.h.\n...\n> static inline const char *typename(unsigned int type)\n> {\n> -\treturn type_names[type > TYPE_TAG ? TYPE_BAD : type];\n> +\treturn type_names[type > OBJ_TAG ? OBJ_BAD : type];\n\nThat should be \"[type > OBJ_BAD ? OBJ_BAD : type]\"\n\nNot that any users will care, because the current users of the typename() \nmacro are the old TYPE_xxxx users which will never see the OBJ_DELTA case \nanyway. So it's not a big deal, but let's do it right for future users (ie \nif the pack-file things want to start using \"typename(type)\" and they \nactually _have_ delta descriptors).\n\n\t\tLinus\n"},{"id":"23693","messageId":"7vzmffz5xe.fsf@assigned-by-dhcp.cox.net","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111755180.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-12T06:35:41Z","receivedAt":"2006-07-12T06:35:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> I shouldn't have introduced the new TYPE_xxx macros. I should just have \n> used the same OBJ_xxx macros that we use in the pack-file.\n>\n> Junio: this patch is totally independent from the other patches, and is on \n> top of you current \"master\". It gets rid of TYPE_xxx in favor of the \n> OBJ_xxx enums, which are moved from pack.h into object.h.\n\nYou seem to have forgotten one file (fetch-pack.c) but that was\ntrivial. I'll apply and push it out shortly.\n\nThis might collide with a few topic branches but I have rerere\nto help me cope with it ;-).\n"},{"id":"23694","messageId":"slrneb96rd.dma.Peter.B.Baumann@xp.machine.xx","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607111656250.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Peter Baumann","fromEmail":"peter.b.baumann@stud.informatik.uni-erlangen.de","sentAt":"2006-07-12T06:49:17Z","receivedAt":"2006-07-12T06:49:17Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On 2006-07-12, Linus Torvalds wrote:\n[...]\n> Anyway, I think this following patch replaces the old 2/3 and 3/3 (it \n> still depends on the original [1/3] cleanup.\n>\n> (It also renames and reverses the meaning of the config file option: it's \n> now \"[core] LegacyHeaders = true\" for using legacy headers.)\n>\n> Not heavily tested, but seems ok.\n>\n> sf? Dscho? Can you check this thing out?\n>\n> \t\tLinus\n> ----\n[...]\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 8734d50..475b23d 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -684,26 +684,74 @@ static void *map_sha1_file_internal(cons\n> \treturn map;\n> }\n> \n> -static int unpack_sha1_header(z_stream *stream, void *map, unsigned long mapsize, void *buffer, unsigned long size)\n> +static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz)\n> {\n> +\tunsigned char c;\n> +\tunsigned int word, bits;\n> +\tunsigned long size;\n> +\tstatic const char *typename[8] = {\n> +\t\tNULL,\t/* OBJ_EXT */\n> +\t\t\"commit\", \"tree\", \"blob\", \"tag\",\n> +\t\tNULL, NULL, NULL\n> +\t};\n> +\tconst char *type;\n> +\n> \t/* Get the data stream */\n> \tmemset(stream, 0, sizeof(*stream));\n> \tstream->next_in = map;\n> \tstream->avail_in = mapsize;\n> \tstream->next_out = buffer;\n> -\tstream->avail_out = size;\n> +\tstream->avail_out = bufsiz;\n> +\n> +\t/*\n> +\t * Is it a zlib-compressed buffer? If so, the first byte\n> +\t * must be 0x78 (15-bit window size, deflated), and the\n> +\t * first 16-bit word is evenly divisible by 31\n> +\t */\n> +\tword = (map[0] << 8) + map[1];\n> +\tif (map[0] == 0x78 && !(word % 31)) {\n> +\t\tinflateInit(stream);\n> +\t\treturn inflate(stream, 0);\n> +\t}\n> +\n> +\tc = *map++;\n> +\tmapsize--;\n> +\ttype = typename[(c >> 4) & 7];\n> +\tif (!type)\n> +\t\treturn -1;\n> +\n> +\tbits = 4;\n> +\tsize = c & 0xf;\n> +\twhile (!(c & 0x80)) {\n> +\t\tif (bits >= 8*sizeof(long))\n> +\t\t\treturn -1;\n> +\t\tc = *map++;\n> +\t\tsize += (c & 0x7f) << bits;\n> +\t\tbits += 7;\n> +\t\tmapsize--;\n> +\t}\n\nThis doesn't match the logic used in unpack_object_header, which is used\nin the packs:\n\nstatic unsigned long unpack_object_header(struct packed_git *p, unsigned long offset,\n enum object_type *type, unsigned long *sizep)\n{\n\tunsigned shift;\n\tunsigned char *pack, c;\n\tunsigned long size;\n\n\tif (offset >= p->pack_size)\n\t\tdie(\"object offset outside of pack file\");\n\n\tpack = (unsigned char *) p->pack_base + offset;\n\tc = *pack++;\n\toffset++;\n\t*type = (c >> 4) & 7;\n\tsize = c & 15;\n\tshift = 4;\n\twhile (c & 0x80) {\t\t\t<==========\n\t\tif (offset >= p->pack_size)\n\t \tdie(\"object offset outside of pack file\");\n\t\tc = *pack++;\n\t\toffset++;\n\t\tsize += (c & 0x7f) << shift;\n\t\tshift += 7;\n\t}\n\t*sizep = size;\t\t\t\t<==========\n\treturn offset;\n}\n\n> @@ -1414,6 +1462,49 @@ static int write_buffer(int fd, const vo\n> \treturn 0;\n> }\n> \n> +static int write_binary_header(unsigned char *hdr, enum object_type type, unsigned long len)\n> +{\n> +\tint hdr_len;\n> +\tunsigned char c;\n> +\n> +\tc = (type << 4) | (len & 15);\n> +\tlen >>= 4;\n> +\thdr_len = 1;\n> +\twhile (len) {\n> +\t\t*hdr++ = c;\n> +\t\thdr_len++;\n> +\t\tc = (len & 0x7f);\n> +\t\tlen >>= 7;\n> +\t}\n> +\t*hdr = c | 0x80;\n> +\treturn hdr_len;\n> +}\n> +\n\nDito, but in this case see pack-objects.c\n\n/*\n * The per-object header is a pretty dense thing, which is\n * - first byte: low four bits are \"size\", then three bits of \"type\",\n * and the high bit is \"size continues\".\n * - each byte afterwards: low seven bits are size continuation,\n * with the high bit being \"size continues\"\n */\nstatic int encode_header(enum object_type type, unsigned long size, unsigned char *hdr)\n{\n int n = 1;\n unsigned char c;\n\n if (type < OBJ_COMMIT || type > OBJ_DELTA)\n die(\"bad type %d\", type);\n\n c = (type << 4) | (size & 15);\n size >>= 4;\n while (size) {\n *hdr++ = c | 0x80;\t<=======\n c = size & 0x7f;\n size >>= 7;\n n++;\n }\n *hdr = c;\t\t\t<=======\n return n;\n}\n\n\n\n-Peter Baumann\n"},{"id":"23695","messageId":"7vveq3z41v.fsf@assigned-by-dhcp.cox.net","threadId":"4840","inReplyTo":"slrneb96rd.dma.Peter.B.Baumann@xp.machine.xx","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-12T07:16:12Z","receivedAt":"2006-07-12T07:16:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Baumann \nwrites:\n\n>> +\tbits = 4;\n>> +\tsize = c & 0xf;\n>> +\twhile (!(c & 0x80)) {\n>> +\t\tif (bits >= 8*sizeof(long))\n>> +\t\t\treturn -1;\n>> +\t\tc = *map++;\n>> +\t\tsize += (c & 0x7f) << bits;\n>> +\t\tbits += 7;\n>> +\t\tmapsize--;\n>> +\t}\n>\n> This doesn't match the logic used in unpack_object_header, which is used\n> in the packs:\n> ...\n>> +\tc = (type << 4) | (len & 15);\n>> +\tlen >>= 4;\n>> +\thdr_len = 1;\n>> +\twhile (len) {\n>> +\t\t*hdr++ = c;\n>> +\t\thdr_len++;\n>> +\t\tc = (len & 0x7f);\n>> +\t\tlen >>= 7;\n>> +\t}\n>> +\t*hdr = c | 0x80;\n>> +\treturn hdr_len;\n>> +}\n>> +\n>\n> Dito, but in this case see pack-objects.c\n\nWell, while these are not strictly needed to match, there is no\ngood reason to make them inconsistent. Very well spotted.\n"},{"id":"23697","messageId":"20060712082833.GA6842@xp.machine.xx","threadId":"4840","inReplyTo":"7vveq3z41v.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Peter Baumann","fromEmail":"peter.b.baumann@stud.informatik.uni-erlangen.de","sentAt":"2006-07-12T08:28:33Z","receivedAt":"2006-07-12T08:28:33Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Wed, Jul 12, 2006 at 12:16:12AM -0700, Junio C Hamano wrote:\n> >> +\tbits = 4;\n> >> +\tsize = c & 0xf;\n> >> +\twhile (!(c & 0x80)) {\n> >> +\t\tif (bits >= 8*sizeof(long))\n> >> +\t\t\treturn -1;\n> >> +\t\tc = *map++;\n> >> +\t\tsize += (c & 0x7f) << bits;\n> >> +\t\tbits += 7;\n> >> +\t\tmapsize--;\n> >> +\t}\n> >\n> > This doesn't match the logic used in unpack_object_header, which is used\n> > in the packs:\n> > ...\n> >> +\tc = (type << 4) | (len & 15);\n> >> +\tlen >>= 4;\n> >> +\thdr_len = 1;\n> >> +\twhile (len) {\n> >> +\t\t*hdr++ = c;\n> >> +\t\thdr_len++;\n> >> +\t\tc = (len & 0x7f);\n> >> +\t\tlen >>= 7;\n> >> +\t}\n> >> +\t*hdr = c | 0x80;\n> >> +\treturn hdr_len;\n> >> +}\n> >> +\n> >\n> > Dito, but in this case see pack-objects.c\n> \n> Well, while these are not strictly needed to match, there is no\n> good reason to make them inconsistent. Very well spotted.\n> \n\nDuring packing one could simply copy the existing object into the generated\npack in the non delta case, so I think this _is_ necessary/usefull.\n\n-Peter Baumann\n"},{"id":"23709","messageId":"Pine.LNX.4.64.0607120810320.5623@g5.osdl.org","threadId":"4840","inReplyTo":"slrneb96rd.dma.Peter.B.Baumann@xp.machine.xx","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-12T15:13:22Z","receivedAt":"2006-07-12T15:13:22Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 12 Jul 2006, Peter Baumann wrote:\n> \n> This doesn't match the logic used in unpack_object_header, which is used\n> in the packs:\n\nYeah, good point. I reversed the meaning of the high bit by mistake. In \npack-files, the high bit is a \"there is more to come\" bit, and in my new \ncode it was a \"this is the last byte\" bit.\n\nIt doesn't matter for the ultimate reason for this (being able to re-use \nthe actual _bulk_ of the data), but having it be different would be stupid \nand confusing, and means that we can't later try to use the same routines \nfor packing/unpacking the objects.\n\nJunio - do you want me to send an updated patch, or do you want to reverse \nbit#7 yourself?\n\n\t\tLinus\n"},{"id":"23711","messageId":"7vlkqyzvvr.fsf@assigned-by-dhcp.cox.net","threadId":"4840","inReplyTo":"Pine.LNX.4.64.0607120810320.5623@g5.osdl.org","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-12T15:27:20Z","receivedAt":"2006-07-12T15:27:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Junio - do you want me to send an updated patch, or do you want to reverse \n> bit#7 yourself?\n\nPushed out in \"pu\" already, thanks.\n"},{"id":"23717","messageId":"Pine.LNX.4.64.0607120921380.5623@g5.osdl.org","threadId":"4840","inReplyTo":"7vzmffz5xe.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] sha1_file: add the ability to parse objects in \"pack file format\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-12T16:29:09Z","receivedAt":"2006-07-12T16:29:09Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 11 Jul 2006, Junio C Hamano wrote:\n> \n> You seem to have forgotten one file (fetch-pack.c) but that was\n> trivial. I'll apply and push it out shortly.\n\nActually, it was fetch.c, not fetch-pack.c, and I forgot that due to a \nvery real (and totally separate) bug.\n\nThe makefile doesn't have any dependencies for \"fetch.o\" and \"rsh.o\", so \nwhen you change the headers, they never get rebuilt.\n\nI think fetch.o and rsh.o should either get added to the library files, or \nwe need something like this..\n\n(I didn't check that I caught all the appropriate *.o files, but this \nshould be better than what we have now).\n\nEven better would be to make the dependancies automatic. The kernel does \nthat really well with some GNU Makefile magic, but it also depends on \nmagic gcc command line flags (\"-Wp,-MD,$(depfile)\")\n\n\t\tLinus\n\n---\ndiff --git a/Makefile b/Makefile\nindex e75fb13..854e0af 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -236,6 +236,9 @@ BUILTIN_OBJS = \\\n \tbuiltin-cat-file.o builtin-mailsplit.o builtin-stripspace.o \\\n \tbuiltin-update-ref.o builtin-fmt-merge-msg.o\n \n+MISC_OBJS = \\\n+\tfetch.o rsh.o http-fetch.o http-push.o\n+\n GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\n LIBS = $(GITLIBS) -lz\n \n@@ -615,7 +618,7 @@ git-http-push$X: revision.o http.o http-\n \t$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)\n \n-$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)\n+$(LIB_OBJS) $(BUILTIN_OBJS) $(MISC_OBJS): $(LIB_H)\n $(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)\n $(DIFF_OBJS): diffcore.h\n \n"}]}