All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.