| Message ID | 20200302230400.107428-3-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add git-bugreport tool | expand |
Hi Emily,
On Mon, 2 Mar 2020, Emily Shaffer wrote:
> + char *option_suffix = "%F-%H%M";
Unfortunately, this still is not an `strftime` format we can use on
Windows. I still needed to fix that in the previously-mentioned manner to
fix the test run on Windows.
Thanks,
Dscho
Hi, On Mon, 2 Mar 2020, Emily Shaffer wrote: > .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 > > 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 > >
On 3/4/2020 4:35 PM, Johannes Schindelin wrote: > Hi, > > On Mon, 2 Mar 2020, Emily Shaffer wrote: > >> .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 Having this command be a stand-alone exe rather than a builtin allows it to have a different linkage. For example, you could include the libcurl and other libraries that are only linked into the transports. And then report version numbers for them if you wanted. Jeff
Hi Jeff, On Thu, 5 Mar 2020, Jeff Hostetler wrote: > On 3/4/2020 4:35 PM, Johannes Schindelin wrote: > > > > On Mon, 2 Mar 2020, Emily Shaffer wrote: > > > > > .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 > > Having this command be a stand-alone exe rather than a builtin allows > it to have a different linkage. For example, you could include the > libcurl and other libraries that are only linked into the transports. > And then report version numbers for them if you wanted. While that is true, I would argue that this is quite likely using the _wrong_ information if the user has a different `git-remote-https` in their `PATH`. In other words, that information could _mislead_ the recipient of the bug report. Now, other people might argue that the different linkage lets `git-bugreport` maybe work where `git` would fail to load a required dynamic library. But again, I would counter that this is a false assumption: since `git-bugreport` relies on `libgit.a`, it essentially _has_ to have the same linkage as `git`, or a superset thereof. So: No, I really think this is going the wrong direction. If we want `bugreport` to be a first-class citizen, we should treat it as such. That entails making it a built-in. That entails teaching `git-remote-https` (and potentially other non-builtins) to sprout that option to enquire other information that should be included in the generated part of the bug report. Ciao, Dscho
Jeff Hostetler <git@jeffhostetler.com> writes: > Having this command be a stand-alone exe rather than a builtin allows > it to have a different linkage. For example, you could include the > libcurl and other libraries that are only linked into the transports. > And then report version numbers for them if you wanted. I actually do not think that is a good rationale. Unless your version of "git bugreport" links into the same binary as the "transports", it still is possible that the version of cURL (for example) "git bugreport" can learn internally from may not have anything to do with the version of the library used by the transports. Of course, making "bugreport" a built-in, i.e. the same binary as the non-transport part of Git, is not a solution for that issue, either. As Dscho suggested and recent rounds of "git bugreport" implements, teaching the transport binaries an option to report relevant pieces of information regarding the libraries they use, and making "git bugreport" ask them, is a very good solution for that. What makes it possible by making "git bugreport" stand-alone is for it to link with libraries that the remainder of Git, including the transports that link with libcurl, has no business linking with (a library to obtain system details for diagnostic purposes, for example). Early versions of "git bugreport" may not link with anything special, but making sure it starts and stays standalone leaves it easier to do so _without_ having to worry about splitting the code that started as a built-in out to make it independent later.
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > If we want `bugreport` to be a first-class citizen, we should > treat it as such. That entails making it a built-in. The question is, for "bugreport", if it is better to be standalone or to be built-in, and I do not think 'first-class' has anything to do with the question. The transport based on curl-library are separate binaries but they are of course first-class citizens. Pushing and fetching will happen a lot less often than local operations like "add", "commit", "show", and not paying start-up latencies to load code and link libraries that are not necessary for local operations in built-in, while paying the cost of having to duplicate the memory and disk footprint necessary for libgit.a stuff by making the transport binaries separate from the built-ins, was conscious implementation decision we made earlier to prioritize the reduction of overhead of subcommands for local operation. Yes, the decision de-prioritizes the transport, but that does not mean they are not first-class citizens. It just means that the start-up latency for them (e.g. linking with libcurl) is not huge burden for the overall time cost for the network transfer, and it is necessary cost for them to perform what they need to do anyway, while it is something subcommands for local operations should not have to pay. > That entails teaching > `git-remote-https` (and potentially other non-builtins) to sprout that > option to enquire other information that should be included in the > generated part of the bug report. I think later rounds of "git bugreports" already did so. As I mentioned in a separate message to JeffH, this is a good thing to do, whether "git bugreport" is a built-in or a standalone.
On 3/6/2020 1:08 PM, Junio C Hamano wrote: > Jeff Hostetler <git@jeffhostetler.com> writes: > >> Having this command be a stand-alone exe rather than a builtin allows >> it to have a different linkage. For example, you could include the >> libcurl and other libraries that are only linked into the transports. >> And then report version numbers for them if you wanted. > > I actually do not think that is a good rationale. Unless your > version of "git bugreport" links into the same binary as the > "transports", it still is possible that the version of cURL (for > example) "git bugreport" can learn internally from may not have > anything to do with the version of the library used by the > transports. > > Of course, making "bugreport" a built-in, i.e. the same binary as > the non-transport part of Git, is not a solution for that issue, > either. As Dscho suggested and recent rounds of "git bugreport" > implements, teaching the transport binaries an option to report > relevant pieces of information regarding the libraries they use, and > making "git bugreport" ask them, is a very good solution for that. > > What makes it possible by making "git bugreport" stand-alone is for > it to link with libraries that the remainder of Git, including the > transports that link with libcurl, has no business linking with (a > library to obtain system details for diagnostic purposes, for > example). Early versions of "git bugreport" may not link with > anything special, but making sure it starts and stays standalone > leaves it easier to do so _without_ having to worry about splitting > the code that started as a built-in out to make it independent later. > > Yeah, that makes sense. Thanks Jeff
Hi Junio, On Fri, 6 Mar 2020, Junio C Hamano wrote: > What makes it possible by making "git bugreport" stand-alone is for > it to link with libraries that the remainder of Git, including the > transports that link with libcurl, has no business linking with (a > library to obtain system details for diagnostic purposes, for > example). That would, however, make `git-bugreport` more fragile than `git`, i.e. the former might fail to launch under more circumstances than the latter. Not a good idea for a tool meant to help reporting bugs, to be more fragile to even _start_ than `git`. A tool that is intended to help users come up with bug reports, despite the fact that users are totally unfamiliar with the implementation details of Git and hence unable to judge competently what information to include or to omit, needs to be _robust_. You do not want to end up, for example, calling a stale `git-bugreport` from an earlier `make install` (or from a different installation altogether). That would give the recipient not only bogus information, it would be outright misleading. And that's what I meant by "a stand-alone `git-bugreport` would not be a 1st-class citizen". I would see it as a serious design issue if this command is anything but a built-in. Ciao, Dscho
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Hi Junio, > > On Fri, 6 Mar 2020, Junio C Hamano wrote: > >> What makes it possible by making "git bugreport" stand-alone is for >> it to link with libraries that the remainder of Git, including the >> transports that link with libcurl, has no business linking with (a >> library to obtain system details for diagnostic purposes, for >> example). > > That would, however, make `git-bugreport` more fragile than `git`, i.e. > the former might fail to launch under more circumstances than the latter. That's a bug. You can go fix it when it happens.
Hi Junio, On Mon, 9 Mar 2020, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > On Fri, 6 Mar 2020, Junio C Hamano wrote: > > > >> What makes it possible by making "git bugreport" stand-alone is for > >> it to link with libraries that the remainder of Git, including the > >> transports that link with libcurl, has no business linking with (a > >> library to obtain system details for diagnostic purposes, for > >> example). > > > > That would, however, make `git-bugreport` more fragile than `git`, i.e. > > the former might fail to launch under more circumstances than the latter. > > That's a bug. You can go fix it when it happens. Heh... yeah, that would be a bug, and the user would not be able to report it via `git bugreport`... Which is the whole point of my complaint. Isn't it obvious that we should not have an independent `git-bugreport` by now? With a stand-alone `git-bugreport`, we might - fail to load the .so files under more circumstances than `git` would (since we link to `libgit.a`, we cannot have a subset of dependencies, only a superset, or the same), - launch a stale `git-bugreport` from a completely different Git, - make users angry for wasting 3MB of their diskspace for `git-bugreport` when only a dozen kilobyte would suffice. On the other hand, if we make `git-bugreport` a built-in, I cannot see any downsides. For me, therefore, having it as a built-in is a clear win. What am I missing? Ciao, Dscho
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > On the other hand, if we make `git-bugreport` a built-in, I cannot see any > downsides. > > For me, therefore, having it as a built-in is a clear win. What am I > missing? The right sense of relative importance between efficiently running the rest of Git by not bloting the main binary and making sure not to ship Git that does not run unless "git bugreport" runs (which makes sure that "bugreport" runs) is what you are missing, I suspect. Another thing is that you are giving "git bugreport" too much weight and too little credit to inexperienced users, by assuming that we will never hear from them when "bugreport" is incapable to run for them. They will report, with or without "git bugreport", and the more important thing you seem to be missing is that after the initial contact it would become an interactive process---there is no reason to prioritize to ensure that the initial contact has everything that is needed to diagnose any issue without further interaction. "With my build, even 'bugreport' dumps core." is perfectly good place to start. Besides, wouldn't the ones on platforms, on which "git bugreport" may have trouble running, i.e. the ones on minority and exotic platforms, tend to be self sufficient and capable (both diagnosing the issue for themselves, and reaching out to us as appropriate) ones in practice (e.g. I have NonStop folks in mind)?
Hi Junio, On Mon, 9 Mar 2020, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > On the other hand, if we make `git-bugreport` a built-in, I cannot see any > > downsides. > > > > For me, therefore, having it as a built-in is a clear win. What am I > > missing? > > The right sense of relative importance between efficiently running > the rest of Git by not bloting the main binary and making sure not > to ship Git that does not run unless "git bugreport" runs (which > makes sure that "bugreport" runs) is what you are missing, I > suspect. By that reasoning, `git bugreport` should not be included in core Git. > Another thing is that you are giving "git bugreport" too much weight > and too little credit to inexperienced users, by assuming that we > will never hear from them when "bugreport" is incapable to run for > them. They will report, with or without "git bugreport", and the > more important thing you seem to be missing is that after the > initial contact it would become an interactive process---there is > no reason to prioritize to ensure that the initial contact has > everything that is needed to diagnose any issue without further > interaction. "With my build, even 'bugreport' dumps core." is > perfectly good place to start. > > Besides, wouldn't the ones on platforms, on which "git bugreport" > may have trouble running, i.e. the ones on minority and exotic > platforms, tend to be self sufficient and capable (both diagnosing > the issue for themselves, and reaching out to us as appropriate) > ones in practice (e.g. I have NonStop folks in mind)? Yes, I can agree that inexperienced users will not give up and keep up the conversation until they see their problems fixed. I can also agree that inexperienced users will report bugs even if it is not made super easy for them to do so. I can agree to that _iff_ I ignore my entire experience maintaining Git for Windows, that is. Sarcasm aside, I think that you underestimate the importance of a good bug reporting tool like `git bugreport`, and I suspect that Emily does not. Ciao, Dscho
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> The right sense of relative importance between efficiently running >> the rest of Git by not bloting the main binary and making sure not >> to ship Git that does not run unless "git bugreport" runs (which >> makes sure that "bugreport" runs) is what you are missing, I >> suspect. > > By that reasoning, `git bugreport` should not be included in core Git. The world does not have to be black-or-white. "git bugreport" that covers mainstream platforms in widely-used configurations may be only an 80% solution, but that is better than nothing at all. >> Another thing is that you are giving "git bugreport" too much weight >> and too little credit to inexperienced users, by assuming that we >> will never hear from them when "bugreport" is incapable to run for >> them. They will report, with or without "git bugreport", and the >> more important thing you seem to be missing is that after the >> initial contact it would become an interactive process---there is >> no reason to prioritize to ensure that the initial contact has >> everything that is needed to diagnose any issue without further >> interaction. "With my build, even 'bugreport' dumps core." is >> perfectly good place to start. >> >> Besides, wouldn't the ones on platforms, on which "git bugreport" >> may have trouble running, i.e. the ones on minority and exotic >> platforms, tend to be self sufficient and capable (both diagnosing >> the issue for themselves, and reaching out to us as appropriate) >> ones in practice (e.g. I have NonStop folks in mind)? > > Yes, I can agree that inexperienced users will not give up and keep > up the conversation until they see their problems fixed. Inexperienced users won't be on minority platforms for which we do not have enough resource to get "git bugreport" to run adequetly in the first place. Even if those on minority platforms whose userbase are all technically incapable ones, if they do not help us help them, what can "git bugreport" can do, and more importantly, how would it make a difference between "bugreport" being a built-in vs a standalone? At this point, I'd have to say that your quibbling does not deserve a serious response and it would be better use of my time to disengage from this thread.
Hi Junio, On Tue, 10 Mar 2020, Junio C Hamano wrote: > At this point, I'd have to say that your quibbling does not deserve > a serious response and it would be better use of my time to > disengage from this thread. I thought we were discussing whether it makes sense to leave `git-bugreport` as a stand-alone executable, or rather make it a built-in. If my attempts to convince you that it would be better as a built-in strike you as "quibbling", then I should probably pull out from trying to review and help with this here patch series. I really thought I was helping. My apologies for annoying you instead. Ciao, Dscho
On Wed, Mar 04, 2020 at 10:35:04PM +0100, Johannes Schindelin wrote: > Hi, > > On Mon, 2 Mar 2020, Emily Shaffer wrote: > > > .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. This is the kind of stuff I really wanted to get straightened out by sending the smaller changeset, so I'm glad to be having this conversation (again, and hopefully for the last time). I read the replies to this mail, which I'm truncating as I think many of them are distracting rather than discussion-worthy, and have tried to summarize: Builtin: + Don't have to call out-of-process to identify 'git version --build-options' + Better assurance that we aren't shipping a broken bugreport alongside a new version - Binary bloat, possible startup time hit ? Libraries will behave identically to where the user is seeing issues (This point is a possible pro but also a possible con; see similar point in standalone list) Standalone: + Could investigate libraries who aren't behaving the way they should. (This seems like it'd be better addressed by regression tests, and if we're so sure that git-bugreport is working with the info at hand correctly, why don't we just write the git binary that way too?) + Avoid binary bloat in the main executable. - Have to ship a big standalone binary instead. - To get accurate version/build info, need to query the Git executable in a new process Of course if I missed capturing something, please add/correct. Some of these concerns are quantifiable - binary size and overhead, for example - so I'm planning on doing some experiments in the coming days. I plan put together the handful of patches in the latest version of the topic in both standalone and builtin mode, and then gather the following info: - Binary size of 'git' - Binary size of 'git-bugreport' when applicable - Time for "make" following clean - Time for 'git status' on an identical real-world repo (I'll use ours, without touching the repo state e.g. changing branch or fetching in between) - Time to editor for 'git bugreport' with the same setup as previous - Peak memory footprint during 'git status' And of course, if there's more to compare that I can quantify, please let me know that too. I expect I'll be ready to run the experiments by Monday (taking some time off tomorrow) so there's time for me to hear about other concerns. I'll hold off on sharing my own preference until after we've got some benchmarking to look at, so I can understand the whole picture. - Emily
Emily Shaffer <emilyshaffer@google.com> writes: > Builtin: > + Don't have to call out-of-process to identify 'git version --build-options' > + Better assurance that we aren't shipping a broken bugreport alongside a new > version > - Binary bloat, possible startup time hit > ? Libraries will behave identically to where the user is seeing issues > (This point is a possible pro but also a possible con; see similar point in > standalone list) - Makes it hard to pull in libraries that "bugreport" alone will want to use in the future without negatively impacting the rest of Git. > - Time to editor for 'git bugreport' with the same setup as previous This is something we absolutely should totally ignore. Even if the bulitin version takes 1 sec to spawn an editor while a standalone one took only 0.1 sec, if other criteria you listed that are more important favours the builtin solution, we should pick it. In any case, I think we've wasted enough time on this. Let's see a minimum working version that won't break/affect the rest of Git and ship it standalone. Thanks.
On Thu, 19 Mar 2020, Junio C Hamano wrote: > Emily Shaffer <emilyshaffer@google.com> writes: > > > Builtin: > > + Don't have to call out-of-process to identify 'git version --build-options' > > + Better assurance that we aren't shipping a broken bugreport alongside a new > > version > > - Binary bloat, possible startup time hit > > ? Libraries will behave identically to where the user is seeing issues > > (This point is a possible pro but also a possible con; see similar point in > > standalone list) > > - Makes it hard to pull in libraries that "bugreport" alone will > want to use in the future without negatively impacting the rest > of Git. And of course we should never do that, lest `git bugreport` won't even load because of it while `git` would. So even in the purely speculative scenario that we _wanted_ to add more libraries (which we don't, why should we?), we wouldn't actually do it. > > - Time to editor for 'git bugreport' with the same setup as previous > > This is something we absolutely should totally ignore. Even if the > bulitin version takes 1 sec to spawn an editor while a standalone > one took only 0.1 sec, if other criteria you listed that are more > important favours the builtin solution, we should pick it. > > In any case, I think we've wasted enough time on this. Let's see a > minimum working version that won't break/affect the rest of Git and > ship it standalone. I still disagree with that, but it seems that no amount of arguments will convince Junio, and he's dead set on the standalone version.
Hi Emily, On Thu, 19 Mar 2020, Emily Shaffer wrote: > On Wed, Mar 04, 2020 at 10:35:04PM +0100, Johannes Schindelin wrote: > > > On Mon, 2 Mar 2020, Emily Shaffer wrote: > > > > > .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. > > This is the kind of stuff I really wanted to get straightened out by > sending the smaller changeset, so I'm glad to be having this > conversation (again, and hopefully for the last time). I read the > replies to this mail, which I'm truncating as I think many of them are > distracting rather than discussion-worthy, and have tried to summarize: > > Builtin: > + Don't have to call out-of-process to identify 'git version --build-options' > + Better assurance that we aren't shipping a broken bugreport alongside a new > version > - Binary bloat, possible startup time hit > ? Libraries will behave identically to where the user is seeing issues > (This point is a possible pro but also a possible con; see similar point in > standalone list) > > Standalone: > + Could investigate libraries who aren't behaving the way they should. > (This seems like it'd be better addressed by regression tests, and if we're so > sure that git-bugreport is working with the info at hand correctly, why don't > we just write the git binary that way too?) No, you're linking with libgit.a, you will have _at least_ the same set of dependencies as `git`. I made that point before. > + Avoid binary bloat in the main executable. What? What bloat? Have you measured this? > - Have to ship a big standalone binary instead. > - To get accurate version/build info, need to query the Git executable in a new > process Worse. You might pick up inconsistent info from unrelated Git versions, and would be none the wiser. I made that point before. For example, if you call `git bugreport` in a Git version that does not even include `git-bugreport`, you could pick up a `git-bugreport` from a _different_ Git version. This is a very real scenario on Windows, where we have many applications that ship with their own minimal version of Git for Windows. > Of course if I missed capturing something, please add/correct. Some of > these concerns are quantifiable - binary size and overhead, for example > - so I'm planning on doing some experiments in the coming days. I plan > put together the handful of patches in the latest version of the topic > in both standalone and builtin mode, and then gather the following info: > > - Binary size of 'git' > - Binary size of 'git-bugreport' when applicable > - Time for "make" following clean > - Time for 'git status' on an identical real-world repo (I'll use ours, > without touching the repo state e.g. changing branch or fetching in > between) > - Time to editor for 'git bugreport' with the same setup as previous > - Peak memory footprint during 'git status' Something that you cannot quantify, of course, is the robustness. Which in my mind is more important than all of the above, and of course you would only be able to guarantee that with a built-in version. > And of course, if there's more to compare that I can quantify, please > let me know that too. I expect I'll be ready to run the experiments by > Monday (taking some time off tomorrow) so there's time for me to hear > about other concerns. > > I'll hold off on sharing my own preference until after we've got some > benchmarking to look at, so I can understand the whole picture. Why? Why not strong-arm your preference? Junio and I are not shy doing the same, and those are _your_ patches. Junio is clearly not interested in the command at all, but you are clearly interested in it [*1*]. You should have more than just the final say over this. Ciao, Dscho Footnote *1*: I am *obviously* interested in this because it might potentially make my life as Git for Windows' maintainer a lot easier. If it is robust enough that I don't get bug reports about `git-bugreport` itself.
Emily Shaffer <emilyshaffer@google.com> writes: > This is the kind of stuff I really wanted to get straightened out by > sending the smaller changeset, so I'm glad to be having this > conversation (again, and hopefully for the last time). I actually have a suspicion that "git bugreport" that is spawned via "git" wrapper is a bad idea (in other words, /usr/bin/git-bug that is totally standalone may be better). The thing is, anything launched by "git" as its subcommand (be it standalone or builtin) sees an environment already modified by "git", so inspecting say "$PATH" from "git bugreport" (be it standalone or builtin) does not show what the end-user, who may be having trouble, actually has. The mangling of $PATH alone happens to be simple and (we may think) we can easily reverse without losing bits, but there would probably other differences that we think is so subtle and insignificant, right now in this discussion without having actual end-user who is having trouble in front of us, that having "git" layer in between may hide from us. Having "git-bug" a totally separate tool, that does not even go through git.c::cmd_main(), would also allow (and force) us to treat "git" just like any system component on the end-users' system whose versions and configurations may affect the use of "git" by the end user. The tool, instead of relying on git_version_string[] that is determined at the compile time, would ask "git version" just like the end-users would when asked by us "what version of git do you run?" It also means that "git-bug" can be updated at different cadence and the mismatch of versions between it and "git" does not matter at all. Having said all that, after suggesting to make the tool even more distant from the remainder of the binary, quite honestly, I do not even want to see us wasting any more time on builtin/standalone at this point, and instead would like to see reports from the end-users produced by a lets-start-small version of the tool and measure how having the tool actually helps. Thanks.
Hi Junio, On Fri, 20 Mar 2020, Junio C Hamano wrote: > Emily Shaffer <emilyshaffer@google.com> writes: > > > This is the kind of stuff I really wanted to get straightened out by > > sending the smaller changeset, so I'm glad to be having this > > conversation (again, and hopefully for the last time). > > I actually have a suspicion that "git bugreport" that is spawned via > "git" wrapper is a bad idea (in other words, /usr/bin/git-bug that > is totally standalone may be better). The obvious downside of `/usr/bin/git-bug`, of course, is that it has no way to provide accurate data regarding, say, the cURL version in use. That's a pretty severe downside if you are truly interested in helping bug reporters and bug receivers. Ciao, Dscho > > The thing is, anything launched by "git" as its subcommand (be it > standalone or builtin) sees an environment already modified by > "git", so inspecting say "$PATH" from "git bugreport" (be it > standalone or builtin) does not show what the end-user, who may be > having trouble, actually has. The mangling of $PATH alone happens > to be simple and (we may think) we can easily reverse without losing > bits, but there would probably other differences that we think is so > subtle and insignificant, right now in this discussion without > having actual end-user who is having trouble in front of us, that > having "git" layer in between may hide from us. > > Having "git-bug" a totally separate tool, that does not even go > through git.c::cmd_main(), would also allow (and force) us to treat > "git" just like any system component on the end-users' system whose > versions and configurations may affect the use of "git" by the end > user. The tool, instead of relying on git_version_string[] that is > determined at the compile time, would ask "git version" just like > the end-users would when asked by us "what version of git do you > run?" It also means that "git-bug" can be updated at different > cadence and the mismatch of versions between it and "git" does not > matter at all. > > Having said all that, after suggesting to make the tool even more > distant from the remainder of the binary, quite honestly, I do not > even want to see us wasting any more time on builtin/standalone at > this point, and instead would like to see reports from the end-users > produced by a lets-start-small version of the tool and measure how > having the tool actually helps. > > Thanks. >
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> I actually have a suspicion that "git bugreport" that is spawned via >> "git" wrapper is a bad idea (in other words, /usr/bin/git-bug that >> is totally standalone may be better). > > The obvious downside of `/usr/bin/git-bug`, of course, is that it has no > way to provide accurate data regarding, say, the cURL version in use. Sorry, but I do not see what's new in your argument this time. I thought we've already established that the best solution for the "accurate data regarding, say, the cURL version in use" is to use your earlier idea, i.e. give an interface to git-remote-curl so that "git bugreport" can ask it such details, because "git bugreport" that is known by git.c::cmd_main(), whether it is builtin or standalone, is *NOT* linked to the transport anyway. And the same interface can be used by an independent "git-bug", or even by the end user who is sitting at a terminal when asked by git developers "what version of curl library does your build link with?".
Hi Junio, On Fri, 20 Mar 2020, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > >> I actually have a suspicion that "git bugreport" that is spawned via > >> "git" wrapper is a bad idea (in other words, /usr/bin/git-bug that > >> is totally standalone may be better). > > > > The obvious downside of `/usr/bin/git-bug`, of course, is that it has no > > way to provide accurate data regarding, say, the cURL version in use. > > Sorry, but I do not see what's new in your argument this time. > > I thought we've already established that the best solution for the > "accurate data regarding, say, the cURL version in use" is to use > your earlier idea, i.e. give an interface to git-remote-curl so that > "git bugreport" can ask it such details, because "git bugreport" > that is known by git.c::cmd_main(), whether it is builtin or > standalone, is *NOT* linked to the transport anyway. > > And the same interface can be used by an independent "git-bug", or > even by the end user who is sitting at a terminal when asked by git > developers "what version of curl library does your build link > with?". You are _asking_ for version mismatch here. If `git-bug` is a totally standalone program, then it cannot rely at all on `git` to give it the information it wants. Forward compatibility becomes another concern. In other words, all the avoidably incurred complexity will come back to make the entire `git bugreport` not worth all the effort Emily put into it. Ciao, Dscho
On Fri, Mar 20, 2020 at 04:42:36PM +0100, Johannes Schindelin wrote: > Hi Emily, > > On Thu, 19 Mar 2020, Emily Shaffer wrote: > > I'll hold off on sharing my own preference until after we've got some > > benchmarking to look at, so I can understand the whole picture. > > Why? Why not strong-arm your preference? Junio and I are not shy doing the > same, and those are _your_ patches. Junio is clearly not interested in the > command at all, but you are clearly interested in it [*1*]. You should > have more than just the final say over this. We are different people, and we approach problems and disagreements differently. I am almost never the type to strong-arm my preference; I'd rather listen to other arguments and look at data, which is why I proposed the experiments I mentioned here. As for you and Junio not being shy, as you put it, I found most of the discussion offputting, and the tone used absolutely contributed to me finding other things to work on instead of coming back to this minefield sooner. - Emily
On Fri, Mar 20, 2020 at 04:35:45PM +0100, Johannes Schindelin wrote: > On Thu, 19 Mar 2020, Junio C Hamano wrote: > > > In any case, I think we've wasted enough time on this. Let's see a > > minimum working version that won't break/affect the rest of Git and > > ship it standalone. > > I still disagree with that, but it seems that no amount of arguments will > convince Junio, and he's dead set on the standalone version. In this case, I'll leave the build settings alone and focus on other comments on this patch. Will try to work on it today. - Emily
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
Teach Git how to prompt the user for a good bug report: reproduction steps, expected behavior, and actual behavior. Later, Git can learn how to collect some diagnostic information from the repository. If users can send us a well-written bug report which contains diagnostic information we would otherwise need to ask the user for, we can reduce the number of question-and-answer round trips between the reporter and the Git contributor. Users may also wish to send a report like this to their local "Git expert" if they have put their repository into a state they are confused by. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- .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