{"thread":{"id":"53201","subject":"[PATCH] pack-format: document missing fields","startedAt":"2020-04-09T19:11:41Z","lastAt":"2020-04-10T01:19:31Z","messageCount":3,"participants":["Alba Mendez via GitGitGadget","Junio C Hamano","Danh Doan"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"395138","messageId":"pull.605.git.1586459494321.gitgitgadget@gmail.com","threadId":"53201","inReplyTo":null,"subject":"[PATCH] pack-format: document missing fields","fromName":"Alba Mendez via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-09T19:11:34Z","receivedAt":"2020-04-09T19:11:41Z","isPatch":true,"sender":{"key":"name:Alba Mendez","avatar":null},"body":"From: Alba Mendez <me@alba.sh>\n\nDocument missing fields in the description of the delta data, added at\n011b648 (pack-format.txt: more details on pack file format, 2018-05-11).\n\nAlso, the description of object entries at the main section is a bit\nvague. There's an equivalent but more detailed description in the index\nsection; since they are redundant, put that one in place of the first.\n\nI have also expanded tabs to prevent alignment issues and rephrased a\nbit to make things clearer.\n\nSigned-off-by: Alba Mendez <me@alba.sh>\n---\n    pack-format: document missing fields\n    \n    Hello! These are some fixes & improvements for the pack-format\n    documentation.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-605%2Fmildsunrise%2Fpatch-1-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-605/mildsunrise/patch-1-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/605\n\n Documentation/technical/pack-format.txt | 75 ++++++++++---------------\n 1 file changed, 31 insertions(+), 44 deletions(-)\n\ndiff --git a/Documentation/technical/pack-format.txt b/Documentation/technical/pack-format.txt\nindex d3a142c6520..b78e03137f4 100644\n--- a/Documentation/technical/pack-format.txt\n+++ b/Documentation/technical/pack-format.txt\n@@ -9,7 +9,7 @@ Git pack format\n          The signature is: {'P', 'A', 'C', 'K'}\n \n      4-byte version number (network byte order):\n-\t Git currently accepts version number 2 or 3 but\n+         Git currently accepts version number 2 or 3 but\n          generates version 2 only.\n \n      4-byte number of objects contained in the pack (network byte order)\n@@ -20,19 +20,34 @@ Git pack format\n    - The header is followed by number of object entries, each of\n      which looks like this:\n \n-     (undeltified representation)\n-     n-byte type and length (3-bit type, (n-1)*7+4-bit length)\n-     compressed data\n-\n-     (deltified representation)\n-     n-byte type and length (3-bit type, (n-1)*7+4-bit length)\n-     20-byte base object name if OBJ_REF_DELTA or a negative relative\n-\t offset from the delta object's position in the pack if this\n-\t is an OBJ_OFS_DELTA object\n-     compressed delta data\n+     packed object header:\n+        1-byte size extension bit (MSB)\n+            type (next 3-bit)\n+            size0 (lower 4-bit)\n+        n-byte sizeN (as long as MSB is set, each 7-bit)\n+            size0..sizeN form 4+7+7+..+7 bit integer, size0\n+            is the least significant part, and sizeN is the\n+            most significant part.\n+     packed object data:\n+        If it is not DELTA, then deflated bytes (the\n+            size above is the size before compression).\n+        If it is REF_DELTA, then\n+            20-byte base object name SHA-1.\n+            deflated delta data (the size above is the\n+                size of this data before compression).\n+        If it is OFS_DELTA, then\n+            n-byte offset (see below) interpreted as a\n+                negative offset from the type-byte of\n+                the header of the ofs-delta entry.\n+            deflated delta data (the size above is the\n+                size of this data before compression).\n \n-     Observation: length of each object is encoded in a variable\n-     length format and is not constrained to 32-bit or anything.\n+     offset encoding:\n+        n bytes with MSB set in all but the last one.\n+        The offset is then the number constructed by\n+        concatenating the lower 7 bit of each byte, and\n+        for n >= 2 adding 2^7 + 2^14 + ... + 2^(7*(n-1))\n+        to the result.\n \n   - The trailer records 20-byte SHA-1 checksum of all of the above.\n \n@@ -67,7 +82,9 @@ Ref-delta can also refer to an object outside the pack (i.e. the\n so-called \"thin pack\"). When stored on disk however, the pack should\n be self contained to avoid cyclic dependency.\n \n-The delta data is a sequence of instructions to reconstruct an object\n+The deflated delta data begins with two n-byte sizes: the size of\n+the base object data, and the size of the reconstructed object data.\n+What follows is a sequence of instructions to reconstruct the object\n from the base object. If the base object is deltified, it must be\n converted to canonical form first. Each instruction appends more and\n more data to the target object until it's complete. There are two\n@@ -186,36 +203,6 @@ trailer\t  | | packfile checksum              |\n                   |\n Pack file entry: <+\n \n-     packed object header:\n-\t1-byte size extension bit (MSB)\n-\t       type (next 3 bit)\n-\t       size0 (lower 4-bit)\n-        n-byte sizeN (as long as MSB is set, each 7-bit)\n-\t\tsize0..sizeN form 4+7+7+..+7 bit integer, size0\n-\t\tis the least significant part, and sizeN is the\n-\t\tmost significant part.\n-     packed object data:\n-        If it is not DELTA, then deflated bytes (the size above\n-\t\tis the size before compression).\n-\tIf it is REF_DELTA, then\n-\t  20-byte base object name SHA-1 (the size above is the\n-\t\tsize of the delta data that follows).\n-          delta data, deflated.\n-\tIf it is OFS_DELTA, then\n-\t  n-byte offset (see below) interpreted as a negative\n-\t\toffset from the type-byte of the header of the\n-\t\tofs-delta entry (the size above is the size of\n-\t\tthe delta data that follows).\n-\t  delta data, deflated.\n-\n-     offset encoding:\n-\t  n bytes with MSB set in all but the last one.\n-\t  The offset is then the number constructed by\n-\t  concatenating the lower 7 bit of each byte, and\n-\t  for n >= 2 adding 2^7 + 2^14 + ... + 2^(7*(n-1))\n-\t  to the result.\n-\n-\n \n == Version 2 pack-*.idx files support packs larger than 4 GiB, and\n    have some other reorganizations.  They have the format:\n\nbase-commit: 9fadedd637b312089337d73c3ed8447e9f0aa775\n-- \ngitgitgadget\n"},{"id":"395144","messageId":"xmqqlfn4mjm3.fsf@gitster.c.googlers.com","threadId":"53201","inReplyTo":"pull.605.git.1586459494321.gitgitgadget@gmail.com","subject":"Re: [PATCH] pack-format: document missing fields","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-09T21:31:32Z","receivedAt":"2020-04-09T21:31:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alba Mendez via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>       4-byte version number (network byte order):\n> -\t Git currently accepts version number 2 or 3 but\n> +         Git currently accepts version number 2 or 3 but\n>           generates version 2 only.\n\nHmph, what is this hunk about?\n\n> -     (undeltified representation)\n> -     n-byte type and length (3-bit type, (n-1)*7+4-bit length)\n> -     compressed data\n> -\n> -     (deltified representation)\n> -     n-byte type and length (3-bit type, (n-1)*7+4-bit length)\n> -     20-byte base object name if OBJ_REF_DELTA or a negative relative\n> -\t offset from the delta object's position in the pack if this\n> -\t is an OBJ_OFS_DELTA object\n> -     compressed delta data\n\n\n> +     packed object header:\n> +        1-byte size extension bit (MSB)\n> +            type (next 3-bit)\n> +            size0 (lower 4-bit)\n> +        n-byte sizeN (as long as MSB is set, each 7-bit)\n> +            size0..sizeN form 4+7+7+..+7 bit integer, size0\n> +            is the least significant part, and sizeN is the\n> +            most significant part.\n> +     packed object data:\n> +        If it is not DELTA, then deflated bytes (the\n> +            size above is the size before compression).\n\nCorrect.\n\n> +        If it is REF_DELTA, then\n> +            20-byte base object name SHA-1.\n> +            deflated delta data (the size above is the\n> +                size of this data before compression).\n\nCorrect.\n\n> +        If it is OFS_DELTA, then\n> +            n-byte offset (see below) interpreted as a\n> +                negative offset from the type-byte of\n> +                the header of the ofs-delta entry.\n\nCorrect, and \"see below\" here is very important.\n\n> +            deflated delta data (the size above is the\n> +                size of this data before compression).\n\n> +     offset encoding:\n> +        n bytes with MSB set in all but the last one.\n> +        The offset is then the number constructed by\n> +        concatenating the lower 7 bit of each byte, and\n> +        for n >= 2 adding 2^7 + 2^14 + ... + 2^(7*(n-1))\n> +        to the result.\n\nCorrect.  Shouldn't we have a similar explanation for the object\nlength varint above (in other words, how 4+7+7+..+7 bits are\ncombined to form an integer)?\n\n> @@ -67,7 +82,9 @@ Ref-delta can also refer to an object outside the pack (i.e. the\n>  so-called \"thin pack\"). When stored on disk however, the pack should\n>  be self contained to avoid cyclic dependency.\n>  \n> -The delta data is a sequence of instructions to reconstruct an object\n> +The deflated delta data begins with two n-byte sizes: the size of\n> +the base object data, and the size of the reconstructed object data.\n\nSimilarly, not just \"n-byte size\", but clarify that this is a pair\nof \"varint\" and specify which kind of \"varint\" it is (IIRC, this\nencoding did not use the \"save-one-bit\" trick the offset encoding\nuses).\n\n> @@ -186,36 +203,6 @@ trailer\t  | | packfile checksum              |\n>                    |\n>  Pack file entry: <+\n>  \n> -     packed object header:\n> -\t1-byte size extension bit (MSB)\n> -\t       type (next 3 bit)\n> -\t       size0 (lower 4-bit)\n> -        n-byte sizeN (as long as MSB is set, each 7-bit)\n> -\t\tsize0..sizeN form 4+7+7+..+7 bit integer, size0\n> -\t\tis the least significant part, and sizeN is the\n> -\t\tmost significant part.\n> -     packed object data:\n> -        If it is not DELTA, then deflated bytes (the size above\n> -\t\tis the size before compression).\n> -\tIf it is REF_DELTA, then\n> -\t  20-byte base object name SHA-1 (the size above is the\n> -\t\tsize of the delta data that follows).\n> -          delta data, deflated.\n> -\tIf it is OFS_DELTA, then\n> -\t  n-byte offset (see below) interpreted as a negative\n> -\t\toffset from the type-byte of the header of the\n> -\t\tofs-delta entry (the size above is the size of\n> -\t\tthe delta data that follows).\n> -\t  delta data, deflated.\n> -\n> -     offset encoding:\n> -\t  n bytes with MSB set in all but the last one.\n> -\t  The offset is then the number constructed by\n> -\t  concatenating the lower 7 bit of each byte, and\n> -\t  for n >= 2 adding 2^7 + 2^14 + ... + 2^(7*(n-1))\n> -\t  to the result.\n> -\n> -\n\nHmph, I kept saying \"Correct\" above, but it seems that the\ncorrectness come from the original.  So, why is this patch so big?\n\nAs far as I can tell, the only thing it improved (content-wise) was\nto say that the delta data has two varints that lets reader to\nvalidate the length of the original contents that the delta applies\nto, and to allocate the buffer to hold the result before applying\nthe delta.  Puzzled.\n\nThanks.\n\n"},{"id":"395163","messageId":"20200410011925.GB2462@danh.dev","threadId":"53201","inReplyTo":"xmqqlfn4mjm3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] pack-format: document missing fields","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-10T01:19:25Z","receivedAt":"2020-04-10T01:19:31Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-09 14:31:32-0700, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Alba Mendez via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n> >       4-byte version number (network byte order):\n> > -\t Git currently accepts version number 2 or 3 but\n> > +         Git currently accepts version number 2 or 3 but\n> >           generates version 2 only.\n> \n> Hmph, what is this hunk about?\n\nThe original line is \"<TAB><SPACE>Git\", she changed to 9 <SPACE>\nto match with other lines.\nI guess she set tabstop=4, this paragraph will be mis-aligned on that.\n\n> > -     (undeltified representation)\n> > -     n-byte type and length (3-bit type, (n-1)*7+4-bit length)\n> > -     compressed data\n> > -\n> > -     (deltified representation)\n> > -     n-byte type and length (3-bit type, (n-1)*7+4-bit length)\n> > -     20-byte base object name if OBJ_REF_DELTA or a negative relative\n> > -\t offset from the delta object's position in the pack if this\n> > -\t is an OBJ_OFS_DELTA object\n\nAlba, if you gonna change the above <TAB> to 8 <SPACE> above, you may\nwant to change here, too.\nThis paragraph was written with tabstop=4 in mind.\nAnd it's in the context of below hunk.\n\nI'm not sure if changing <TAB> to 8 <SPACE> was expected in this\nfile because <TAB> and <SPACE> is also mixed in \"Pack file entry\"\nsection.\n\n-- \nDanh\n"}]}