Subject: Re: [PATCH v8 12/15] bugreport: count loose objects

From: Emily Shaffer <[email-masked]>
Date: Tue, 25 Feb 2020 15:22:00 -0800

On Thu, Feb 20, 2020 at 01:04:33PM -0800, Junio C Hamano wrote:
> Emily Shaffer <[email-masked]> writes:
> 
> > The number of unpacked objects in a user's repository may help us
> > understand the root of the problem they're seeing, especially if a
> > command is running unusually slowly.
> >
> > Helped-by: Johannes Schindelin <[email-masked]>
> > Signed-off-by: Emily Shaffer <[email-masked]>
> > ---
> >  Documentation/git-bugreport.txt |  1 +
> >  bugreport.c                     | 52 +++++++++++++++++++++++++++++++++
> >  2 files changed, 53 insertions(+)
> >
> > diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
> > index 4d01528540..4fe1c60506 100644
> > --- a/Documentation/git-bugreport.txt
> > +++ b/Documentation/git-bugreport.txt
> > @@ -32,6 +32,7 @@ The following information is captured automatically:
> >   - $SHELL
> >   - Selected config values
> >   - A list of enabled hooks
> > + - The number of loose objects in the repository
> >  
> >  This tool is invoked via the typical Git setup process, which means that in some
> >  cases, it might not be able to launch - for example, if a relevant config file
> > diff --git a/bugreport.c b/bugreport.c
> > index b5a0714a7f..fb7bc72723 100644
> > --- a/bugreport.c
> > +++ b/bugreport.c
> > @@ -10,6 +10,7 @@
> >  #include "bugreport-config-safelist.h"
> >  #include "khash.h"
> >  #include "run-command.h"
> > +#include "object-store.h"
> >  
> >  static void get_git_remote_https_version_info(struct strbuf *version_info)
> >  {
> > @@ -128,6 +129,54 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit)
> >  	}
> >  }
> >  
> > +static int loose_object_cb(const struct object_id *oid, const char *path,
> > +			   void *data) {
> > +	int *loose_object_count = data;
> > +
> > +	if (loose_object_count) {
> > +		(*loose_object_count)++;
> > +		return 0;
> > +	}
> > +
> > +	return 1;
> 
> What is the point of returning 1 here to abort the iteration early?
> Wouldn't it be a BUG() if this callback ends up gettting called with
> NULL in data?

Hrm. Wouldn't BUG() throw away the rest of the generated report?

Maybe it's better, in NULL case, to indicate an error occurred there and
write that information into the report, and continue gathering the rest
of the info.