{"thread":{"id":"55863","subject":"[suggestion] support non-negative float number in git-repack --max-pack-size","startedAt":"2021-06-07T06:44:06Z","lastAt":"2021-06-18T15:18:17Z","messageCount":9,"participants":["Bagas Sanjaya","Jeff King","Junio C Hamano","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"426559","messageId":"776cb2f9-5fef-4486-5aef-f3ee62fcda7e@gmail.com","threadId":"55863","inReplyTo":null,"subject":"[suggestion] support non-negative float number in git-repack --max-pack-size","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-06-07T06:42:47Z","receivedAt":"2021-06-07T06:44:06Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"Hi,\n\nI would like to create packfiles with charm-numbered size (that is for \nexample use 49.99M instead of 50M) with git-repack:\n\n$ git repack --max-pack-size=49.99M -a -d\n\nBut Git didn't support it:\n\n> error: option `max-pack-size' expects a non-negative integer value with an optional k/m/g suffix\n\nThe workaround was scaling down to kibibytes:\n\n$ git repack --max-pack-size=52418K -a -d\n\nBut the workaround is a rather convoluted to me, because I must convert \nmebibytes (MiB) to kibibytes (KiB). I had to multiply the desired \npackfile size by 1024, as opposed to by 1000 in familiar size notation \n(kilobytes [KB] and megabytes [MB]).\n\nIt would be nice if non-negative floating-point number can be allowed in \n--max-pack-size option, so that many users don't have to scale down size \nnotation like above.\n\nPS: charm numbers are most often used in pricing, because it's almost \nused everywhere (part of psychological pricing).\n\nThanks.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"426717","messageId":"YL8RfGijMY95miHI@coredump.intra.peff.net","threadId":"55863","inReplyTo":"776cb2f9-5fef-4486-5aef-f3ee62fcda7e@gmail.com","subject":"Re: [suggestion] support non-negative float number in git-repack --max-pack-size","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-08T06:43:08Z","receivedAt":"2021-06-08T06:43:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 07, 2021 at 01:42:47PM +0700, Bagas Sanjaya wrote:\n\n> I would like to create packfiles with charm-numbered size (that is for\n> example use 49.99M instead of 50M) with git-repack:\n> \n> $ git repack --max-pack-size=49.99M -a -d\n\nThe parser for numbers with units is shared by many options and config\nvariables. In general, I'm not really opposed to allowing floating point\nvalues which get rounded to the nearest byte for any of them. So no\nobjection for the general feature if somebody wants to implement it.\n\nBut I would note that --max-pack-size is almost never a good idea in\ngeneral. I don't know what you think it's accomplishing, but it is\nprobably making your repository larger than it needs to be, as well as\nless performant.\n\n-Peff\n"},{"id":"426723","messageId":"xmqqfsxsq1l4.fsf@gitster.g","threadId":"55863","inReplyTo":"YL8RfGijMY95miHI@coredump.intra.peff.net","subject":"Re: [suggestion] support non-negative float number in git-repack --max-pack-size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-08T07:04:23Z","receivedAt":"2021-06-08T07:04:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Jun 07, 2021 at 01:42:47PM +0700, Bagas Sanjaya wrote:\n>\n>> I would like to create packfiles with charm-numbered size (that is for\n>> example use 49.99M instead of 50M) with git-repack:\n>> \n>> $ git repack --max-pack-size=49.99M -a -d\n>\n> The parser for numbers with units is shared by many options and config\n> variables. In general, I'm not really opposed to allowing floating point\n> values which get rounded to the nearest byte for any of them. So no\n> objection for the general feature if somebody wants to implement it.\n>\n> But I would note that --max-pack-size is almost never a good idea in\n> general. I don't know what you think it's accomplishing, but it is\n> probably making your repository larger than it needs to be, as well as\n> less performant.\n\nPerhaps a doc update is in order?  It might have been cute and\nsuperficially useful to be able to cut packfiles in 650M chunks to\nfit on a CD-ROM, but I suspect that it would be more useful to feed\na single large file to a generic multi-volume archive tool and let\nit split it to fit the physical volume the tool deals with.\n"},{"id":"426727","messageId":"YL8bQMOAHxApjVVe@coredump.intra.peff.net","threadId":"55863","inReplyTo":"xmqqfsxsq1l4.fsf@gitster.g","subject":"[PATCH] doc: warn people against --max-pack-size","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-08T07:24:48Z","receivedAt":"2021-06-08T07:24:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 08, 2021 at 04:04:23PM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Mon, Jun 07, 2021 at 01:42:47PM +0700, Bagas Sanjaya wrote:\n> >\n> >> I would like to create packfiles with charm-numbered size (that is for\n> >> example use 49.99M instead of 50M) with git-repack:\n> >> \n> >> $ git repack --max-pack-size=49.99M -a -d\n> >\n> > The parser for numbers with units is shared by many options and config\n> > variables. In general, I'm not really opposed to allowing floating point\n> > values which get rounded to the nearest byte for any of them. So no\n> > objection for the general feature if somebody wants to implement it.\n> >\n> > But I would note that --max-pack-size is almost never a good idea in\n> > general. I don't know what you think it's accomplishing, but it is\n> > probably making your repository larger than it needs to be, as well as\n> > less performant.\n> \n> Perhaps a doc update is in order?  It might have been cute and\n> superficially useful to be able to cut packfiles in 650M chunks to\n> fit on a CD-ROM, but I suspect that it would be more useful to feed\n> a single large file to a generic multi-volume archive tool and let\n> it split it to fit the physical volume the tool deals with.\n\nYeah. Let's do that while we're thinking about it. Here's what I came up\nwith.\n\n-- >8 --\nSubject: [PATCH] doc: warn people against --max-pack-size\n\nThis option is almost never a good idea, as the resulting repository is\nlarger and slower (see the new explanations in the docs).\n\nI outlined the potential problems. We could go further and make the\noption harder to find (or at least, make the command-line option\ndescriptions a much more terse \"you probably don't want this; see\npack.packsizeLimit for details\"). But this seems like a minimal change\nthat may prevent people from thinking it's more useful than it is.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/config/pack.txt      | 23 +++++++++++++++++------\n Documentation/git-pack-objects.txt |  6 +++---\n Documentation/git-repack.txt       |  4 +++-\n 3 files changed, 23 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt\nindex c0844d8d8e..763f7af7c4 100644\n--- a/Documentation/config/pack.txt\n+++ b/Documentation/config/pack.txt\n@@ -99,12 +99,23 @@ pack.packSizeLimit::\n \tpacking to a file when repacking, i.e. the git:// protocol\n \tis unaffected.  It can be overridden by the `--max-pack-size`\n \toption of linkgit:git-repack[1].  Reaching this limit results\n-\tin the creation of multiple packfiles; which in turn prevents\n-\tbitmaps from being created.\n-\tThe minimum size allowed is limited to 1 MiB.\n-\tThe default is unlimited.\n-\tCommon unit suffixes of 'k', 'm', or 'g' are\n-\tsupported.\n+\tin the creation of multiple packfiles.\n++\n+Note that this option is rarely useful, and may result in a larger total\n+on-disk size (because Git will not store deltas between packs), as well\n+as worse runtime performance (object lookup within multiple packs is\n+slower than a single pack, and optimizations like reachability bitmaps\n+cannot cope with multiple packs).\n++\n+If you need to actively run Git using smaller packfiles (e.g., because your\n+filesystem does not support large files), this option may help. But if\n+your goal is to transmit a packfile over a medium that supports limited\n+sizes (e.g., removable media that cannot store the whole repository),\n+you are likely better off creating a single large packfile and splitting\n+it using a generic multi-volume archive tool (e.g., Unix `split`).\n++\n+The minimum size allowed is limited to 1 MiB. The default is unlimited.\n+Common unit suffixes of 'k', 'm', or 'g' are supported.\n \n pack.useBitmaps::\n \tWhen true, git will use pack bitmaps (if available) when packing\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 25d9fbe37a..dbfd1f9017 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -128,10 +128,10 @@ depth is 4095.\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-\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+\t`pack.packSizeLimit` is set. Note that this option may result in\n+\ta larger and slower repository; see the discussion in\n+\t`pack.packSizeLimit`.\n \n --honor-pack-keep::\n \tThis flag causes an object already in a local pack that\ndiff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt\nindex ef310f362e..24c00c9384 100644\n--- a/Documentation/git-repack.txt\n+++ b/Documentation/git-repack.txt\n@@ -121,7 +121,9 @@ depth is 4095.\n \tIf specified, multiple packfiles may be created, which also\n \tprevents the creation of a bitmap index.\n \tThe default is unlimited, unless the config variable\n-\t`pack.packSizeLimit` is set.\n+\t`pack.packSizeLimit` is set. Note that this option may result in\n+\ta larger and slower repository; see the discussion in\n+\t`pack.packSizeLimit`.\n \n -b::\n --write-bitmap-index::\n-- \n2.32.0.527.gfd0058899e\n\n"},{"id":"427160","messageId":"c8723ccc-7167-796f-59ed-2cdaadccdc6f@gmail.com","threadId":"55863","inReplyTo":"776cb2f9-5fef-4486-5aef-f3ee62fcda7e@gmail.com","subject":"Re: [suggestion] support non-negative float number in git-repack --max-pack-size","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-06-12T01:20:41Z","receivedAt":"2021-06-12T01:22:00Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"ping\n"},{"id":"427773","messageId":"d73194b2-2fc9-80d3-d13c-fb5db3757e01@iee.email","threadId":"55863","inReplyTo":"YL8bQMOAHxApjVVe@coredump.intra.peff.net","subject":"Re: [PATCH] doc: warn people against --max-pack-size","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-06-17T17:02:27Z","receivedAt":"2021-06-17T17:02:34Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 08/06/2021 08:24, Jeff King wrote:\n> +The minimum size allowed is limited to 1 MiB. The default is unlimited.\n> +Common unit suffixes of 'k', 'm', or 'g' are supported.\n\nDo we want to include the workaround of scaling in kibibytes (as\noriginally mentioned by Bagas) for the default as 1024k? This also\navoids the easy mistake that the size is in multiples MiB.\n\nPhilip\n"},{"id":"427824","messageId":"YMye+mkRPD0BkULX@coredump.intra.peff.net","threadId":"55863","inReplyTo":"d73194b2-2fc9-80d3-d13c-fb5db3757e01@iee.email","subject":"Re: [PATCH] doc: warn people against --max-pack-size","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-18T13:26:18Z","receivedAt":"2021-06-18T13:26:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 17, 2021 at 06:02:27PM +0100, Philip Oakley wrote:\n\n> On 08/06/2021 08:24, Jeff King wrote:\n> > +The minimum size allowed is limited to 1 MiB. The default is unlimited.\n> > +Common unit suffixes of 'k', 'm', or 'g' are supported.\n> \n> Do we want to include the workaround of scaling in kibibytes (as\n> originally mentioned by Bagas) for the default as 1024k? This also\n> avoids the easy mistake that the size is in multiples MiB.\n\nI'm not quite sure what you're asking. If you mean: should we tell\npeople that they can't use \"4.9m\" and should instead use \"5017k\"\ninstead, then I don't have a strong opinion.\n\nIt might help some people. But OTOH it's not clear to me that this is a\ncommon question, so it might clutter up the documentation. Either way,\nit's orthogonal to the patch in question, and should come on top if\nsomebody cares to work on it.\n\nI'd also be fine with somebody actually implementing fractional unit\nsupport (it would probably go into git_parse_signed() and\ngit_parse_unsigned()). It doesn't seem worth the effort to me, but if\nsomebody feels strongly enough to implement it cleanly, I wouldn't say\nno. :)\n\n-Peff\n"},{"id":"427833","messageId":"de7239bc-d68e-c9f5-1c97-87df24cfe7b1@iee.email","threadId":"55863","inReplyTo":"YMye+mkRPD0BkULX@coredump.intra.peff.net","subject":"Re: [PATCH] doc: warn people against --max-pack-size","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-06-18T15:15:23Z","receivedAt":"2021-06-18T15:15:26Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 18/06/2021 14:26, Jeff King wrote:\n> On Thu, Jun 17, 2021 at 06:02:27PM +0100, Philip Oakley wrote:\n>\n>> On 08/06/2021 08:24, Jeff King wrote:\n>>> +The minimum size allowed is limited to 1 MiB. The default is unlimited.\n>>> +Common unit suffixes of 'k', 'm', or 'g' are supported.\n>> Do we want to include the workaround of scaling in kibibytes (as\n>> originally mentioned by Bagas) for the default as 1024k? This also\n>> avoids the easy mistake that the size is in multiples MiB.\n> I'm not quite sure what you're asking. If you mean: should we tell\n> people that they can't use \"4.9m\" and should instead use \"5017k\"\n> instead, then I don't have a strong opinion.\n\nSorry, I should have included an example based on the patch\n\n> +The minimum size allowed is limited to 1 MiB. The default is unlimited.\n> +Common unit suffixes of 'k', 'm', or 'g' are supported.\n\ne.g.\n\n+The default is unlimited. Common unit suffixes of 'k', 'm', or 'g' are\n+supported. The minimum size allowed is limited to 1 MiB (`1024k`). \n\nI swapped the sentence order to allow the scaled example of the minimum\nto be after the explanation of the suffixes\n>\n> It might help some people. But OTOH it's not clear to me that this is a\n> common question, so it might clutter up the documentation. Either way,\n> it's orthogonal to the patch in question, and should come on top if\n> somebody cares to work on it.\n>\n> I'd also be fine with somebody actually implementing fractional unit\n> support (it would probably go into git_parse_signed() and\n> git_parse_unsigned()). It doesn't seem worth the effort to me, but if\n> somebody feels strongly enough to implement it cleanly, I wouldn't say\n> no. :)\nI'd agree about not offering fractional values, but showing the use of\nsmaller units was an easy tweak.\n\nPhilip\n"},{"id":"427834","messageId":"YMy5Nimj7F1r4Yn4@coredump.intra.peff.net","threadId":"55863","inReplyTo":"de7239bc-d68e-c9f5-1c97-87df24cfe7b1@iee.email","subject":"Re: [PATCH] doc: warn people against --max-pack-size","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-18T15:18:14Z","receivedAt":"2021-06-18T15:18:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 18, 2021 at 04:15:23PM +0100, Philip Oakley wrote:\n\n> > I'm not quite sure what you're asking. If you mean: should we tell\n> > people that they can't use \"4.9m\" and should instead use \"5017k\"\n> > instead, then I don't have a strong opinion.\n> \n> Sorry, I should have included an example based on the patch\n> \n> > +The minimum size allowed is limited to 1 MiB. The default is unlimited.\n> > +Common unit suffixes of 'k', 'm', or 'g' are supported.\n> \n> e.g.\n> \n> +The default is unlimited. Common unit suffixes of 'k', 'm', or 'g' are\n> +supported. The minimum size allowed is limited to 1 MiB (`1024k`). \n> \n> I swapped the sentence order to allow the scaled example of the minimum\n> to be after the explanation of the suffixes\n\nAh, I see. I don't have a strong feeling either way if you want to\nprepare a patch.\n\n-Peff\n"}]}