mbox series

[v5,00/15] add git-bugreport tool

Message ID 20200124033436.81097-1-emilyshaffer@google.com (mailing list archive)
Headers show
Series add git-bugreport tool | expand

Message

Emily Shaffer Jan. 24, 2020, 3:34 a.m. UTC
From: Emily Shaffer <emilyshaffer@google.com>

This topic branch depends on the patch mailed in
lore.kernel.org/git/20191211233820.185153-1-emilyshaffer@google.com in order to
display scopes for configs gathered during "bugreport: add config values from
safelist".

I'll summarize v4-v5. Since v4 has languished for some time, I don't
think an interdiff is too helpful, so I won't include one. Bonus, the
code is much simplified from some suggestions from Junio on how to
inspect objects, so I hope it's easy to review anyways.

Throughout, did some style changes away from C++ bad habits. Otherwise,
as listed:

Emily Shaffer (15):
  bugreport: add tool to generate debugging info
    (no change)
  help: move list_config_help to builtin/help
    Changed to agree with f3719846134

  bugreport: gather git version and build info
    Style only

  help: add shell-path to --build-options
    (no change)

  bugreport: add uname info
    Removed nodename for privacy reasons

  bugreport: add compiler info
    Moved glibc tattling into compat/.

    I appreciate a close look at this one - I think I understood the
    right way to go about a compat/ util but it's the first one I've
    done.

  bugreport: add curl version
    Moved curl tattling from git-http-fetch into git-remote-curl

  bugreport: include user interactive shell
    Stop depending on a compiler quirk to save us from segfault

  bugreport: generate config safelist based on docs
    Changed to agree with f3719846134
    Cleaned up script a little

  bugreport: add config values from safelist
    Made git-bugreport dependent on generated safelist header

  bugreport: collect list of populated hooks
    No change.
    Per https://lore.kernel.org/git/20191216235131.GL135450@google.com,
    should we even keep this patch?

  bugreport: count loose objects
    Use helpers from object-store.h instead of manually walking the
    filesystem.

  bugreport: add packed object summary
    Use helpers from object-store.h instead of manually walking the
    filesystem.

  bugreport: list contents of $OBJDIR/info
    (no change)

  bugreport: summarize contents of alternates file
    Rephrase commit message to explain why I can't use the helpers in
    object-store.h.


Thanks.
 - Emily


 .gitignore                              |   3 +
 Documentation/asciidoc.conf             |   8 +
 Documentation/asciidoctor-extensions.rb |   7 +
 Documentation/config/sendemail.txt      |  68 ++--
 Documentation/git-bugreport.txt         |  43 +++
 Makefile                                |  25 +-
 bugreport.c                             | 427 ++++++++++++++++++++++++
 builtin/help.c                          |  86 +++++
 compat/compiler.h                       |  24 ++
 generate-bugreport-config-safelist.sh   |  22 ++
 generate-cmdlist.sh                     |  19 --
 generate-configlist.sh                  |  24 ++
 help.c                                  | 133 ++------
 help.h                                  |   2 +-
 remote-curl.c                           |   8 +
 t/t0091-bugreport.sh                    |  41 +++
 16 files changed, 780 insertions(+), 160 deletions(-)
 create mode 100644 Documentation/git-bugreport.txt
 create mode 100644 bugreport.c
 create mode 100644 compat/compiler.h
 create mode 100755 generate-bugreport-config-safelist.sh
 create mode 100755 generate-configlist.sh
 create mode 100755 t/t0091-bugreport.sh

Comments

Emily Shaffer Jan. 24, 2020, 3:38 a.m. UTC | #1
A sample bugreport generated from this patchset follows:

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

What did you expect to happen? (Expected behavior)

What happened instead? (Actual behavior)

What's different between what you expected and what actually happened?

Anything else you want to add:

Please review the rest of the bug report below.
You can delete any lines you don't wish to send.


[System Info]
git version:
git version 2.25.0.18.g682ab0d3eb
cpu: x86_64
built from commit: 682ab0d3eb8b84f8af4db1a161d24ca53d2f39fc
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname -a: Linux 5.2.17-1rodete3-amd64 #1 SMP Debian 5.2.17-1rodete3 (2019-10-21 > 2018) x86_64
compiler info: glibc: 2.28
$SHELL (typically, interactive shell): /bin/bash
git-remote-https --build-info:
git-http-fetch version: 2.25.0.18.g682ab0d3eb
built from commit: 682ab0d3eb8b84f8af4db1a161d24ca53d2f39fc
curl version: libcurl/7.66.0 GnuTLS/3.6.9 zlib/1.2.11 brotli/1.0.7 libidn2/2.2.0 libpsl/0.20.2 (+libidn2/2.2.0) libssh2/1.8.0 nghttp2/1.39.2 librtmp/2.3


[Safelisted Config Info]
sendemail.from (global) : emilyshaffer@google.com
sendemail.from (repo) : emilyshaffer@google.com


[Configured Hooks]
pre-commit
prepare-commit-msg


[Loose Object Counts]
2549 local loose objects
2641 alternate loose objects
5190 total loose objects


