Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Derrick Stolee <stolee@gmail.com>
To: Emily Shaffer <emilyshaffer@google.com>, git@vger.kernel.org
Subject: Re: [PATCH] bugreport: add tool to generate debugging info
Date: Thu, 15 Aug 2019 10:15:24 -0400
Message-ID: <e6d56d97-99c9-064a-71b5-2b7eb9b71e01@gmail.com> (raw)
In-Reply-To: <20190815023418.33407-1-emilyshaffer@google.com>

On 8/14/2019 10:34 PM, Emily Shaffer wrote:
> Make it easier for users who encounter a bug to send a report by
> collecting some state information, intended to be viewed by humans
> familiar with Git.

This is an excellent idea! VFS for Git has a similar "diagnose" command
that collects logs, config, and other details (like packfile sizes and
loose object counts). That has been a critical tool for supporting users.

> Teach Git how to prompt the user for a good bug report: reproduction
> steps, expected behavior, and actual behavior. Also, teach Git to write
> down its own version, the version of some of its dependencies, the
> operating system information, the effective gitconfig, the configured
> hooks, and the contents of $GIT_DIR/logs. Finally, make sure Git asks
> the user to review the contents of the report after it's generated.
> 
> If users can send us a well-written bug report complete with system
> information, a remote we may be able to clone, and a reflog showing us
> where the problem occurred, we can reduce the number of questions and
> answers which must travel 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>
> ---
> There are some things I'm not certain about from this patch, I'd
> appreciate feedback.
> 
>  - Do we want to advertise the Git mailing list for bug reports?

That is possible. Isn't there another mailing list for git users?

I could see a patch added on top of this for git-for-windows/git that
changes the instructions to create issues on GitHub.

>  - Which config options should we filter to avoid accidentally receiving
>    credentials?

The remote URLs are pretty sensitive. Not only do users sometimes put passwords
or PATs into their URLs, the literal name of the repo could be a secret.

>  - At the moment, the test is trying to check "everything we thought we
>    would populate got populated" - that seemed to me like it would hold
>    up best to changes to the set of information being collected. But
>    maybe there's something more robust we can do.
> 
> And of course, if there are suggestions for other things to include in
> the report I'm interested in adding.
> 
> An example of the output can be found here:
> https://gist.github.com/nasamuffin/2e320892f5c2147e829cbcf5bd0759a2
> 
> Thanks.
>  - Emily
> 
>  .gitignore                      |  1 +
>  Documentation/git-bugreport.txt | 48 ++++++++++++++++++
>  Makefile                        |  1 +
>  command-list.txt                |  1 +
>  git-bugreport.sh                | 86 +++++++++++++++++++++++++++++++++
>  t/t0091-bugreport.sh            | 41 ++++++++++++++++
>  6 files changed, 178 insertions(+)
>  create mode 100644 Documentation/git-bugreport.txt
>  create mode 100755 git-bugreport.sh
>  create mode 100755 t/t0091-bugreport.sh
> 
> diff --git a/.gitignore b/.gitignore
> index 521d8f4fb4..b4f5433084 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..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
> +
> +The following information is captured automatically:
> +
> + - Git version (`git version --build-options`)
> + - Machine information (`uname -a`)
> + - Versions of various dependencies
> + - Git config contents (`git config --show-origin --list`)
> + - The contents of all configured git-hooks in `.git/hooks/`
> + - The contents of `.git/logs`
> +
> +OPTIONS
> +-------
> +-o [<path>]::
> +--output [<path>]::
> +	Place the resulting bug report file in <path> instead of the root of the
> +	Git repository.
> +
> +NOTE
> +----
> +Bug reports can be sent to git@vger.kernel.org.
> +
> +GIT
> +---
> +Part of the linkgit:git[1] suite
> diff --git a/Makefile b/Makefile
> index f9255344ae..69801a1c45 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -606,6 +606,7 @@ TEST_PROGRAMS_NEED_X =
>  unexport CDPATH
>  
>  SCRIPT_SH += git-bisect.sh
> +SCRIPT_SH += git-bugreport.sh
>  SCRIPT_SH += git-difftool--helper.sh
>  SCRIPT_SH += git-filter-branch.sh
>  SCRIPT_SH += git-merge-octopus.sh
> diff --git a/command-list.txt b/command-list.txt
> index a9ac72bef4..be5a605047 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/git-bugreport.sh b/git-bugreport.sh
> new file mode 100755
> index 0000000000..2200703a51
> --- /dev/null
> +++ b/git-bugreport.sh

