Re: [PATCH] bugreport: add tool to generate debugging info

> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
> > new file mode 100644
> > index 0000000000..c5f45bbee8
> > --- /dev/null
> > +++ b/Documentation/git-bugreport.txt
> > @@ -0,0 +1,48 @@
> > +git-bugreport(1)
> > +================
> > +
> > +NAME
> > +----
> > +git-bugreport - Collect information for user to file a bug report
> > +
> > +SYNOPSIS
> > +--------
> > +[verse]
> > +'git bugreport' [-o | --output <path>]
> > +
> > +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
> 
> + - How the above two are different
> 
> It is often helpful to have users explain how the expected and
> actual are different in their own words.

> 
> > +NOTE
> > +----
> > +Bug reports can be sent to git@vger.kernel.org.
> 
> I am not sure if this belongs here.

> 
> > diff --git a/git-bugreport.sh b/git-bugreport.sh
> > new file mode 100755
> > index 0000000000..2200703a51
> > --- /dev/null
> > +++ b/git-bugreport.sh
> > @@ -0,0 +1,86 @@
> > +#!/bin/sh
> > +
> > +print_filenames_and_content() {
> > +while read -r line; do
> > +	echo "$line";
> > +	echo "========";
> > +	cat "$line";
> > +	echo;
> > +done
> > +}
> 
> Style.
> 
>  - have SP on both sides of ()
>  - one more HT indent for the function body
>  - "do" on its own line
>  - no unnecessary semicolons when LF would do
> 
> You probably are better off asking xargs to do this instead of
> relying on "read -r".
> 
> 	find ... | xargs -n 1 sh -c 'echo "$1" && cat "$1"' -
> 
> or something like that, perhaps.

> 
> > +
> > +generate_report_text() {
> > +
> > +	# Generate a form for the user to fill out with their issue.
> > +	gettextln "Thank you for filling out a Git bug report!"
> > +	gettextln "Please answer the following questions to help us understand your issue."
> > +	echo
> > +	gettextln "What did you do before the bug happened? (Steps to reproduce your issue)"
> > +	echo
> > +	gettextln "What did you expect to happen? (Expected behavior)"
> > +	echo
> > +	gettextln "What happened instead? (Actual behavior)"
> > +	echo
> > +	gettextln "Anything else you want to add:"
> > +	echo
> > +	gettextln "Please review the rest of the bug report below."
> > +	gettextln "You can delete any lines you don't wish to send."
> > +	echo
> 
> Would we on the receiving end be able to tell these section headers
> in translated to 47 different languages?  I am sure that i18n is
> used here to encourage non-C-locale users to file bugs in their own
> languages, but are we prepared to react to them?

> 
> > +	echo "[System Information]"
> > +	git version --build-options
> > +	uname -a
> > +	curl-config --version
> > +	ldd --version
> 
> "curl-config: command not found" may be clear enough, but would
> there be a case where errors from two or more consecutive commands
> in the above sequence would make the output confusing to the person
> sitting on the receiving end?  Would it help, as a convention, to
> always ahve "echo [what am I reporting]" before each of these
> commands?

> 
> > +# Create bugreport file
> > +BUGREPORT_FILE="$basedir/git-bugreport-$(whoami)-$(hostname)-$(date -Iminutes)"
> 
> How portable is -Iminutes?

> 
> > diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
> > new file mode 100755
> > index 0000000000..6eb2ee4f66
> > --- /dev/null
> > +++ b/t/t0091-bugreport.sh
> > @@ -0,0 +1,41 @@
> > +#!/bin/bash
> 
> Make sure /bin/sh suffices to run the test script.

> 
> > +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 [$(grep $HEADER_PATTERN $line)]; then
> 
> Documentation/CodingGuidelines
> 
> I am not sure if the traits this test script checks about the
> contents of the output is all that interesting.  Whenever we add new
> sections to the main command because we want other kinds of
> information collected, we'd update this test script because
> otherwise the test would fail, but would it result in quality
> bugreport tool, or is it just additional busywork?

> 
> If we decide later that a header and its body needs to be separated
> with a blank line for better readablity, the check done here would
> also need to be updated, but again, that does not feel like anything
> more than just busywork to me.