Subject: Re: [PATCH] diagnose: require repository

From: Junio C Hamano <[email-masked]>
Date: Sat, 14 Oct 2023 10:15:19 -0700

Martin Ågren <[email-masked]> writes:

> When `git diagnose` is run from outside a repo, it begins collecting
> various information before eventually hitting a segmentation fault,
> leaving an incomplete zip file behind.
>
> Switch from the gentle setup to requiring a git directory. Without a git
> repo, there isn't really much to diagnose.
>
> We could possibly do a best-effort collection of information about the
> machine and then give up. That would roughly be today's behavior but
> with a controlled exit rather than a segfault. However, the purpose of
> this tool is largely to create a zip archive. Rather than creating an
> empty zip file or no zip file at all, and having to explain that
> behavior, it seems more helpful to bail out clearly and early with a
> succinct error message.

Without having thought things through, offhand I agree with your "no
repository?  there is nothing worth tarring up then" assessment.

Because "git bugreport --diag" unconditionally spawns "git
diagnose", the former may also want to be extra careful, perhaps
like the attached patch.

 builtin/bugreport.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git c/builtin/bugreport.c w/builtin/bugreport.c
index d2ae5c305d..ac9e05fcf7 100644
--- c/builtin/bugreport.c
+++ w/builtin/bugreport.c
@@ -146,6 +146,11 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
 		    report_path.buf);
 	}
 
+	if (!startup_info->have_repository && diagnose != DIAGNOSE_NONE) {
+		warning(_("no repository--diagnostic output disabled"));
+		diagnose = DIAGNOSE_NONE;
+	}
+
 	/* Prepare diagnostics, if requested */
 	if (diagnose != DIAGNOSE_NONE) {
 		struct strbuf zip_path = STRBUF_INIT;