From: Junio C Hamano <hidden> Date: 2021-06-08 07:04:27
Jeff King [off-list ref] writes:
On Mon, Jun 07, 2021 at 01:42:47PM +0700, Bagas Sanjaya wrote:
quoted
I would like to create packfiles with charm-numbered size (that is for
example use 49.99M instead of 50M) with git-repack:
$ git repack --max-pack-size=49.99M -a -d
The parser for numbers with units is shared by many options and config
variables. In general, I'm not really opposed to allowing floating point
values which get rounded to the nearest byte for any of them. So no
objection for the general feature if somebody wants to implement it.
But I would note that --max-pack-size is almost never a good idea in
general. I don't know what you think it's accomplishing, but it is
probably making your repository larger than it needs to be, as well as
less performant.
Perhaps a doc update is in order? It might have been cute and
superficially useful to be able to cut packfiles in 650M chunks to
fit on a CD-ROM, but I suspect that it would be more useful to feed
a single large file to a generic multi-volume archive tool and let
it split it to fit the physical volume the tool deals with.
From: Jeff King <hidden> Date: 2021-06-08 07:24:51
On Tue, Jun 08, 2021 at 04:04:23PM +0900, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
On Mon, Jun 07, 2021 at 01:42:47PM +0700, Bagas Sanjaya wrote:
quoted
I would like to create packfiles with charm-numbered size (that is for
example use 49.99M instead of 50M) with git-repack:
$ git repack --max-pack-size=49.99M -a -d
The parser for numbers with units is shared by many options and config
variables. In general, I'm not really opposed to allowing floating point
values which get rounded to the nearest byte for any of them. So no
objection for the general feature if somebody wants to implement it.
But I would note that --max-pack-size is almost never a good idea in
general. I don't know what you think it's accomplishing, but it is
probably making your repository larger than it needs to be, as well as
less performant.
Perhaps a doc update is in order? It might have been cute and
superficially useful to be able to cut packfiles in 650M chunks to
fit on a CD-ROM, but I suspect that it would be more useful to feed
a single large file to a generic multi-volume archive tool and let
it split it to fit the physical volume the tool deals with.
Yeah. Let's do that while we're thinking about it. Here's what I came up
with.
-- >8 --
Subject: [PATCH] doc: warn people against --max-pack-size
This option is almost never a good idea, as the resulting repository is
larger and slower (see the new explanations in the docs).
I outlined the potential problems. We could go further and make the
option harder to find (or at least, make the command-line option
descriptions a much more terse "you probably don't want this; see
pack.packsizeLimit for details"). But this seems like a minimal change
that may prevent people from thinking it's more useful than it is.
Signed-off-by: Jeff King <redacted>
---
Documentation/config/pack.txt | 23 +++++++++++++++++------
Documentation/git-pack-objects.txt | 6 +++---
Documentation/git-repack.txt | 4 +++-
3 files changed, 23 insertions(+), 10 deletions(-)
@@ -99,12 +99,23 @@ pack.packSizeLimit:: packing to a file when repacking, i.e. the git:// protocol is unaffected. It can be overridden by the `--max-pack-size` option of linkgit:git-repack[1]. Reaching this limit results- in the creation of multiple packfiles; which in turn prevents- bitmaps from being created.- The minimum size allowed is limited to 1 MiB.- The default is unlimited.- Common unit suffixes of 'k', 'm', or 'g' are- supported.+ in the creation of multiple packfiles.+++Note that this option is rarely useful, and may result in a larger total+on-disk size (because Git will not store deltas between packs), as well+as worse runtime performance (object lookup within multiple packs is+slower than a single pack, and optimizations like reachability bitmaps+cannot cope with multiple packs).+++If you need to actively run Git using smaller packfiles (e.g., because your+filesystem does not support large files), this option may help. But if+your goal is to transmit a packfile over a medium that supports limited+sizes (e.g., removable media that cannot store the whole repository),+you are likely better off creating a single large packfile and splitting+it using a generic multi-volume archive tool (e.g., Unix `split`).+++The minimum size allowed is limited to 1 MiB. The default is unlimited.+Common unit suffixes of 'k', 'm', or 'g' are supported. pack.useBitmaps:: When true, git will use pack bitmaps (if available) when packing
@@ -128,10 +128,10 @@ depth is 4095. into multiple independent packfiles, each not larger than the given size. The size can be suffixed with "k", "m", or "g". The minimum size allowed is limited to 1 MiB.- This option- prevents the creation of a bitmap index. The default is unlimited, unless the config variable- `pack.packSizeLimit` is set.+ `pack.packSizeLimit` is set. Note that this option may result in+ a larger and slower repository; see the discussion in+ `pack.packSizeLimit`. --honor-pack-keep:: This flag causes an object already in a local pack that
@@ -121,7 +121,9 @@ depth is 4095. If specified, multiple packfiles may be created, which also prevents the creation of a bitmap index. The default is unlimited, unless the config variable- `pack.packSizeLimit` is set.+ `pack.packSizeLimit` is set. Note that this option may result in+ a larger and slower repository; see the discussion in+ `pack.packSizeLimit`. -b:: --write-bitmap-index::
From: Philip Oakley <hidden> Date: 2021-06-17 17:02:34
On 08/06/2021 08:24, Jeff King wrote:
+The minimum size allowed is limited to 1 MiB. The default is unlimited.
+Common unit suffixes of 'k', 'm', or 'g' are supported.
Do we want to include the workaround of scaling in kibibytes (as
originally mentioned by Bagas) for the default as 1024k? This also
avoids the easy mistake that the size is in multiples MiB.
Philip
From: Jeff King <hidden> Date: 2021-06-18 13:26:20
On Thu, Jun 17, 2021 at 06:02:27PM +0100, Philip Oakley wrote:
On 08/06/2021 08:24, Jeff King wrote:
quoted
+The minimum size allowed is limited to 1 MiB. The default is unlimited.
+Common unit suffixes of 'k', 'm', or 'g' are supported.
Do we want to include the workaround of scaling in kibibytes (as
originally mentioned by Bagas) for the default as 1024k? This also
avoids the easy mistake that the size is in multiples MiB.
I'm not quite sure what you're asking. If you mean: should we tell
people that they can't use "4.9m" and should instead use "5017k"
instead, then I don't have a strong opinion.
It might help some people. But OTOH it's not clear to me that this is a
common question, so it might clutter up the documentation. Either way,
it's orthogonal to the patch in question, and should come on top if
somebody cares to work on it.
I'd also be fine with somebody actually implementing fractional unit
support (it would probably go into git_parse_signed() and
git_parse_unsigned()). It doesn't seem worth the effort to me, but if
somebody feels strongly enough to implement it cleanly, I wouldn't say
no. :)
-Peff
From: Philip Oakley <hidden> Date: 2021-06-18 15:15:26
On 18/06/2021 14:26, Jeff King wrote:
On Thu, Jun 17, 2021 at 06:02:27PM +0100, Philip Oakley wrote:
quoted
On 08/06/2021 08:24, Jeff King wrote:
quoted
+The minimum size allowed is limited to 1 MiB. The default is unlimited.
+Common unit suffixes of 'k', 'm', or 'g' are supported.
Do we want to include the workaround of scaling in kibibytes (as
originally mentioned by Bagas) for the default as 1024k? This also
avoids the easy mistake that the size is in multiples MiB.
I'm not quite sure what you're asking. If you mean: should we tell
people that they can't use "4.9m" and should instead use "5017k"
instead, then I don't have a strong opinion.
Sorry, I should have included an example based on the patch
+The minimum size allowed is limited to 1 MiB. The default is unlimited.
+Common unit suffixes of 'k', 'm', or 'g' are supported.
e.g.
+The default is unlimited. Common unit suffixes of 'k', 'm', or 'g' are
+supported. The minimum size allowed is limited to 1 MiB (`1024k`).
I swapped the sentence order to allow the scaled example of the minimum
to be after the explanation of the suffixes
It might help some people. But OTOH it's not clear to me that this is a
common question, so it might clutter up the documentation. Either way,
it's orthogonal to the patch in question, and should come on top if
somebody cares to work on it.
I'd also be fine with somebody actually implementing fractional unit
support (it would probably go into git_parse_signed() and
git_parse_unsigned()). It doesn't seem worth the effort to me, but if
somebody feels strongly enough to implement it cleanly, I wouldn't say
no. :)
I'd agree about not offering fractional values, but showing the use of
smaller units was an easy tweak.
Philip
From: Jeff King <hidden> Date: 2021-06-18 15:18:17
On Fri, Jun 18, 2021 at 04:15:23PM +0100, Philip Oakley wrote:
quoted
I'm not quite sure what you're asking. If you mean: should we tell
people that they can't use "4.9m" and should instead use "5017k"
instead, then I don't have a strong opinion.
Sorry, I should have included an example based on the patch
quoted
+The minimum size allowed is limited to 1 MiB. The default is unlimited.
+Common unit suffixes of 'k', 'm', or 'g' are supported.
e.g.
+The default is unlimited. Common unit suffixes of 'k', 'm', or 'g' are
+supported. The minimum size allowed is limited to 1 MiB (`1024k`).
I swapped the sentence order to allow the scaled example of the minimum
to be after the explanation of the suffixes
Ah, I see. I don't have a strong feeling either way if you want to
prepare a patch.
-Peff