Re: [PATCH v9 2/5] bugreport: add tool to generate debugging info

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 20, 2020, 15:42 UTC
Message-ID: <nycvar.QRO.7.76.6.2003201636040.46@tvgsbejvaqbjf.bet>
In-Reply-To: <20200319213902.GB45325@google.com>

Hi Emily,

On Thu, 19 Mar 2020, Emily Shaffer wrote:

Show 43 quoted lines

> On Wed, Mar 04, 2020 at 10:35:04PM +0100, Johannes Schindelin wrote:
>
> > On Mon, 2 Mar 2020, Emily Shaffer wrote:
> >
> > >  .gitignore                      |   1 +
> > >  Documentation/git-bugreport.txt |  46 ++++++++++++++
> > >  Makefile                        |   5 ++
> > >  bugreport.c                     | 105 ++++++++++++++++++++++++++++++++
> > >  command-list.txt                |   1 +
> > >  strbuf.c                        |   4 ++
> > >  strbuf.h                        |   1 +
> > >  t/t0091-bugreport.sh            |  61 +++++++++++++++++++
> > >  8 files changed, 224 insertions(+)
> > >  create mode 100644 Documentation/git-bugreport.txt
> > >  create mode 100644 bugreport.c
> > >  create mode 100755 t/t0091-bugreport.sh
> >
> > Hmm. I am still _quite_ convinced that this would be much better as a
> > built-in. Remember, non-built-ins come with a footprint, and I do not
> > necessarily think that you will want to spend 3MB on a `git-bugreport`
> > executable when you could have it for a couple dozen kilobytes inside
> > `git` instead.
>
> This is the kind of stuff I really wanted to get straightened out by
> sending the smaller changeset, so I'm glad to be having this
> conversation (again, and hopefully for the last time). I read the
> replies to this mail, which I'm truncating as I think many of them are
> distracting rather than discussion-worthy, and have tried to summarize:
>
> Builtin:
> + Don't have to call out-of-process to identify 'git version --build-options'
> + Better assurance that we aren't shipping a broken bugreport alongside a new
>   version
> - Binary bloat, possible startup time hit
> ? Libraries will behave identically to where the user is seeing issues
>   (This point is a possible pro but also a possible con; see similar point in
>   standalone list)
>
> Standalone:
> + Could investigate libraries who aren't behaving the way they should.
>   (This seems like it'd be better addressed by regression tests, and if we're so
>   sure that git-bugreport is working with the info at hand correctly, why don't
>   we just write the git binary that way too?)

No, you're linking with libgit.a, you will have _at least_ the same set of dependencies as `git`. I made that point before.

> + Avoid binary bloat in the main executable.

What? What bloat? Have you measured this?

> - Have to ship a big standalone binary instead.
> - To get accurate version/build info, need to query the Git executable in a new
>   process

Worse. You might pick up inconsistent info from unrelated Git versions, and would be none the wiser. I made that point before.

For example, if you call `git bugreport` in a Git version that does not even include `git-bugreport`, you could pick up a `git-bugreport` from a _different_ Git version. This is a very real scenario on Windows, where we have many applications that ship with their own minimal version of Git for Windows.

Show 14 quoted lines

> Of course if I missed capturing something, please add/correct. Some of
> these concerns are quantifiable - binary size and overhead, for example
> - so I'm planning on doing some experiments in the coming days. I plan
> put together the handful of patches in the latest version of the topic
> in both standalone and builtin mode, and then gather the following info:
>
>  - Binary size of 'git'
>  - Binary size of 'git-bugreport' when applicable
>  - Time for "make" following clean
>  - Time for 'git status' on an identical real-world repo (I'll use ours,
>    without touching the repo state e.g. changing branch or fetching in
>    between)
>  - Time to editor for 'git bugreport' with the same setup as previous
>  - Peak memory footprint during 'git status'

Something that you cannot quantify, of course, is the robustness. Which in my mind is more important than all of the above, and of course you would only be able to guarantee that with a built-in version.

Show 7 quoted lines

> And of course, if there's more to compare that I can quantify, please
> let me know that too. I expect I'll be ready to run the experiments by
> Monday (taking some time off tomorrow) so there's time for me to hear
> about other concerns.
>
> I'll hold off on sharing my own preference until after we've got some
> benchmarking to look at, so I can understand the whole picture.

Why? Why not strong-arm your preference? Junio and I are not shy doing the same, and those are _your_ patches. Junio is clearly not interested in the command at all, but you are clearly interested in it [*1*]. You should have more than just the final say over this.

Ciao, Dscho

Footnote *1*: I am *obviously* interested in this because it might potentially make my life as Git for Windows' maintainer a lot easier. If it is robust enough that I don't get bug reports about `git-bugreport` itself.