diff mbox series

[v3,7/9] bugreport: add packed object summary

Message ID 20191025025129.250049-8-emilyshaffer@google.com (mailing list archive)
State New, archived
Headers show
Series add git-bugreport tool | expand

Commit Message

Emily Shaffer Oct. 25, 2019, 2:51 a.m. UTC
Alongside the list of loose objects, it's useful to see the list of
object packs as well. It can help us to examine what Git did and did not
pack.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 bugreport.c         | 21 +++++++++++++++++++++
 bugreport.h         |  6 ++++++
 builtin/bugreport.c |  4 ++++
 3 files changed, 31 insertions(+)

Comments

Johannes Schindelin Oct. 28, 2019, 3:43 p.m. UTC | #1
Hi Emily,

On Thu, 24 Oct 2019, Emily Shaffer wrote:

> Alongside the list of loose objects, it's useful to see the list of
> object packs as well. It can help us to examine what Git did and did not
> pack.

Yes, I was write! Packs are next ;-)

>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>  bugreport.c         | 21 +++++++++++++++++++++
>  bugreport.h         |  6 ++++++
>  builtin/bugreport.c |  4 ++++
>  3 files changed, 31 insertions(+)
>
> diff --git a/bugreport.c b/bugreport.c
> index 54e1d47103..79ddb8baaa 100644
> --- a/bugreport.c
> +++ b/bugreport.c
> @@ -219,3 +219,24 @@ void get_loose_object_summary(struct strbuf *obj_info)
>  			    objects.nr);
>  	}
>  }
> +
> +void get_packed_object_summary(struct strbuf *obj_info)
> +{
> +	struct strbuf dirpath = STRBUF_INIT;
> +	struct string_list contents = STRING_LIST_INIT_DUP;
> +	struct string_list_item *entry;
> +
> +	strbuf_reset(obj_info);
> +
> +	strbuf_addstr(&dirpath, get_object_directory());
> +	strbuf_complete(&dirpath, '/');
> +	strbuf_addstr(&dirpath, "pack/");
> +	list_contents_of_dir(&contents, &dirpath, 0, 0);
> +
> +	// list full contents of $GIT_OBJECT_DIRECTORY/pack/
> +	for_each_string_list_item(entry, &contents) {
> +		strbuf_addbuf(obj_info, &dirpath);
> +		strbuf_addstr(obj_info, entry->string);
> +		strbuf_complete_line(obj_info);
> +	}
> +}

Okay, but I think that you will want to discern between regular `.pack`
files, `.idx` files and `tmp_*` files.

> diff --git a/bugreport.h b/bugreport.h
> index 09ad0c2599..11ff7df41b 100644
> --- a/bugreport.h
> +++ b/bugreport.h
> @@ -24,3 +24,9 @@ void get_populated_hooks(struct strbuf *hook_info);
>   * will be discarded.
>   */
>  void get_loose_object_summary(struct strbuf *obj_info);
> +
> +/**
> + * Adds a list of the contents of '.git/objects/pack'. The previous contents of
> + * hook_info will be discarded.
> + */
> +void get_packed_object_summary(struct strbuf *obj_info);
> diff --git a/builtin/bugreport.c b/builtin/bugreport.c
> index b2ab194207..da91a3944e 100644
> --- a/builtin/bugreport.c
> +++ b/builtin/bugreport.c
> @@ -68,6 +68,10 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
>  	get_loose_object_summary(&buffer);
>  	strbuf_write(&buffer, report);
>
> +	add_header(report, "Packed Object Summary");
> +	get_packed_object_summary(&buffer);
> +	strbuf_write(&buffer, report);
> +

Hmm. At this point, I am unclear whether you want to write into an
`strbuf`, or directly into a `FILE *`? I would rather have only one, not
a mix.

Ciao,
Dscho