At first I was alarmed by "What? another shell script?" but this command should
prioritize flexibility and extensibility over speed. Running multiple processes
shouldn't be too taxing for what we are trying to do here.

> @@ -0,0 +1,86 @@
> +#!/bin/sh
> +
> +print_filenames_and_content() {
> +while read -r line; do
> +	echo "$line";
> +	echo "========";
> +	cat "$line";
> +	echo;
> +done
> +}
> +
> +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
> +
> +	echo "[System Information]"
> +	git version --build-options
> +	uname -a
> +	curl-config --version
> +	ldd --version
> +	echo
> +
> +	echo "[Git Config]"
> +	# TODO: Pass this through grep -v to avoid users sending us their credentials.
> +	git config --show-origin --list
> +	echo

Config options to consider stripping out:

	*url*
	*pass* (anything "password" but also "sendmail.smtppass")

> +	echo "[Configured Hooks]"
> +	find "$GIT_DIR/hooks/" -type f | grep -v "\.sample$" | print_filenames_and_content
> +	echo

Remove the sample hooks, but focus on the others. Will this look like garbage if a hook
is a binary file?

> +
> +	echo "[Git Logs]"
> +	find "$GIT_DIR/logs" -type f | print_filenames_and_content
> +	echo

As mentioned before, I've sometimes found it helpful to know the data shape for the object
store. Having a few extra steps such as the following could be nice:

	echo "[Loose Objects]"
	for objdir in $(find "$GIT_DIR/objects/??" -type d)
	do
		echo "$objdir: $(ls $objdir | wc -l)"
	done
	echo

	echo "[Pack Data]"
	ls -l "$GIT_DIR/objects/pack"
	echo

	echo "[Object Info Data]"
	ls -lR "$GIT_DIR/objects/info"
	echo

	echo "[Alternates File]"
	echo "========"
	cat "$GIT_DIR/objects/info/alternates"
	echo

That last one will collect information on the commit-graph file, even if it is an
incremental file.

> +
> +}
> +
> +USAGE="[-o | --output <path>]"
> +
> +SUBDIRECTORY_OK=t
> +OPTIONS_SPEC=
> +. git-sh-setup
> +. git-sh-i18n
> +
> +basedir="$PWD"
> +while :
> +do
> +	case "$1" in
> +	-o|--output)
> +		shift
> +		basedir="$1"
> +		shift
> +		continue
> +		;;
> +	"")
> +		break
> +		;;
> +	*)
> +		usage
> +		;;
> +	esac
> +done
> +
> +
> +# Create bugreport file
> +BUGREPORT_FILE="$basedir/git-bugreport-$(whoami)-$(hostname)-$(date -Iminutes)"
> +
> +generate_report_text >$BUGREPORT_FILE
> +
> +git_editor $BUGREPORT_FILE
> +
> +eval_gettextln "Your new bug report is in \$BUGREPORT_FILE."
> 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
> +
> +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
> +			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
> 

I think this is a great start, and I'll take some time later to try it out.

