Re: [PATCH 3/5] scalar: teach `diagnose` to gather packfile info
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Jan 26, 2022, 22:43 UTC
- Message-ID
- <YfHOo8Mf3RP4j0Y6@nand.local>
- In-Reply-To
- <330b36de799f82425c22bec50e6e42f0e495cab8.1643186507.git.gitgitgadget@gmail.com>
On Wed, Jan 26, 2022 at 08:41:45AM +0000, Matthew John Cheetham via GitGitGadget wrote:
Show 39 quoted lines
> From: Matthew John Cheetham <mjcheetham@outlook.com>
>
> Teach the `scalar diagnose` command to gather file size information
> about pack files.
>
> Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
> ---
> contrib/scalar/scalar.c | 39 ++++++++++++++++++++++++++++++++
> contrib/scalar/t/t9099-scalar.sh | 2 ++
> 2 files changed, 41 insertions(+)
>
> diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
> index e26fb2fc018..690933ffdf3 100644
> --- a/contrib/scalar/scalar.c
> +++ b/contrib/scalar/scalar.c
> @@ -653,6 +653,39 @@ cleanup:
> return res;
> }
>
> +static void dir_file_stats(struct strbuf *buf, const char *path)
> +{
> + DIR *dir = opendir(path);
> + struct dirent *e;
> + struct stat e_stat;
> + struct strbuf file_path = STRBUF_INIT;
> + size_t base_path_len;
> +
> + if (!dir)
> + return;
> +
> + strbuf_addstr(buf, "Contents of ");
> + strbuf_add_absolute_path(buf, path);
> + strbuf_addstr(buf, ":\n");
> +
> + strbuf_add_absolute_path(&file_path, path);
> + strbuf_addch(&file_path, '/');
> + base_path_len = file_path.len;
> +
> + while ((e = readdir(dir)) != NULL)Hmm. Is there a reason that this couldn't use for_each_file_in_pack_dir() with a callback that just does the stat() and buffer manipulation?
I don't think it's critical either way, but it would eliminate some of the boilerplate that is shared between this implementation and the one that already exists in for_each_file_in_pack_dir().
> + if (!is_dot_or_dotdot(e->d_name) && e->d_type == DT_REG) {
> + strbuf_setlen(&file_path, base_path_len);
> + strbuf_addstr(&file_path, e->d_name);For what it's worth, I think the callback would start here:
> + if (!stat(file_path.buf, &e_stat)) > + strbuf_addf(buf, "%-70s %16"PRIuMAX"\n", > + e->d_name, > + (uintmax_t)e_stat.st_size);
...and end here.
Thanks, Taylor