On Sat, 14 Oct 2023 at 19:15, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Martin Ågren <martin.agren@xxxxxxxxx> writes:
>
> > 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
Correcting myself: The zip archive would actually contain
`diagnostics.log` with some general info about the machine and Git
build.
> > 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.
Good point. TBH, I had no idea about `git bugreport --diagnose`.
> + if (!startup_info->have_repository && diagnose != DIAGNOSE_NONE) {
> + warning(_("no repository--diagnostic output disabled"));
> + diagnose = DIAGNOSE_NONE;
> + }
> +
When the user explicitly provides that option, it seems unfortunate to
me to drop it. Yes, we'd warn, but `git bugreport` then pops a text
editor, so you would only see the warning after finishing up the report.
(Maybe. By the time you quit your editor, you might not consider
checking the terminal for warnings and such.)
So I'm inclined to instead just die if we see the option outside a repo.
If `diagnose` the command fundamentally requires a repo (as with my
patch) it seems surprising to me to not have `--diagnose` the option
behave the same.
Martin