>  	fclose(report);
>
>  	launch_editor(report_path.buf, NULL, NULL);
> --
> 2.24.0.rc0.303.g954a862665-goog
>
>
Emily Shaffer Dec. 11, 2019, 12:29 a.m. UTC | #2
On Mon, Oct 28, 2019 at 04:43:50PM +0100, Johannes Schindelin wrote:
> Hi Emily,
> 
> On Thu, 24 Oct 2019, Emily Shaffer wrote:
> 
> > Alongside the list of loose objects, it's useful to see the list of
> > object packs as well. It can help us to examine what Git did and did not
> > pack.
> 
> Yes, I was write! Packs are next ;-)
> 
> >
> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > ---
> >  bugreport.c         | 21 +++++++++++++++++++++
> >  bugreport.h         |  6 ++++++
> >  builtin/bugreport.c |  4 ++++
> >  3 files changed, 31 insertions(+)
> >
> > diff --git a/bugreport.c b/bugreport.c
> > index 54e1d47103..79ddb8baaa 100644
> > --- a/bugreport.c
> > +++ b/bugreport.c
> > @@ -219,3 +219,24 @@ void get_loose_object_summary(struct strbuf *obj_info)
> >  			    objects.nr);
> >  	}
> >  }
> > +
> > +void get_packed_object_summary(struct strbuf *obj_info)
> > +{
> > +	struct strbuf dirpath = STRBUF_INIT;
> > +	struct string_list contents = STRING_LIST_INIT_DUP;
> > +	struct string_list_item *entry;
> > +
> > +	strbuf_reset(obj_info);
> > +
> > +	strbuf_addstr(&dirpath, get_object_directory());
> > +	strbuf_complete(&dirpath, '/');
> > +	strbuf_addstr(&dirpath, "pack/");
> > +	list_contents_of_dir(&contents, &dirpath, 0, 0);
> > +
> > +	// list full contents of $GIT_OBJECT_DIRECTORY/pack/
> > +	for_each_string_list_item(entry, &contents) {
> > +		strbuf_addbuf(obj_info, &dirpath);
> > +		strbuf_addstr(obj_info, entry->string);
> > +		strbuf_complete_line(obj_info);
> > +	}
> > +}
> 
> Okay, but I think that you will want to discern between regular `.pack`
> files, `.idx` files and `tmp_*` files.

Discern in what way? How would you like to see them treated separately?
They're all being listed here, not just counted, so it seems to me like
I can read the generated bugreport and see which files are index, pack,
or temporary here.

> 
> > diff --git a/bugreport.h b/bugreport.h
> > index 09ad0c2599..11ff7df41b 100644
> > --- a/bugreport.h
> > +++ b/bugreport.h
> > @@ -24,3 +24,9 @@ void get_populated_hooks(struct strbuf *hook_info);
> >   * will be discarded.
> >   */
> >  void get_loose_object_summary(struct strbuf *obj_info);
> > +
> > +/**
> > + * Adds a list of the contents of '.git/objects/pack'. The previous contents of
> > + * hook_info will be discarded.
> > + */
> > +void get_packed_object_summary(struct strbuf *obj_info);
> > diff --git a/builtin/bugreport.c b/builtin/bugreport.c
> > index b2ab194207..da91a3944e 100644
> > --- a/builtin/bugreport.c
> > +++ b/builtin/bugreport.c
> > @@ -68,6 +68,10 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
> >  	get_loose_object_summary(&buffer);
> >  	strbuf_write(&buffer, report);
> >
> > +	add_header(report, "Packed Object Summary");
> > +	get_packed_object_summary(&buffer);
> > +	strbuf_write(&buffer, report);
> > +
> 
> Hmm. At this point, I am unclear whether you want to write into an
> `strbuf`, or directly into a `FILE *`? I would rather have only one, not
> a mix.
> 
> Ciao,
> Dscho
> 
> >  	fclose(report);
> >
> >  	launch_editor(report_path.buf, NULL, NULL);
> > --
> > 2.24.0.rc0.303.g954a862665-goog
> >
> >
Johannes Schindelin Dec. 11, 2019, 1:37 p.m. UTC | #3
Hi Emily,

On Tue, 10 Dec 2019, Emily Shaffer wrote:

> On Mon, Oct 28, 2019 at 04:43:50PM +0100, Johannes Schindelin wrote:
> > Hi Emily,
> >
> > On Thu, 24 Oct 2019, Emily Shaffer wrote:
> >
> > > Alongside the list of loose objects, it's useful to see the list of
> > > object packs as well. It can help us to examine what Git did and did not
> > > pack.
> >
> > Yes, I was write! Packs are next ;-)
> >
> > >
> > > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > > ---
> > >  bugreport.c         | 21 +++++++++++++++++++++
> > >  bugreport.h         |  6 ++++++
> > >  builtin/bugreport.c |  4 ++++
> > >  3 files changed, 31 insertions(+)
> > >
> > > diff --git a/bugreport.c b/bugreport.c
> > > index 54e1d47103..79ddb8baaa 100644
> > > --- a/bugreport.c
> > > +++ b/bugreport.c
> > > @@ -219,3 +219,24 @@ void get_loose_object_summary(struct strbuf *obj_info)
> > >  			    objects.nr);
> > >  	}
> > >  }
> > > +
> > > +void get_packed_object_summary(struct strbuf *obj_info)
> > > +{
> > > +	struct strbuf dirpath = STRBUF_INIT;
> > > +	struct string_list contents = STRING_LIST_INIT_DUP;
> > > +	struct string_list_item *entry;
> > > +
> > > +	strbuf_reset(obj_info);
> > > +
> > > +	strbuf_addstr(&dirpath, get_object_directory());
> > > +	strbuf_complete(&dirpath, '/');
> > > +	strbuf_addstr(&dirpath, "pack/");
> > > +	list_contents_of_dir(&contents, &dirpath, 0, 0);
> > > +
> > > +	// list full contents of $GIT_OBJECT_DIRECTORY/pack/
> > > +	for_each_string_list_item(entry, &contents) {
> > > +		strbuf_addbuf(obj_info, &dirpath);
> > > +		strbuf_addstr(obj_info, entry->string);
> > > +		strbuf_complete_line(obj_info);
> > > +	}
> > > +}
> >
> > Okay, but I think that you will want to discern between regular `.pack`
> > files, `.idx` files and `tmp_*` files.
>
> Discern in what way? How would you like to see them treated separately?
> They're all being listed here, not just counted, so it seems to me like
> I can read the generated bugreport and see which files are index, pack,
> or temporary here.

