{"thread":{"id":"46649","subject":"[PATCH] Doc: clarify that pack-objects makes packs, plural","startedAt":"2017-08-22T18:22:53Z","lastAt":"2017-08-24T19:09:09Z","messageCount":7,"participants":["Jonathan Tan","Junio C Hamano","Jeff King","Jacob Keller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"326952","messageId":"20170822182242.20862-1-jonathantanmy@google.com","threadId":"46649","inReplyTo":null,"subject":"[PATCH] Doc: clarify that pack-objects makes packs, plural","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-08-22T18:22:42Z","receivedAt":"2017-08-22T18:22:53Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"The documentation for pack-objects describes that it creates \"a packed\narchive of objects\", which is confusing because it may create multiple\npacks if --max-pack-size is set. Update the documentation to clarify\nthis.\n\nSigned-off-by: Jonathan Tan \n---\nIt took me quite some time before I realized that pack-objects actually\nmay write multiple packs, the opening lines of the doc confusing me.\nHere's a doc update.\n---\n Documentation/git-pack-objects.txt | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 8973510a4..d8264ad57 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -3,7 +3,7 @@ git-pack-objects(1)\n \n NAME\n ----\n-git-pack-objects - Create a packed archive of objects\n+git-pack-objects - Create packed archives of objects\n \n \n SYNOPSIS\n@@ -18,8 +18,9 @@ SYNOPSIS\n \n DESCRIPTION\n -----------\n-Reads list of objects from the standard input, and writes a packed\n-archive with specified base-name, or to the standard output.\n+Reads list of objects from the standard input, and writes either one or\n+more packed archives with the specified base-name to disk, or a packed\n+archive to the standard output.\n \n A packed archive is an efficient way to transfer a set of objects\n between two repositories as well as an access efficient archival\n@@ -47,9 +48,9 @@ transport by their peers.\n OPTIONS\n -------\n base-name::\n-\tWrite into a pair of files (.pack and .idx), using\n+\tWrite into pairs of files (.pack and .idx), using\n \t to determine the name of the created file.\n-\tWhen this option is used, the two files are written in\n+\tWhen this option is used, the two files in a pair are written in\n \t-.{pack,idx} files. is a hash\n \tbased on the pack content and is written to the standard\n \toutput of the command.\n-- \n2.14.1.342.g6490525c54-goog\n\n"},{"id":"326961","messageId":"xmqqbmn72x7a.fsf@gitster.mtv.corp.google.com","threadId":"46649","inReplyTo":"20170822182242.20862-1-jonathantanmy@google.com","subject":"Re: [PATCH] Doc: clarify that pack-objects makes packs, plural","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-22T19:56:25Z","receivedAt":"2017-08-22T19:56:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> The documentation for pack-objects describes that it creates \"a packed\n> archive of objects\", which is confusing because it may create multiple\n> packs if --max-pack-size is set. Update the documentation to clarify\n> this.\n>\n> Signed-off-by: Jonathan Tan \n> ---\n> It took me quite some time before I realized that pack-objects actually\n> may write multiple packs, the opening lines of the doc confusing me.\n> Here's a doc update.\n> ---\n\nI have a mixed feeling about this one. \n\nYes, the command _can_ be told to split a packfile into multiple\nwith an option, but the actual benefit of doing so is rather\ndubious. On boxes with smaller address space, I thought windowed\nmmap access into large packfiles work just fine. I also think the\nmotivation behind the \"max-size\" thing was to split into smaller\npieces so that sneaker-netting on multiple CD-ROMs becomes easier or\nsomething (silly) like that---there should be a more suitable tool\nthat is not specific to Git for such usecase, I would imagine.\n\nSo I am OK with \"and writes either one or more\" in the description,\nbut I'd prefer to see that \"--max-pack-size\" thing gets described as\nan aberration, not a norm.\n\nIOW,\n\n - I think the \"NAME\" part that gives a single line summary of what\n the command is about can and should stay as before. A single\n archive is the norm, and we do not particularly recommend people\n to think it is a good idea to produce multiple packfiles.\n\n - The change in this patch for description part, which should give\n a fairly complete view of what it can do, is good.\n\n - The change for 'base-name' documentation that stresses that\n .pack/.idx come in pairs and share the same is good.\n\n - There should be an update to say max-pack-size is not something\n normal users would ever want.\n\nFor the last one, perhaps something like this:\n\n\n Documentation/git-pack-objects.txt | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 8973510a41..3aa6234501 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -108,9 +108,13 @@ base-name::\n \tis taken from the `pack.windowMemory` configuration variable.\n \n --max-pack-size=::\n-\tMaximum size of each output pack file. The size can be suffixed with\n+\tIn unusual scenarios, you may not be able to create files\n+\tlarger than certain size on your filesystem, and this option\n+\tcan be used to tell the command to split the output packfile\n+\tinto multiple independent packfiles and what the maximum\n+\tsize of each packfile is. The size can be suffixed with\n \t\"k\", \"m\", or \"g\". The minimum size allowed is limited to 1 MiB.\n-\tIf specified, multiple packfiles may be created, which also\n+\tThis option\n \tprevents the creation of a bitmap index.\n \tThe default is unlimited, unless the config variable\n \t`pack.packSizeLimit` is set.\n"},{"id":"327006","messageId":"20170823004010.3077-1-jonathantanmy@google.com","threadId":"46649","inReplyTo":"xmqqbmn72x7a.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2] Doc: clarify that pack-objects makes packs, plural","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-08-23T00:40:10Z","receivedAt":"2017-08-23T00:40:29Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"The documentation for pack-objects describes that it creates \"a packed\narchive of objects\", which is confusing because it may create multiple\npacks if --max-pack-size is set. Update the documentation to clarify\nthis, and explaining in which cases such a feature would be useful.\n\nSigned-off-by: Jonathan Tan \n---\nThanks. I've reverted the NAME change and used some of your suggestion\nfor the --max-pack-size documentation.\n---\n Documentation/git-pack-objects.txt | 17 +++++++++++------\n 1 file changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 8973510a4..473a16135 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -18,8 +18,9 @@ SYNOPSIS\n \n DESCRIPTION\n -----------\n-Reads list of objects from the standard input, and writes a packed\n-archive with specified base-name, or to the standard output.\n+Reads list of objects from the standard input, and writes either one or\n+more packed archives with the specified base-name to disk, or a packed\n+archive to the standard output.\n \n A packed archive is an efficient way to transfer a set of objects\n between two repositories as well as an access efficient archival\n@@ -47,9 +48,9 @@ transport by their peers.\n OPTIONS\n -------\n base-name::\n-\tWrite into a pair of files (.pack and .idx), using\n+\tWrite into pairs of files (.pack and .idx), using\n \t to determine the name of the created file.\n-\tWhen this option is used, the two files are written in\n+\tWhen this option is used, the two files in a pair are written in\n \t-.{pack,idx} files. is a hash\n \tbased on the pack content and is written to the standard\n \toutput of the command.\n@@ -108,9 +109,13 @@ base-name::\n \tis taken from the `pack.windowMemory` configuration variable.\n \n --max-pack-size=::\n-\tMaximum size of each output pack file. The size can be suffixed with\n+\tIn unusual scenarios, you may not be able to create files\n+\tlarger than a certain size on your filesystem, and this option\n+\tcan be used to tell the command to split the output packfile\n+\tinto multiple independent packfiles, each not larger than the\n+\tgiven size. The size can be suffixed with\n \t\"k\", \"m\", or \"g\". The minimum size allowed is limited to 1 MiB.\n-\tIf specified, multiple packfiles may be created, which also\n+\tThis option\n \tprevents the creation of a bitmap index.\n \tThe default is unlimited, unless the config variable\n \t`pack.packSizeLimit` is set.\n-- \n2.14.1.342.g6490525c54-goog\n\n"},{"id":"327091","messageId":"20170823212251.li5fs4kypeveydbm@sigill.intra.peff.net","threadId":"46649","inReplyTo":"xmqqbmn72x7a.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Doc: clarify that pack-objects makes packs, plural","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-23T21:22:51Z","receivedAt":"2017-08-23T21:29:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 22, 2017 at 12:56:25PM -0700, Junio C Hamano wrote:\n\n> - There should be an update to say max-pack-size is not something\n> normal users would ever want.\n\nAgreed.\n\n> diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\n> index 8973510a41..3aa6234501 100644\n> --- a/Documentation/git-pack-objects.txt\n> +++ b/Documentation/git-pack-objects.txt\n> @@ -108,9 +108,13 @@ base-name::\n> \tis taken from the `pack.windowMemory` configuration variable.\n> \n> --max-pack-size=::\n> -\tMaximum size of each output pack file. The size can be suffixed with\n> +\tIn unusual scenarios, you may not be able to create files\n> +\tlarger than certain size on your filesystem, and this option\n> +\tcan be used to tell the command to split the output packfile\n> +\tinto multiple independent packfiles and what the maximum\n> +\tsize of each packfile is. The size can be suffixed with\n> \t\"k\", \"m\", or \"g\". The minimum size allowed is limited to 1 MiB.\n> -\tIf specified, multiple packfiles may be created, which also\n> +\tThis option\n> \tprevents the creation of a bitmap index.\n> \tThe default is unlimited, unless the config variable\n> \t`pack.packSizeLimit` is set.\n\nI wonder if it is worth mentioning the other downside: that the sum of\nthe split packfiles may be substantially larger than a single packfile\nwould be (due to lost delta opportunities between the split packs).\n\nFor the sneaker-net case, you are much better off generating a single\npack and then using \"split\" and \"cat\" to reconstruct it on the other end\nNot that I think we should go into such detail in the manpage, but I\nhave to wonder if --max-pack-size has outlived its usefulness. The only\nuse case I can think of is a filesystem that cannot hold files larger\nthan N bytes.\n\n-Peff\n"},{"id":"327111","messageId":"CA+P7+xoPscnzCGLHSK-gk80GcZLZ5T4e7E29ztVX7qbjMcGbnw@mail.gmail.com","threadId":"46649","inReplyTo":"20170823212251.li5fs4kypeveydbm@sigill.intra.peff.net","subject":"Re: [PATCH] Doc: clarify that pack-objects makes packs, plural","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-08-24T07:27:52Z","receivedAt":"2017-08-24T07:28:18Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Aug 23, 2017 at 2:22 PM, Jeff King wrote:\n> On Tue, Aug 22, 2017 at 12:56:25PM -0700, Junio C Hamano wrote:\n>\n>> - There should be an update to say max-pack-size is not something\n>> normal users would ever want.\n>\n> Agreed.\n>\n>> diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\n>> index 8973510a41..3aa6234501 100644\n>> --- a/Documentation/git-pack-objects.txt\n>> +++ b/Documentation/git-pack-objects.txt\n>> @@ -108,9 +108,13 @@ base-name::\n>> is taken from the `pack.windowMemory` configuration variable.\n>>\n>> --max-pack-size=::\n>> - Maximum size of each output pack file. The size can be suffixed with\n>> + In unusual scenarios, you may not be able to create files\n>> + larger than certain size on your filesystem, and this option\n>> + can be used to tell the command to split the output packfile\n>> + into multiple independent packfiles and what the maximum\n>> + size of each packfile is. The size can be suffixed with\n>> \"k\", \"m\", or \"g\". The minimum size allowed is limited to 1 MiB.\n>> - If specified, multiple packfiles may be created, which also\n>> + This option\n>> prevents the creation of a bitmap index.\n>> The default is unlimited, unless the config variable\n>> `pack.packSizeLimit` is set.\n>\n> I wonder if it is worth mentioning the other downside: that the sum of\n> the split packfiles may be substantially larger than a single packfile\n> would be (due to lost delta opportunities between the split packs).\n>\n> For the sneaker-net case, you are much better off generating a single\n> pack and then using \"split\" and \"cat\" to reconstruct it on the other end\n> Not that I think we should go into such detail in the manpage, but I\n> have to wonder if --max-pack-size has outlived its usefulness. The only\n> use case I can think of is a filesystem that cannot hold files larger\n> than N bytes.\n>\n> -Peff\n\nIs it possible to detect on the file system that we can't store files\nthat large, and remove the option, while enabling it only when we\ndetect the filesystem is unable to store large files?\n\nThanks,\nJake\n"},{"id":"327126","messageId":"20170824133842.vqzydienll6ka4vw@sigill.intra.peff.net","threadId":"46649","inReplyTo":"CA+P7+xoPscnzCGLHSK-gk80GcZLZ5T4e7E29ztVX7qbjMcGbnw@mail.gmail.com","subject":"Re: [PATCH] Doc: clarify that pack-objects makes packs, plural","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-24T13:38:43Z","receivedAt":"2017-08-24T13:38:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 24, 2017 at 12:27:52AM -0700, Jacob Keller wrote:\n\n> > For the sneaker-net case, you are much better off generating a single\n> > pack and then using \"split\" and \"cat\" to reconstruct it on the other end\n> > Not that I think we should go into such detail in the manpage, but I\n> > have to wonder if --max-pack-size has outlived its usefulness. The only\n> > use case I can think of is a filesystem that cannot hold files larger\n> > than N bytes.\n> \n> Is it possible to detect on the file system that we can't store files\n> that large, and remove the option, while enabling it only when we\n> detect the filesystem is unable to store large files?\n\nI'm not sure how easy it would be to do such a check. But even if it\nwas, I'm not sure that buys us much. We'd still carry the code. We could\nin theory remove the option, simplifying the interface. But that removes\nthe possibility of somebody wanting to stage the smaller packfiles on a\nmore capable filesystem in preparation for moving them to the\nmore-limited one.\n\n-Peff\n"},{"id":"327146","messageId":"xmqqd17kwzox.fsf@gitster.mtv.corp.google.com","threadId":"46649","inReplyTo":"20170824133842.vqzydienll6ka4vw@sigill.intra.peff.net","subject":"Re: [PATCH] Doc: clarify that pack-objects makes packs, plural","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-24T19:09:02Z","receivedAt":"2017-08-24T19:09:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Aug 24, 2017 at 12:27:52AM -0700, Jacob Keller wrote:\n>\n>> > For the sneaker-net case, you are much better off generating a single\n>> > pack and then using \"split\" and \"cat\" to reconstruct it on the other end\n>> > Not that I think we should go into such detail in the manpage, but I\n>> > have to wonder if --max-pack-size has outlived its usefulness. The only\n>> > use case I can think of is a filesystem that cannot hold files larger\n>> > than N bytes.\n>> \n>> Is it possible to detect on the file system that we can't store files\n>> that large, and remove the option, while enabling it only when we\n>> detect the filesystem is unable to store large files?\n>\n> I'm not sure how easy it would be to do such a check. But even if it\n> was, I'm not sure that buys us much. We'd still carry the code. We could\n> in theory remove the option, simplifying the interface. But that removes\n> the possibility of somebody wanting to stage the smaller packfiles on a\n> more capable filesystem in preparation for moving them to the\n> more-limited one.\n\nI agree that it would not help anybody to _disable_ --max-pack-size\nfeature for those on certain filesystems, but it _might_ make sense\nto automatically _enable_ it (and set it to the maximum number) when\nyour destination filesystem is limited.\n\nEven in that case, failing with an error code from the filesystem\nand then asking the user to redo with --max-pack-size specified\nwouldn't be the end of the world, though.\n"}]}