Thanks,
-Stolee

  reply index

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-15  2:34 Emily Shaffer
2019-08-15 14:15 ` Derrick Stolee [this message]
2019-08-15 14:36   ` Junio C Hamano
2019-08-15 22:52     ` Emily Shaffer
2019-08-15 23:40       ` Junio C Hamano
2019-08-16  1:25         ` Emily Shaffer
2019-08-16 16:41           ` Junio C Hamano
2019-08-16 19:08             ` Emily Shaffer
2019-08-15 20:07   ` Johannes Schindelin
2019-08-15 22:24     ` Emily Shaffer
2019-08-16 20:19       ` Johannes Schindelin
2019-08-15 20:13   ` Emily Shaffer
2019-08-15 18:10 ` Junio C Hamano
2019-08-15 21:52   ` Emily Shaffer
2019-08-15 22:29     ` Junio C Hamano
2019-08-15 22:54       ` Emily Shaffer
2019-08-17  0:39 [PATCH v2 0/2] add git-bugreport tool Emily Shaffer
2019-08-17  0:39 ` [PATCH v2 1/2] bugreport: add tool to generate debugging info Emily Shaffer
2019-10-25  2:51 ` [PATCH v3 0/9] add git-bugreport tool Emily Shaffer
2019-10-25  2:51   ` [PATCH v3 1/9] bugreport: add tool to generate debugging info Emily Shaffer
2019-10-29 20:29     ` Josh Steadmon
2019-11-16  3:11     ` Junio C Hamano
2019-11-19 20:25       ` Emily Shaffer
2019-11-19 23:24         ` Johannes Schindelin
2019-11-20  0:37           ` Junio C Hamano
2019-11-20 10:51             ` Johannes Schindelin
2019-11-19 23:31         ` Johannes Schindelin
2019-11-20  0:39           ` Junio C Hamano
2019-11-20  2:09           ` Emily Shaffer
2019-11-20  0:32         ` Junio C Hamano
2019-10-25  2:51   ` [PATCH v3 2/9] bugreport: generate config whitelist based on docs Emily Shaffer
2019-10-28 13:27     ` Johannes Schindelin
2019-10-25  2:51   ` [PATCH v3 3/9] bugreport: add version and system information Emily Shaffer
2019-10-28 13:49     ` Johannes Schindelin
2019-11-08 21:48       ` Emily Shaffer
2019-11-11 13:48         ` Johannes Schindelin
2019-11-14 21:42           ` Emily Shaffer
2019-10-29 20:43     ` Josh Steadmon
2019-10-25  2:51   ` [PATCH v3 4/9] bugreport: add config values from whitelist Emily Shaffer
2019-10-28 14:14     ` Johannes Schindelin
2019-12-11 20:48       ` Emily Shaffer
2019-12-15 17:30         ` Johannes Schindelin
2019-10-29 20:58     ` Josh Steadmon
2019-10-30  1:37       ` Junio C Hamano
2019-11-14 21:55         ` Emily Shaffer
2019-10-25  2:51   ` [PATCH v3 5/9] bugreport: collect list of populated hooks Emily Shaffer
2019-10-28 14:31     ` Johannes Schindelin
2019-12-11 20:51       ` Emily Shaffer
2019-12-15 17:40         ` Johannes Schindelin
2019-10-25  2:51   ` [PATCH v3 6/9] bugreport: count loose objects Emily Shaffer
2019-10-28 15:07     ` Johannes Schindelin
2019-12-10 22:34       ` Emily Shaffer
2019-10-29 21:18     ` Josh Steadmon
2019-10-25  2:51   ` [PATCH v3 7/9] bugreport: add packed object summary Emily Shaffer
2019-10-28 15:43     ` Johannes Schindelin
2019-12-11  0:29       ` Emily Shaffer
2019-12-11 13:37         ` Johannes Schindelin
2019-12-11 20:52           ` Emily Shaffer
2019-10-25  2:51   ` [PATCH v3 8/9] bugreport: list contents of $OBJDIR/info Emily Shaffer
2019-10-28 15:51     ` Johannes Schindelin
2019-10-25  2:51   ` [PATCH v3 9/9] bugreport: print contents of alternates file Emily Shaffer
2019-10-28 15:57     ` Johannes Schindelin
2019-11-19 20:40       ` Emily Shaffer
2019-10-29  1:54   ` [PATCH v3 0/9] add git-bugreport tool Junio C Hamano
2019-10-29 11:13     ` Johannes Schindelin
2019-08-17  0:39 [PATCH v2 2/2] bugreport: generate config whitelist based on docs Emily Shaffer
2019-08-17 20:38 ` Martin Ågren
2019-08-21 17:40   ` Emily Shaffer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e6d56d97-99c9-064a-71b5-2b7eb9b71e01@gmail.com \
    --to=stolee@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git