I take your word for it (sorry, it's been half an eternity since I wrapped
my head around the diff, I forgotten pretty much all about it ;-))

Ciao,
Dscho

>
> >
> > > diff --git a/bugreport.h b/bugreport.h
> > > index 09ad0c2599..11ff7df41b 100644
> > > --- a/bugreport.h
> > > +++ b/bugreport.h
> > > @@ -24,3 +24,9 @@ void get_populated_hooks(struct strbuf *hook_info);
> > >   * will be discarded.
> > >   */
> > >  void get_loose_object_summary(struct strbuf *obj_info);
> > > +
> > > +/**
> > > + * Adds a list of the contents of '.git/objects/pack'. The previous contents of
> > > + * hook_info will be discarded.
> > > + */
> > > +void get_packed_object_summary(struct strbuf *obj_info);
> > > diff --git a/builtin/bugreport.c b/builtin/bugreport.c
> > > index b2ab194207..da91a3944e 100644
> > > --- a/builtin/bugreport.c
> > > +++ b/builtin/bugreport.c
> > > @@ -68,6 +68,10 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
> > >  	get_loose_object_summary(&buffer);
> > >  	strbuf_write(&buffer, report);
> > >
> > > +	add_header(report, "Packed Object Summary");
> > > +	get_packed_object_summary(&buffer);
> > > +	strbuf_write(&buffer, report);
> > > +
> >
> > Hmm. At this point, I am unclear whether you want to write into an
> > `strbuf`, or directly into a `FILE *`? I would rather have only one, not
> > a mix.
> >
> > Ciao,
> > Dscho
> >
> > >  	fclose(report);
> > >
> > >  	launch_editor(report_path.buf, NULL, NULL);
> > > --
> > > 2.24.0.rc0.303.g954a862665-goog
> > >
> > >
>
Emily Shaffer Dec. 11, 2019, 8:52 p.m. UTC | #4
On Wed, Dec 11, 2019 at 02:37:10PM +0100, Johannes Schindelin wrote:
> Hi Emily,
> 
> On Tue, 10 Dec 2019, Emily Shaffer wrote:
> 
> > On Mon, Oct 28, 2019 at 04:43:50PM +0100, Johannes Schindelin wrote:
> > > Hi Emily,
> > >
> > > On Thu, 24 Oct 2019, Emily Shaffer wrote:
> > >
> > > > Alongside the list of loose objects, it's useful to see the list of
> > > > object packs as well. It can help us to examine what Git did and did not
> > > > pack.
> > >
> > > Yes, I was write! Packs are next ;-)
> > >
> > > >
> > > > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > > > ---
> > > >  bugreport.c         | 21 +++++++++++++++++++++
> > > >  bugreport.h         |  6 ++++++
> > > >  builtin/bugreport.c |  4 ++++
> > > >  3 files changed, 31 insertions(+)
> > > >
> > > > diff --git a/bugreport.c b/bugreport.c
> > > > index 54e1d47103..79ddb8baaa 100644
> > > > --- a/bugreport.c
> > > > +++ b/bugreport.c
> > > > @@ -219,3 +219,24 @@ void get_loose_object_summary(struct strbuf *obj_info)
> > > >  			    objects.nr);
> > > >  	}
> > > >  }
> > > > +
> > > > +void get_packed_object_summary(struct strbuf *obj_info)
> > > > +{
> > > > +	struct strbuf dirpath = STRBUF_INIT;
> > > > +	struct string_list contents = STRING_LIST_INIT_DUP;
> > > > +	struct string_list_item *entry;
> > > > +
> > > > +	strbuf_reset(obj_info);
> > > > +
> > > > +	strbuf_addstr(&dirpath, get_object_directory());
> > > > +	strbuf_complete(&dirpath, '/');
> > > > +	strbuf_addstr(&dirpath, "pack/");
> > > > +	list_contents_of_dir(&contents, &dirpath, 0, 0);
> > > > +
> > > > +	// list full contents of $GIT_OBJECT_DIRECTORY/pack/
> > > > +	for_each_string_list_item(entry, &contents) {
> > > > +		strbuf_addbuf(obj_info, &dirpath);
> > > > +		strbuf_addstr(obj_info, entry->string);
> > > > +		strbuf_complete_line(obj_info);
> > > > +	}
> > > > +}
> > >
> > > Okay, but I think that you will want to discern between regular `.pack`
> > > files, `.idx` files and `tmp_*` files.
> >
> > Discern in what way? How would you like to see them treated separately?
> > They're all being listed here, not just counted, so it seems to me like
> > I can read the generated bugreport and see which files are index, pack,
> > or temporary here.
> 
> I take your word for it (sorry, it's been half an eternity since I wrapped
> my head around the diff, I forgotten pretty much all about it ;-))

