Re: [PATCH v9 2/5] bugreport: add tool to generate debugging info
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Mar 4, 2020, 21:35 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2003042232340.46@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20200302230400.107428-3-emilyshaffer@google.com>
Hi,
On Mon, 2 Mar 2020, Emily Shaffer wrote:
Show 12 quoted lines
> .gitignore | 1 + > Documentation/git-bugreport.txt | 46 ++++++++++++++ > Makefile | 5 ++ > bugreport.c | 105 ++++++++++++++++++++++++++++++++ > command-list.txt | 1 + > strbuf.c | 4 ++ > strbuf.h | 1 + > t/t0091-bugreport.sh | 61 +++++++++++++++++++ > 8 files changed, 224 insertions(+) > create mode 100644 Documentation/git-bugreport.txt > create mode 100644 bugreport.c > create mode 100755 t/t0091-bugreport.sh
Hmm. I am still _quite_ convinced that this would be much better as a built-in. Remember, non-built-ins come with a footprint, and I do not necessarily think that you will want to spend 3MB on a `git-bugreport` executable when you could have it for a couple dozen kilobytes inside `git` instead.
Ciao, Dscho
Show 309 quoted lines
>
> diff --git a/.gitignore b/.gitignore
> index ea97de83f3..d89bf9e11e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -25,6 +25,7 @@
> /git-bisect--helper
> /git-blame
> /git-branch
> +/git-bugreport
> /git-bundle
> /git-cat-file
> /git-check-attr
> diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
> new file mode 100644
> index 0000000000..1f9fde5cde
> --- /dev/null
> +++ b/Documentation/git-bugreport.txt
> @@ -0,0 +1,46 @@
> +git-bugreport(1)
> +================
> +
> +NAME
> +----
> +git-bugreport - Collect information for user to file a bug report
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git bugreport' [(-o | --output-directory) <path>] [(-s | --suffix) <format>]
> +
> +DESCRIPTION
> +-----------
> +Captures information about the user's machine, Git client, and repository state,
> +as well as a form requesting information about the behavior the user observed,
> +into a single text file which the user can then share, for example to the Git
> +mailing list, in order to report an observed bug.
> +
> +The following information is requested from the user:
> +
> + - Reproduction steps
> + - Expected behavior
> + - Actual behavior
> +
> +This tool is invoked via the typical Git setup process, which means that in some
> +cases, it might not be able to launch - for example, if a relevant config file
> +is unreadable. In this kind of scenario, it may be helpful to manually gather
> +the kind of information listed above when manually asking for help.
> +
> +OPTIONS
> +-------
> +-o <path>::
> +--output-directory <path>::
> + Place the resulting bug report file in `<path>` instead of the root of
> + the Git repository.
> +
> +-s <format>::
> +--suffix <format>::
> + Specify an alternate suffix for the bugreport name, to create a file
> + named 'git-bugreport-<formatted suffix>'. This should take the form of a
> + link:strftime[3] format string; the current local time will be used.
> +
> +GIT
> +---
> +Part of the linkgit:git[1] suite
> diff --git a/Makefile b/Makefile
> index c552312d3f..9e6705061d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -681,6 +681,7 @@ EXTRA_PROGRAMS =
> # ... and all the rest that could be moved out of bindir to gitexecdir
> PROGRAMS += $(EXTRA_PROGRAMS)
>
> +PROGRAM_OBJS += bugreport.o
> PROGRAM_OBJS += credential-store.o
> PROGRAM_OBJS += daemon.o
> PROGRAM_OBJS += fast-import.o
> @@ -2461,6 +2462,10 @@ 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/bugreport.c b/bugreport.c
> new file mode 100644
> index 0000000000..f473d606f2
> --- /dev/null
> +++ b/bugreport.c
> @@ -0,0 +1,105 @@
> +#include "builtin.h"
> +#include "parse-options.h"
> +#include "stdio.h"
> +#include "strbuf.h"
> +#include "time.h"
> +
> +static const char * const bugreport_usage[] = {
> + N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
> + NULL
> +};
> +
> +static int get_bug_template(struct strbuf *template)
> +{
> + const char template_text[] = N_(
> +"Thank you for filling out a Git bug report!\n"
> +"Please answer the following questions to help us understand your issue.\n"
> +"\n"
> +"What did you do before the bug happened? (Steps to reproduce your issue)\n"
> +"\n"
> +"What did you expect to happen? (Expected behavior)\n"
> +"\n"
> +"What happened instead? (Actual behavior)\n"
> +"\n"
> +"What's different between what you expected and what actually happened?\n"
> +"\n"
> +"Anything else you want to add:\n"
> +"\n"
> +"Please review the rest of the bug report below.\n"
> +"You can delete any lines you don't wish to share.\n");
> +
> + strbuf_addstr(template, _(template_text));
> + return 0;
> +}
> +
> +int cmd_main(int argc, const char **argv)
> +{
> + 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 = "%F-%H%M";
> + int nongit_ok = 0;
> + const char *prefix = NULL;
> + const char *user_relative_path = NULL;
> +
> + const struct option bugreport_options[] = {
> + OPT_STRING('o', "output-directory", &option_output, N_("path"),
> + N_("specify a destination for the bugreport file")),
> + OPT_STRING('s', "suffix", &option_suffix, N_("format"),
> + N_("specify a strftime format suffix for the filename")),
> + OPT_END()
> + };
> +
> + prefix = setup_git_directory_gently(&nongit_ok);
> +
> + argc = parse_options(argc, argv, prefix, bugreport_options,
> + bugreport_usage, 0);
> +
> + /* Prepare the path to put the result */
> + strbuf_addstr(&report_path,
> + prefix_filename(prefix,
> + option_output ? option_output : ""));
> + strbuf_complete(&report_path, '/');
> +
> + strbuf_addstr(&report_path, "git-bugreport-");
> + strbuf_addftime(&report_path, option_suffix, localtime(&now), 0, 0);
> + strbuf_addstr(&report_path, ".txt");
> +
> + switch (safe_create_leading_directories(report_path.buf)) {
> + case SCLD_OK:
> + case SCLD_EXISTS:
> + break;
> + default:
> + die(_("could not create leading directories for '%s'"),
> + report_path.buf);
> + }
> +
> + /* Prepare the report contents */
> + get_bug_template(&buffer);
> +
> + /* fopen doesn't offer us an O_EXCL alternative, except with glibc. */
> + report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);
> +
> + if (report < 0) {
> + UNLEAK(report_path);
> + die(_("couldn't create a new file at '%s'"), report_path.buf);
> + }
> +
> + strbuf_write_fd(&buffer, report);
> + close(report);
> +
> + /*
> + * We want to print the path relative to the user, but we still need the
> + * path relative to us to give to the editor.
> + */
> + if (!(prefix && skip_prefix(report_path.buf, prefix, &user_relative_path)))
> + user_relative_path = report_path.buf;
> + fprintf(stderr, _("Created new report at '%s'.\n"),
> + user_relative_path);
> +
> + UNLEAK(buffer);
> + UNLEAK(report_path);
> + return !!launch_editor(report_path.buf, NULL, NULL);
> +}
> diff --git a/command-list.txt b/command-list.txt
> index 2087894655..185e5e3f05 100644
> --- a/command-list.txt
> +++ b/command-list.txt
> @@ -54,6 +54,7 @@ git-archive mainporcelain
> git-bisect mainporcelain info
> git-blame ancillaryinterrogators complete
> git-branch mainporcelain history
> +git-bugreport ancillaryinterrogators
> git-bundle mainporcelain
> git-cat-file plumbinginterrogators
> git-check-attr purehelpers
> diff --git a/strbuf.c b/strbuf.c
> index f19da55b07..f1d66c7848 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -539,6 +539,10 @@ ssize_t strbuf_write(struct strbuf *sb, FILE *f)
> return sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;
> }
>
> +ssize_t strbuf_write_fd(struct strbuf *sb, int fd)
> +{
> + return sb->len ? write(fd, sb->buf, sb->len) : 0;
> +}
>
> #define STRBUF_MAXLINK (2*PATH_MAX)
>
> diff --git a/strbuf.h b/strbuf.h
> index aae7ac3a82..bbf6204de7 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -462,6 +462,7 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
> * NUL bytes.
> */
> ssize_t strbuf_write(struct strbuf *sb, FILE *stream);
> +ssize_t strbuf_write_fd(struct strbuf *sb, int fd);
>
> /**
> * Read a line from a FILE *, overwriting the existing contents of
> diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
> new file mode 100755
> index 0000000000..65f664fdac
> --- /dev/null
> +++ b/t/t0091-bugreport.sh
> @@ -0,0 +1,61 @@
> +#!/bin/sh
> +
> +test_description='git bugreport'
> +
> +. ./test-lib.sh
> +
> +# Headers "[System Info]" will be followed by a non-empty line if we put some
> +# information there; we can make sure all our headers were followed by some
> +# information to check if the command was successful.
> +HEADER_PATTERN="^\[.*\]$"
> +
> +check_all_headers_populated () {
> + while read -r line
> + do
> + if test "$(grep "$HEADER_PATTERN" "$line")"
> + then
> + echo "$line"
> + read -r nextline
> + if test -z "$nextline"; then
> + return 1;
> + fi
> + fi
> + done
> +}
> +
> +test_expect_success 'creates a report with content in the right places' '
> + git bugreport -s check-headers &&
> + check_all_headers_populated <git-bugreport-check-headers.txt &&
> + test_when_finished rm git-bugreport-check-headers.txt
> +'
> +
> +test_expect_success 'dies if file with same name as report already exists' '
> + >>git-bugreport-duplicate.txt &&
> + test_must_fail git bugreport --suffix duplicate &&
> + test_when_finished rm git-bugreport-duplicate.txt
> +'
> +
> +test_expect_success '--output-directory puts the report in the provided dir' '
> + git bugreport -o foo/ &&
> + test_path_is_file foo/git-bugreport-* &&
> + test_when_finished rm -fr foo/
> +'
> +
> +test_expect_success 'incorrect arguments abort with usage' '
> + test_must_fail git bugreport --false 2>output &&
> + test_i18ngrep usage output &&
> + test_path_is_missing git-bugreport-*
> +'
> +
> +test_expect_success 'runs outside of a git dir' '
> + nongit git bugreport &&
> + test_when_finished rm non-repo/git-bugreport-*
> +'
> +
> +test_expect_success 'can create leading directories outside of a git dir' '
> + nongit git bugreport -o foo/bar/baz &&
> + test_when_finished rm -fr foo/bar/baz
> +'
> +
> +
> +test_done
> --
> 2.25.0.265.gbab2e86ba0-goog
>
>