git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCH 0/5] slimming down installed size
Date: Thu, 13 Aug 2020 11:13:29 -0400	[thread overview]
Message-ID: <20200813151329.GD2244@syl.lan> (raw)
In-Reply-To: <20200813145515.GA891139@coredump.intra.peff.net>

On Thu, Aug 13, 2020 at 10:55:15AM -0400, Jeff King wrote:
> A while ago I was looking at the size of an installed (and stripped)
> build of Git, and noticed we could make a few more things into builtins.
>
> Patches 2-4 do that.  I don't think there's any real downside here.
> We're not bringing in any new link dependencies to the main binary. And
> even if these programs don't use all of the code that's in the main
> binary, operating systems are good at just loading the pages that we do
> need (and you're probably running other Git programs anyway, which means
> the main binary is in RAM anyway and we're actually reducing the overall
> memory footprint).
>
> The final patch is not strictly related, but it's something I've had
> sitting around for a long time. I actually posted it almost exactly two
> years ago:
>
>   https://lore.kernel.org/git/20180817190310.GA5360@sigill.intra.peff.net/
>
> which ended with Jonathan saying:
>
> >  - keeping this in-tree for the benefit of just one user is excessive,
> >    so removing it is probably the right thing
> >
> >  - it would be nice if the commit removing this code from Git includes
> >    a note to help people find its new home
> >
> > Would you mind holding off until I'm able to arrange that last bit?
>
> So...I held off for a bit, but I'd really like to be done with this (and
> I prefer dropping the code, because I have a few other tree-wide
> cleanups for which I'd just as soon not have to deal with vcs-svn).
>
>   [1/5]: Makefile: drop builtins from MSVC pdb list
>   [2/5]: make credential helpers builtins
>   [3/5]: make git-bugreport a builtin
>   [4/5]: make git-fast-import a builtin
>   [5/5]: drop vcs-svn experiment
>
>  .gitignore                                    |    1 -
>  Makefile                                      |   45 +-
>  builtin.h                                     |    5 +
>  bugreport.c => builtin/bugreport.c            |   10 +-
>  .../credential-cache--daemon.c                |   29 +-
>  .../credential-cache.c                        |   29 +-
>  .../credential-store.c                        |    6 +-
>  fast-import.c => builtin/fast-import.c        |    3 +-
>  contrib/buildsystems/CMakeLists.txt           |   52 +-
>  contrib/svn-fe/.gitignore                     |    4 -
>  contrib/svn-fe/Makefile                       |  105 --
>  contrib/svn-fe/svn-fe.c                       |   18 -
>  contrib/svn-fe/svn-fe.txt                     |   71 --
>  contrib/svn-fe/svnrdump_sim.py                |   68 -
>  git.c                                         |    5 +
>  remote-testsvn.c                              |  341 -----
>  t/helper/.gitignore                           |    2 -
>  t/helper/test-line-buffer.c                   |   81 --
>  t/helper/test-svn-fe.c                        |   52 -
>  t/t0081-line-buffer.sh                        |   90 --
>  t/t9010-svn-fe.sh                             | 1105 -----------------
>  t/t9011-svn-da.sh                             |  248 ----
>  t/t9020-remote-svn.sh                         |   95 --
>  t/test-lib-functions.sh                       |    2 +-
>  vcs-svn/LICENSE                               |   32 -
>  vcs-svn/fast_export.c                         |  365 ------
>  vcs-svn/line_buffer.c                         |  126 --
>  vcs-svn/line_buffer.txt                       |   77 --
>  vcs-svn/sliding_window.c                      |   79 --
>  vcs-svn/svndiff.c                             |  309 -----
>  vcs-svn/svndump.c                             |  540 --------
>  31 files changed, 80 insertions(+), 3915 deletions(-)
>  rename bugreport.c => builtin/bugreport.c (96%)
>  rename credential-cache--daemon.c => builtin/credential-cache--daemon.c (91%)
>  rename credential-cache.c => builtin/credential-cache.c (83%)
>  rename credential-store.c => builtin/credential-store.c (96%)
>  rename fast-import.c => builtin/fast-import.c (99%)
>  delete mode 100644 contrib/svn-fe/.gitignore
>  delete mode 100644 contrib/svn-fe/Makefile
>  delete mode 100644 contrib/svn-fe/svn-fe.c
>  delete mode 100644 contrib/svn-fe/svn-fe.txt
>  delete mode 100755 contrib/svn-fe/svnrdump_sim.py
>  delete mode 100644 remote-testsvn.c
>  delete mode 100644 t/helper/test-line-buffer.c
>  delete mode 100644 t/helper/test-svn-fe.c
>  delete mode 100755 t/t0081-line-buffer.sh
>  delete mode 100755 t/t9010-svn-fe.sh
>  delete mode 100755 t/t9011-svn-da.sh
>  delete mode 100755 t/t9020-remote-svn.sh
>  delete mode 100644 vcs-svn/LICENSE
>  delete mode 100644 vcs-svn/fast_export.c
>  delete mode 100644 vcs-svn/line_buffer.c
>  delete mode 100644 vcs-svn/line_buffer.txt
>  delete mode 100644 vcs-svn/sliding_window.c
>  delete mode 100644 vcs-svn/svndiff.c
>  delete mode 100644 vcs-svn/svndump.c

Looks all very good to me, and I'm certainly a fan of being able to drop
the installation size down substantially as you have done here. I didn't
comment on patches 3 and 4, since they are obviously correct and a good
thing to be doing.

It would be interesting to think about how we might prevent someone from
introducing something that should be a builtin and has to link against
libgit.a other than looking for it carefully in review, but that's
definitely outside the scope of what you're trying to do here.

Thanks.

Taylor

      parent reply	other threads:[~2020-08-13 15:13 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-13 14:55 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
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 ` Taylor Blau [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20200813151329.GD2244@syl.lan \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --subject='Re: [PATCH 0/5] slimming down installed size' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).