[Packed Object Summary]
60 total packs (591308 objects)


[Object Info Summary]
.git/objects/info/foo
.git/objects/info/bar
.git/objects/info/bar/baz
.git/objects/info/alternates


[Alternates]
2 alternates found (1 working, 1 broken)
Jonathan Tan Jan. 28, 2020, 11:04 p.m. UTC | #2
> From: Emily Shaffer <emilyshaffer@google.com>
> 
> This topic branch depends on the patch mailed in
> lore.kernel.org/git/20191211233820.185153-1-emilyshaffer@google.com in order to
> display scopes for configs gathered during "bugreport: add config values from
> safelist".
> 
> I'll summarize v4-v5. Since v4 has languished for some time, I don't
> think an interdiff is too helpful, so I won't include one. Bonus, the
> code is much simplified from some suggestions from Junio on how to
> inspect objects, so I hope it's easy to review anyways.

To everyone in the developer community interested in this set: what is
the status of this?

If this needs further review, then maybe it would be best if only
patches 1-4 were put up for submission first, with a note in the
bugreport documentation that more information may be added in future Git
versions. For me, patches 1-4 look good and I don't have enough
experience with uname (especially across libcs and OSes) to determine
what should or should not be included - if this is typical of reviewers
in the Git project, it might be better to submit patches 1-4 first, and
then send each additional diagnostic separately, so that people who know
what's going on in one area but not another can just comment on the area
they know about.

Having said that, I see that a few people have already looked at the
entire patchset and made comments, so if they are OK with it, we don't
need to split it up.
Emily Shaffer Jan. 28, 2020, 11:26 p.m. UTC | #3
On Tue, Jan 28, 2020 at 03:04:21PM -0800, Jonathan Tan wrote:
> > From: Emily Shaffer <emilyshaffer@google.com>
> > 
> > This topic branch depends on the patch mailed in
> > lore.kernel.org/git/20191211233820.185153-1-emilyshaffer@google.com in order to
> > display scopes for configs gathered during "bugreport: add config values from
> > safelist".
> > 
> > I'll summarize v4-v5. Since v4 has languished for some time, I don't
> > think an interdiff is too helpful, so I won't include one. Bonus, the
> > code is much simplified from some suggestions from Junio on how to
> > inspect objects, so I hope it's easy to review anyways.
> 
> To everyone in the developer community interested in this set: what is
> the status of this?
> 
> If this needs further review, then maybe it would be best if only
> patches 1-4 were put up for submission first, with a note in the
> bugreport documentation that more information may be added in future Git
> versions. For me, patches 1-4 look good and I don't have enough
> experience with uname (especially across libcs and OSes) to determine
> what should or should not be included - if this is typical of reviewers
> in the Git project, it might be better to submit patches 1-4 first, and
> then send each additional diagnostic separately, so that people who know
> what's going on in one area but not another can just comment on the area
> they know about.
> 
> Having said that, I see that a few people have already looked at the
> entire patchset and made comments, so if they are OK with it, we don't
> need to split it up.

I'd be fine with either; from now, my gut says the only ones I'm not
comfortable merging today are maybe 10, as Junio had some concerns about
whether to allow glob expansion, and 11, as it's prone to rot and I'm
doing other work in that area which would cause 11 to be
dropped/refactored anyways once it goes in. Otherwise I think they're
all pretty OK to go in and let the other work continue later in a
different thread.

Of course, as an author my opinion that they're good to go doesn't mean
much ;)

 - Emily
Martin Ågren Jan. 30, 2020, 10:15 p.m. UTC | #4
On Fri, 24 Jan 2020 at 04:35, <emilyshaffer@google.com> wrote:
> This topic branch depends on the patch mailed in
> lore.kernel.org/git/20191211233820.185153-1-emilyshaffer@google.com in order to
> display scopes for configs gathered during "bugreport: add config values from
> safelist".

Should this use `config_scope_name()` which looks like it's about
to graduate [1]? Disclaimer: I haven't followed that patch set too closely.

[1] https://lore.kernel.org/git/xmqqzhe66dav.fsf@gitster-ct.c.googlers.com/



Martin
Emily Shaffer Feb. 4, 2020, 12:07 a.m. UTC | #5
On Thu, Jan 30, 2020 at 11:15:25PM +0100, Martin Ågren wrote:
> On Fri, 24 Jan 2020 at 04:35, <emilyshaffer@google.com> wrote:
> > This topic branch depends on the patch mailed in
> > lore.kernel.org/git/20191211233820.185153-1-emilyshaffer@google.com in order to
> > display scopes for configs gathered during "bugreport: add config values from
> > safelist".
> 
> Should this use `config_scope_name()` which looks like it's about
> to graduate [1]? Disclaimer: I haven't followed that patch set too closely.

Yeah, you're right that I can use it. config.h:config_scope_name() is a
new feature on that patch since last time I looked. Thanks for the tip!

 - Emily

> [1] https://lore.kernel.org/git/xmqqzhe66dav.fsf@gitster-ct.c.googlers.com/