Re: [PATCH] diagnose: require repository
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 14, 2023, 17:15 UTC
- Message-ID
- <xmqq5y39unvc.fsf@gitster.g>
- In-Reply-To
- <20231014135302.13095-1-martin.agren@gmail.com>
Martin Ågren <martin.agren@gmail.com> writes:
Show 14 quoted lines
> 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;