That makes two of us. :)

> 
> Ciao,
> Dscho
> 
> >
> > >
> > > > diff --git a/bugreport.h b/bugreport.h
> > > > index 09ad0c2599..11ff7df41b 100644
> > > > --- a/bugreport.h
> > > > +++ b/bugreport.h
> > > > @@ -24,3 +24,9 @@ void get_populated_hooks(struct strbuf *hook_info);
> > > >   * will be discarded.
> > > >   */
> > > >  void get_loose_object_summary(struct strbuf *obj_info);
> > > > +
> > > > +/**
> > > > + * Adds a list of the contents of '.git/objects/pack'. The previous contents of
> > > > + * hook_info will be discarded.
> > > > + */
> > > > +void get_packed_object_summary(struct strbuf *obj_info);
> > > > diff --git a/builtin/bugreport.c b/builtin/bugreport.c
> > > > index b2ab194207..da91a3944e 100644
> > > > --- a/builtin/bugreport.c
> > > > +++ b/builtin/bugreport.c
> > > > @@ -68,6 +68,10 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
> > > >  	get_loose_object_summary(&buffer);
> > > >  	strbuf_write(&buffer, report);
> > > >
> > > > +	add_header(report, "Packed Object Summary");
> > > > +	get_packed_object_summary(&buffer);
> > > > +	strbuf_write(&buffer, report);
> > > > +
> > >
> > > Hmm. At this point, I am unclear whether you want to write into an
> > > `strbuf`, or directly into a `FILE *`? I would rather have only one, not
> > > a mix.
> > >
> > > Ciao,
> > > Dscho
> > >
> > > >  	fclose(report);
> > > >
> > > >  	launch_editor(report_path.buf, NULL, NULL);
> > > > --
> > > > 2.24.0.rc0.303.g954a862665-goog
> > > >
> > > >
> >
diff mbox series

Patch

diff --git a/bugreport.c b/bugreport.c
index 54e1d47103..79ddb8baaa 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -219,3 +219,24 @@  void get_loose_object_summary(struct strbuf *obj_info)
 			    objects.nr);
 	}
 }
+
+void get_packed_object_summary(struct strbuf *obj_info)
+{
+	struct strbuf dirpath = STRBUF_INIT;
+	struct string_list contents = STRING_LIST_INIT_DUP;
+	struct string_list_item *entry;
+
+	strbuf_reset(obj_info);
+
+	strbuf_addstr(&dirpath, get_object_directory());
+	strbuf_complete(&dirpath, '/');
+	strbuf_addstr(&dirpath, "pack/");
+	list_contents_of_dir(&contents, &dirpath, 0, 0);
+
+	// list full contents of $GIT_OBJECT_DIRECTORY/pack/
+	for_each_string_list_item(entry, &contents) {
+		strbuf_addbuf(obj_info, &dirpath);
+		strbuf_addstr(obj_info, entry->string);
+		strbuf_complete_line(obj_info);
+	}
+}
diff --git a/bugreport.h b/bugreport.h
index 09ad0c2599..11ff7df41b 100644
--- a/bugreport.h
+++ b/bugreport.h
@@ -24,3 +24,9 @@  void get_populated_hooks(struct strbuf *hook_info);
  * will be discarded.
  */
 void get_loose_object_summary(struct strbuf *obj_info);
+
+/**
+ * Adds a list of the contents of '.git/objects/pack'. The previous contents of
+ * hook_info will be discarded.
+ */
+void get_packed_object_summary(struct strbuf *obj_info);
diff --git a/builtin/bugreport.c b/builtin/bugreport.c
index b2ab194207..da91a3944e 100644
--- a/builtin/bugreport.c
+++ b/builtin/bugreport.c
@@ -68,6 +68,10 @@  int cmd_bugreport(int argc, const char **argv, const char *prefix)
 	get_loose_object_summary(&buffer);
 	strbuf_write(&buffer, report);
 
+	add_header(report, "Packed Object Summary");
+	get_packed_object_summary(&buffer);
+	strbuf_write(&buffer, report);
+
 	fclose(report);
 
 	launch_editor(report_path.buf, NULL, NULL);