git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Justin Tobler <jltobler@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 4/6] builtin/repo: add inflated object info to structure table
Date: Wed, 10 Dec 2025 07:28:34 +0100	[thread overview]
Message-ID: <aTkTEselZ4yL11qd@pks.im> (raw)
In-Reply-To: <20251209225820.2861276-5-jltobler@gmail.com>

On Tue, Dec 09, 2025 at 04:58:18PM -0600, Justin Tobler wrote:
> Update the table output format for the git-repo(1) structure command to
> begin printing the total inflated object size info by object type. To be
> more human-friendly, larger values are scaled down and displayed with
> the appropriate unit prefix. Output for the keyvalue and nul formats
> remains unchanged.
> 
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  builtin/repo.c            | 57 +++++++++++++++++++++++++++++++++--
>  t/t1901-repo-structure.sh | 62 +++++++++++++++++++++++----------------
>  2 files changed, 90 insertions(+), 29 deletions(-)
> 
> diff --git a/builtin/repo.c b/builtin/repo.c
> index a67215ae31..5c37f4116f 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -315,6 +315,44 @@ static void stats_table_count_addf(struct stats_table *table, size_t value,
>  	va_end(ap);
>  }
>  
> +static const char *unit_B = "B";
> +static const char *unit_KiB = "KiB";
> +static const char *unit_MiB = "MiB";
> +static const char *unit_GiB = "GiB";

Okay, nice, you already use KiB et al as I suggested in an earlier
comment. But I guess these should also be marked as translatable.

