git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Derrick Stolee <stolee@gmail.com>
Cc: git@vger.kernel.org, Emily Shaffer <emilyshaffer@google.com>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 3/5] make git-bugreport a builtin
Date: Thu, 13 Aug 2020 13:38:45 -0400	[thread overview]
Message-ID: <20200813173845.GA1598703@coredump.intra.peff.net> (raw)
In-Reply-To: <f576fdce-9005-4653-3f91-0ddd2fff125d@gmail.com>

On Thu, Aug 13, 2020 at 01:01:42PM -0400, Derrick Stolee wrote:

> On 8/13/2020 10:59 AM, Jeff King wrote:
> > There's no reason that bugreport has to be a separate binary. And since
> > it links against libgit.a, it has a rather large disk footprint. Let's
> > make it a builtin, which reduces the size of a stripped installation
> > from 24MB to 22MB.
> > 
> > This also simplifies our Makefile a bit. And we can take advantage of
> > builtin niceties like RUN_SETUP_GENTLY.
> 
> I agree that this is a good change. I'm a bit surprised that a
> feature added so recently wasn't added as a builtin in the first
> place. Perhaps that was a conscious decision? Maybe it was just
> because there was some point in time where it might have been
> relegated to contrib.

I looked in the history for any rationale, but couldn't find any. There
is some discussion in the list, though. It looks like we moved from a
builtin to standalone between v3 and v4 of the series, due to this
sub-thread:

  https://lore.kernel.org/git/xmqqr2284is5.fsf@gitster-ct.c.googlers.com/

My take is:

  - I agree with Johannes that in terms of resource usage, builtins are
    better. I'm skeptical that extra code within the Git binary produces
    a meaningful impact on running other builtins (but I'd be happy to
    look at evidence if somebody has it).

  - Adding shared-library dependencies definitely _does_ impact other
    builtin commands. And I think last November when the series was
    shaping up that it wasn't clear if we might link to system-specific
    data gathering libraries. But that wasn't how the series shook out;
    we gather very little general system info, and we do it with basic
    and lightweight techniques (like uname, or compiler preprocessor
    macros). That could change eventually of course, but it's not like
    switching back to a stand-alone at that point is a big deal.

-Peff

  reply	other threads:[~2020-08-13 17:38 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-13 14:55 [PATCH 0/5] slimming down installed size Jeff King
2020-08-13 14:57 ` [PATCH 1/5] Makefile: drop builtins from MSVC pdb list Jeff King
2020-08-13 15:04   ` Taylor Blau
2020-08-13 15:08     ` Jeff King
2020-08-13 16:37       ` Derrick Stolee
2020-08-13 17:40         ` Jeff King
2020-08-14 14:18       ` Johannes Schindelin
2020-08-14 14:32         ` Jeff King
2020-08-17  4:42           ` Johannes Schindelin
2020-08-17 13:20         ` Jeff Hostetler
2020-08-13 14:58 ` [PATCH 2/5] make credential helpers builtins Jeff King
2020-08-13 15:08   ` Taylor Blau
2020-08-13 15:14     ` Jeff King
2020-08-13 17:55       ` Junio C Hamano
2020-08-13 14:59 ` [PATCH 3/5] make git-bugreport a builtin Jeff King
2020-08-13 17:01   ` Derrick Stolee
2020-08-13 17:38     ` Jeff King [this message]
2020-08-13 18:25       ` Junio C Hamano
2020-08-13 18:47         ` Junio C Hamano
2020-08-14 10:13           ` Jeff King
2020-08-14 14:25           ` Johannes Schindelin
2020-08-14 10:05         ` Jeff King
2020-08-13 18:01   ` Junio C Hamano
2020-08-14 14:28     ` Johannes Schindelin
2020-08-15  6:38     ` Jeff King
2020-08-17 12:12       ` Emily Shaffer
2020-08-17 16:58       ` Junio C Hamano
2020-08-17 21:40         ` Jeff King
2020-08-17 12:16   ` Emily Shaffer
2020-08-13 14:59 ` [PATCH 4/5] make git-fast-import " Jeff King
2020-08-13 15:00 ` [PATCH 5/5] drop vcs-svn experiment Jeff King
2020-08-13 15:11   ` Taylor Blau
2020-08-13 15:18     ` Jeff King
2020-08-14 14:39   ` Johannes Schindelin
2020-08-14 15:11     ` Jeff King
2020-08-13 15:13 ` [PATCH 0/5] slimming down installed size Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200813173845.GA1598703@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stolee@gmail.com \
    --subject='Re: [PATCH 3/5] make git-bugreport a builtin' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).