git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Emily Shaffer <emilyshaffer@google.com>
Subject: Re: [PATCH 3/5] make git-bugreport a builtin
Date: Thu, 13 Aug 2020 11:01:23 -0700	[thread overview]
Message-ID: <xmqqr1sa2znw.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200813145936.GC891370@coredump.intra.peff.net> (Jeff King's message of "Thu, 13 Aug 2020 10:59:36 -0400")

Jeff King <peff@peff.net> writes:

> 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.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Makefile                            |  6 +-----
>  builtin.h                           |  1 +
>  bugreport.c => builtin/bugreport.c  | 10 +++-------
>  contrib/buildsystems/CMakeLists.txt |  5 +----
>  git.c                               |  1 +
>  5 files changed, 7 insertions(+), 16 deletions(-)
>  rename bugreport.c => builtin/bugreport.c (96%)

I am on the fence, as bugreport does not seem to be fully completed
part of the system.  The original thinking was that it may soon want
to grow by linking with platform specific libraries for lower-level
system characteristic identification, at which time we'd not want to
have it in builtins and "we can take advantage of builtin niceties"
will cause us regrets.  The only reason that hasn't happened as far
as I can see is because its development speed is rather slow.

So, I dunno.


> diff --git a/Makefile b/Makefile
> index 5b43c0fafb..acaff6968b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -671,7 +671,6 @@ EXTRA_PROGRAMS =
>  # ... and all the rest that could be moved out of bindir to gitexecdir
>  PROGRAMS += $(EXTRA_PROGRAMS)
>  
> -PROGRAM_OBJS += bugreport.o
>  PROGRAM_OBJS += daemon.o
>  PROGRAM_OBJS += fast-import.o
>  PROGRAM_OBJS += http-backend.o
> @@ -1041,6 +1040,7 @@ BUILTIN_OBJS += builtin/archive.o
>  BUILTIN_OBJS += builtin/bisect--helper.o
>  BUILTIN_OBJS += builtin/blame.o
>  BUILTIN_OBJS += builtin/branch.o
> +BUILTIN_OBJS += builtin/bugreport.o
>  BUILTIN_OBJS += builtin/bundle.o
>  BUILTIN_OBJS += builtin/cat-file.o
>  BUILTIN_OBJS += builtin/check-attr.o
> @@ -2458,10 +2458,6 @@ endif
>  git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>  
> -git-bugreport$X: bugreport.o GIT-LDFLAGS $(GITLIBS)
> -	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> -		$(LIBS)
> -
>  git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
>  		$(IMAP_SEND_LDFLAGS) $(LIBS)
> diff --git a/builtin.h b/builtin.h
> index 4a0aed5448..1e78d6c142 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -119,6 +119,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix);
>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix);
>  int cmd_blame(int argc, const char **argv, const char *prefix);
>  int cmd_branch(int argc, const char **argv, const char *prefix);
> +int cmd_bugreport(int argc, const char **argv, const char *prefix);
>  int cmd_bundle(int argc, const char **argv, const char *prefix);
>  int cmd_cat_file(int argc, const char **argv, const char *prefix);
>  int cmd_checkout(int argc, const char **argv, const char *prefix);
> diff --git a/bugreport.c b/builtin/bugreport.c
> similarity index 96%
> rename from bugreport.c
> rename to builtin/bugreport.c
> index 09579e268d..9c920cc065 100644
> --- a/bugreport.c
> +++ b/builtin/bugreport.c
> @@ -1,4 +1,4 @@
> -#include "cache.h"
> +#include "builtin.h"
>  #include "parse-options.h"
>  #include "strbuf.h"
>  #include "help.h"
> @@ -119,16 +119,14 @@ static void get_header(struct strbuf *buf, const char *title)
>  	strbuf_addf(buf, "\n\n[%s]\n", title);
>  }
>  
> -int cmd_main(int argc, const char **argv)
> +int cmd_bugreport(int argc, const char **argv, const char *prefix)
>  {
>  	struct strbuf buffer = STRBUF_INIT;
>  	struct strbuf report_path = STRBUF_INIT;
>  	int report = -1;
>  	time_t now = time(NULL);
>  	char *option_output = NULL;
>  	char *option_suffix = "%Y-%m-%d-%H%M";
> -	int nongit_ok = 0;
> -	const char *prefix = NULL;
>  	const char *user_relative_path = NULL;
>  
>  	const struct option bugreport_options[] = {
> @@ -139,8 +137,6 @@ int cmd_main(int argc, const char **argv)
>  		OPT_END()
>  	};
>  
> -	prefix = setup_git_directory_gently(&nongit_ok);
> -
>  	argc = parse_options(argc, argv, prefix, bugreport_options,
>  			     bugreport_usage, 0);
>  
> @@ -170,7 +166,7 @@ int cmd_main(int argc, const char **argv)
>  	get_system_info(&buffer);
>  
>  	get_header(&buffer, _("Enabled Hooks"));
> -	get_populated_hooks(&buffer, nongit_ok);
> +	get_populated_hooks(&buffer, !startup_info->have_repository);
>  
>  	/* fopen doesn't offer us an O_EXCL alternative, except with glibc. */
>  	report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 4be61247e5..3e211606fd 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -501,7 +501,7 @@ unset(CMAKE_REQUIRED_INCLUDES)
>  
>  #programs
>  set(PROGRAMS_BUILT
> -	git git-bugreport git-daemon git-fast-import git-http-backend git-sh-i18n--envsubst
> +	git git-daemon git-fast-import git-http-backend git-sh-i18n--envsubst
>  	git-shell git-remote-testsvn)
>  
>  if(NOT CURL_FOUND)
> @@ -624,9 +624,6 @@ list(TRANSFORM git_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
>  add_executable(git ${CMAKE_SOURCE_DIR}/git.c ${git_SOURCES})
>  target_link_libraries(git common-main)
>  
> -add_executable(git-bugreport ${CMAKE_SOURCE_DIR}/bugreport.c)
> -target_link_libraries(git-bugreport common-main)
> -
>  add_executable(git-daemon ${CMAKE_SOURCE_DIR}/daemon.c)
>  target_link_libraries(git-daemon common-main)
>  
> diff --git a/git.c b/git.c
> index 39a160fa52..bf790e7f4f 100644
> --- a/git.c
> +++ b/git.c
> @@ -479,6 +479,7 @@ static struct cmd_struct commands[] = {
>  	{ "bisect--helper", cmd_bisect__helper, RUN_SETUP },
>  	{ "blame", cmd_blame, RUN_SETUP },
>  	{ "branch", cmd_branch, RUN_SETUP | DELAY_PAGER_CONFIG },
> +	{ "bugreport", cmd_bugreport, RUN_SETUP_GENTLY },
>  	{ "bundle", cmd_bundle, RUN_SETUP_GENTLY | NO_PARSEOPT },
>  	{ "cat-file", cmd_cat_file, RUN_SETUP },
>  	{ "check-attr", cmd_check_attr, RUN_SETUP },

  parent reply	other threads:[~2020-08-13 18:01 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
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 [this message]
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=xmqqr1sa2znw.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --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).