> +static void stats_table_size_addf(struct stats_table *table, size_t value,
> +				  const char *format, ...)
> +{
> +	struct stats_table_entry *entry;
> +	va_list ap;
> +
> +	CALLOC_ARRAY(entry, 1);
> +
> +	if (value > 1 << 30) {
> +		uintmax_t x = (uintmax_t)value + 5368709;
> +		entry->value = xstrfmt("%" PRIuMAX ".%02" PRIuMAX, x >> 30,
> +				       ((x & ((1 << 30) - 1)) * 100) >> 30);
> +		entry->unit = unit_GiB;
> +	} else if (value > 1 << 20) {
> +		uintmax_t x = (uintmax_t)value + 5243;
> +		entry->value = xstrfmt("%" PRIuMAX ".%02" PRIuMAX, x >> 20,
> +				       ((x & ((1 << 20) - 1)) * 100) >> 20);
> +		entry->unit = unit_MiB;
> +	} else if (value > 1 << 10) {
> +		uintmax_t x = (uintmax_t)value + 5;
> +		entry->value = xstrfmt("%" PRIuMAX ".%02" PRIuMAX, x >> 10,
> +				       ((x & ((1 << 10) - 1)) * 100) >> 10);
> +		entry->unit = unit_KiB;
> +	} else {
> +		entry->value = xstrfmt("%" PRIuMAX, (uintmax_t)value);
> +		entry->unit = unit_B;
> +	}

Euh. What kind of black magic is this? This block at least warrants a
comment how you came up with these incantations.

Also, git-rev-list(1) already has logic to output human-formatted disk
sizes via `git rev-list --disk-usage=human`. Can we share the logic?

> diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh
> index cf5e252f10..0ae96e6bbf 100755
> --- a/t/t1901-repo-structure.sh
> +++ b/t/t1901-repo-structure.sh
> @@ -49,21 +54,26 @@ test_expect_success 'repository with references and objects' '
>  		git notes add -m foo &&
>  
>  		cat >expect <<-\EOF &&
> -		| Repository structure | Value  |
> -		| -------------------- | ------ |
> -		| * References         |        |
> -		|   * Count            |    4   |
> -		|     * Branches       |    1   |
> -		|     * Tags           |    1   |
> -		|     * Remotes        |    1   |
> -		|     * Others         |    1   |
> -		|                      |        |
> -		| * Reachable objects  |        |
> -		|   * Count            | 3.02 k |
> -		|     * Commits        | 1.01 k |
> -		|     * Trees          | 1.01 k |
> -		|     * Blobs          | 1.01 k |
> -		|     * Tags           |    1   |
> +		| Repository structure | Value      |
> +		| -------------------- | ---------- |
> +		| * References         |            |
> +		|   * Count            |      4     |
> +		|     * Branches       |      1     |
> +		|     * Tags           |      1     |
> +		|     * Remotes        |      1     |
> +		|     * Others         |      1     |
> +		|                      |            |
> +		| * Reachable objects  |            |
> +		|   * Count            |   3.02 k   |
> +		|     * Commits        |   1.01 k   |
> +		|     * Trees          |   1.01 k   |
> +		|     * Blobs          |   1.01 k   |
> +		|     * Tags           |      1     |
> +		|   * Inflated size    |  16.03 MiB |
> +		|     * Commits        | 217.92 KiB |
> +		|     * Trees          |  15.81 MiB |
> +		|     * Blobs          |  11.68 KiB |
> +		|     * Tags           |    132 B   |
>  		EOF

Nice, I like the end result.

Patrick

  reply	other threads:[~2025-12-10  6:28 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-09 22:58 [PATCH 0/6] builtin/repo: add object size info to structure output Justin Tobler
2025-12-09 22:58 ` [PATCH 1/6] builtin/repo: group per-type object values into struct Justin Tobler
2025-12-09 22:58 ` [PATCH 2/6] builtin/repo: humanise count values in structure output Justin Tobler
2025-12-10  6:28   ` Patrick Steinhardt
2025-12-10 15:10     ` Justin Tobler
2025-12-11  2:57       ` Junio C Hamano
2025-12-12 16:46         ` Justin Tobler
2025-12-09 22:58 ` [PATCH 3/6] builtin/repo: add inflated object info to keyvalue " Justin Tobler
2025-12-09 22:58 ` [PATCH 4/6] builtin/repo: add inflated object info to structure table Justin Tobler
2025-12-10  6:28   ` Patrick Steinhardt [this message]
2025-12-10 15:21     ` Justin Tobler
2025-12-09 22:58 ` [PATCH 5/6] builtin/repo: add disk size info to keyvalue stucture output Justin Tobler
2025-12-10  6:28   ` Patrick Steinhardt
2025-12-10 15:24     ` Justin Tobler
2025-12-12 20:40     ` Justin Tobler
2025-12-15  5:33       ` Patrick Steinhardt
2025-12-15 16:24         ` Justin Tobler
2025-12-10 14:58   ` Junio C Hamano
2025-12-10 19:09     ` Lucas Seiki Oshiro
2025-12-12 22:36     ` Justin Tobler
2025-12-12 23:58       ` Junio C Hamano
2025-12-09 22:58 ` [PATCH 6/6] builtin/repo: add object disk size info to structure table Justin Tobler
2025-12-10  6:28   ` Patrick Steinhardt
2025-12-10 15:24     ` Justin Tobler
2025-12-12 22:36 ` [PATCH v2 0/7] builtin/repo: add object size info to structure output Justin Tobler
2025-12-12 22:36   ` [PATCH v2 1/7] builtin/repo: group per-type object values into struct Justin Tobler
2025-12-12 22:36   ` [PATCH v2 2/7] strbuf: split out logic to humanise byte values Justin Tobler
2025-12-15  5:33     ` Patrick Steinhardt
2025-12-15 16:26       ` Justin Tobler
2025-12-15  8:21     ` Junio C Hamano
2025-12-15 16:47       ` Justin Tobler
2025-12-16  2:26     ` Jiang Xin
2025-12-16  4:37       ` Junio C Hamano
2025-12-16  6:18         ` Jiang Xin
2025-12-16 14:41           ` Justin Tobler
2025-12-12 22:36   ` [PATCH v2 3/7] builtin/repo: humanise count values in structure output Justin Tobler
2025-12-15  5:33     ` Patrick Steinhardt
2025-12-12 22:36   ` [PATCH v2 4/7] builtin/repo: add inflated object info to keyvalue " Justin Tobler
2025-12-15  5:33     ` Patrick Steinhardt
2025-12-15 16:48       ` Justin Tobler
2025-12-12 22:36   ` [PATCH v2 5/7] builtin/repo: add inflated object info to structure table Justin Tobler
2025-12-12 22:36   ` [PATCH v2 6/7] builtin/repo: add disk size info to keyvalue stucture output Justin Tobler
2025-12-15  5:33     ` Patrick Steinhardt
2025-12-12 22:36   ` [PATCH v2 7/7] builtin/repo: add object disk size info to structure table Justin Tobler
2025-12-15 20:56   ` [PATCH v3 0/7] builtin/repo: add object size info to structure output Justin Tobler
2025-12-15 20:56     ` [PATCH v3 1/7] builtin/repo: group per-type object values into struct Justin Tobler
2025-12-15 20:56     ` [PATCH v3 2/7] strbuf: split out logic to humanise byte values Justin Tobler
2025-12-16  1:19       ` Junio C Hamano
2025-12-16  1:36         ` Justin Tobler
2025-12-15 20:56     ` [PATCH v3 3/7] builtin/repo: humanise count values in structure output Justin Tobler
2025-12-16  8:25       ` Patrick Steinhardt
2025-12-15 20:56     ` [PATCH v3 4/7] builtin/repo: add inflated object info to keyvalue " Justin Tobler
2025-12-15 20:56     ` [PATCH v3 5/7] builtin/repo: add inflated object info to structure table Justin Tobler
2025-12-15 20:56     ` [PATCH v3 6/7] builtin/repo: add disk size info to keyvalue stucture output Justin Tobler
2025-12-15 20:56     ` [PATCH v3 7/7] builtin/repo: add object disk size info to structure table Justin Tobler
2025-12-16  8:25       ` Patrick Steinhardt
2025-12-16 14:48         ` Justin Tobler
2025-12-16 17:38     ` [PATCH v4 0/7] builtin/repo: add object size info to structure output Justin Tobler
2025-12-16 17:38       ` [PATCH v4 1/7] builtin/repo: group per-type object values into struct Justin Tobler
2025-12-16 17:38       ` [PATCH v4 2/7] strbuf: split out logic to humanise byte values Justin Tobler
2025-12-16 18:59         ` Junio C Hamano
2025-12-16 19:39           ` Justin Tobler
2025-12-16 17:38       ` [PATCH v4 3/7] builtin/repo: humanise count values in structure output Justin Tobler
2025-12-16 17:38       ` [PATCH v4 4/7] builtin/repo: add inflated object info to keyvalue " Justin Tobler
2025-12-17  7:03         ` Patrick Steinhardt
2025-12-17 16:10           ` Justin Tobler
2025-12-16 17:38       ` [PATCH v4 5/7] builtin/repo: add inflated object info to structure table Justin Tobler
2025-12-16 17:38       ` [PATCH v4 6/7] builtin/repo: add disk size info to keyvalue stucture output Justin Tobler
2025-12-16 17:38       ` [PATCH v4 7/7] builtin/repo: add object disk size info to structure table Justin Tobler
2025-12-17  7:03       ` [PATCH v4 0/7] builtin/repo: add object size info to structure output Patrick Steinhardt
2025-12-17 17:49         ` Justin Tobler
2025-12-17 17:53       ` [PATCH v5 " Justin Tobler
2025-12-17 17:53         ` [PATCH v5 1/7] builtin/repo: group per-type object values into struct Justin Tobler
2025-12-17 17:53         ` [PATCH v5 2/7] strbuf: split out logic to humanise byte values Justin Tobler
2025-12-17 17:54         ` [PATCH v5 3/7] builtin/repo: humanise count values in structure output Justin Tobler
2025-12-17 17:54         ` [PATCH v5 4/7] builtin/repo: add inflated object info to keyvalue " Justin Tobler
2025-12-17 17:54         ` [PATCH v5 5/7] builtin/repo: add inflated object info to structure table Justin Tobler
2025-12-17 17:54         ` [PATCH v5 6/7] builtin/repo: add disk size info to keyvalue stucture output Justin Tobler
2025-12-17 17:54         ` [PATCH v5 7/7] builtin/repo: add object disk size info to structure table Justin Tobler
2025-12-18  6:32         ` [PATCH v5 0/7] builtin/repo: add object size info to structure output Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aTkTEselZ4yL11qd@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).