Re: [PATCH] bugreport: add tool to generate debugging info
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 15, 2019, 18:10 UTC
- Message-ID
- <xmqqzhka2tbv.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190815023418.33407-1-emilyshaffer@google.com>
Emily Shaffer <emilyshaffer@google.com> writes:
Show 30 quoted lines
> 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.
Show 16 quoted lines
> 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.
Show 18 quoted lines
> +
> +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."
> + echoWould 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?
Show 5 quoted lines
> + 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?
Show 7 quoted lines
> 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.
Show 11 quoted lines
> +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)]; thenDocumentation/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.
The tests for "-o" and unknown options do make sense, though.
Thanks.
Show 35 quoted lines
> +# 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
> + read -r nextline
> + if [-z $nextline]; then
> + return 1;
> + fi
> + fi
> + done
> +}
> +
> +test_expect_success 'creates a report with content in the right places' '
> + git bugreport &&
> + check_all_headers_populated <git-bugreport-* &&
> + rm git-bugreport-*
> +'
> +
> +test_expect_success '--output puts the report in the provided dir' '
> + mkdir foo/ &&
> + git bugreport -o foo/ &&
> + test -f foo/git-bugreport-* &&
> + rm -fr foo/
> +'
> +
> +test_expect_success 'incorrect arguments abort with usage' '
> + test_must_fail git bugreport --false 2>output &&
> + grep usage output &&
> + test ! -f git-bugreport-*
> +'
> +
> +test_done