git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v10 0/5] add git-bugreport tool
@ 2020-03-23 21:43 Emily Shaffer
  2020-03-23 21:43 ` [PATCH v10 1/5] help: move list_config_help to builtin/help Emily Shaffer
                   ` (5 more replies)
  0 siblings, 6 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-23 21:43 UTC (permalink / raw)
  To: git
  Cc: Emily Shaffer, Derrick Stolee, Johannes Schindelin,
	Junio C Hamano, Martin Ågren, Aaron Schrab, Danh Doan,
	Eric Sunshine, SZEDERGábor, Andreas Schwab

Since v9, a few minor fixes only related to MS and clang compatibility,
plus a warning about strbuf_addf vs strbuf_addstr. I have made no
changes to the build setup for v10, so it's still built as a standalone
which is invoked via the cmd_main helper. There is plenty of time to
change the way we build it later if we want, but I generally agree with
the sentiment "let's get something checked in".

GitGitGadget CI is running at:
https://github.com/gitgitgadget/git/pull/573

 - Emily

Emily Shaffer (5):
  help: move list_config_help to builtin/help
  bugreport: add tool to generate debugging info
  bugreport: gather git version and build info
  bugreport: add uname info
  bugreport: add compiler info

 .gitignore                      |   2 +
 Documentation/git-bugreport.txt |  52 ++++++++++++
 Makefile                        |  18 +++-
 bugreport.c                     | 142 ++++++++++++++++++++++++++++++++
 builtin/help.c                  |  86 +++++++++++++++++++
 command-list.txt                |   1 +
 compat/compiler.h               |  38 +++++++++
 generate-cmdlist.sh             |  19 -----
 generate-configlist.sh          |  21 +++++
 help.c                          | 131 +++++++----------------------
 help.h                          |   2 +-
 strbuf.c                        |   4 +
 strbuf.h                        |   1 +
 t/t0091-bugreport.sh            |  61 ++++++++++++++
 14 files changed, 452 insertions(+), 126 deletions(-)
 create mode 100644 Documentation/git-bugreport.txt
 create mode 100644 bugreport.c
 create mode 100644 compat/compiler.h
 create mode 100755 generate-configlist.sh
 create mode 100755 t/t0091-bugreport.sh

-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v10 1/5] help: move list_config_help to builtin/help
  2020-03-23 21:43 [PATCH v10 0/5] add git-bugreport tool Emily Shaffer
@ 2020-03-23 21:43 ` Emily Shaffer
  2020-03-23 21:43 ` [PATCH v10 2/5] bugreport: add tool to generate debugging info Emily Shaffer
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-23 21:43 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

Starting in 3ac68a93fd2, help.o began to depend on builtin/branch.o,
builtin/clean.o, and builtin/config.o. This meant that help.o was
unusable outside of the context of the main Git executable.

To make help.o usable by other commands again, move list_config_help()
into builtin/help.c (where it makes sense to assume other builtin libraries
are present).

When command-list.h is included but a member is not used, we start to
hear a compiler warning. Since the config list is generated in a fairly
different way than the command list, and since commands and config
options are semantically different, move the config list into its own
header and move the generator into its own script and build rule.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 .gitignore             |  1 +
 Makefile               | 13 +++++--
 builtin/help.c         | 86 ++++++++++++++++++++++++++++++++++++++++++
 generate-cmdlist.sh    | 19 ----------
 generate-configlist.sh | 21 +++++++++++
 help.c                 | 85 -----------------------------------------
 help.h                 |  1 -
 7 files changed, 118 insertions(+), 108 deletions(-)
 create mode 100755 generate-configlist.sh

diff --git a/.gitignore b/.gitignore
index aebe7c0908..ea97de83f3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -189,6 +189,7 @@
 /gitweb/gitweb.cgi
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
+/config-list.h
 /command-list.h
 *.tar.gz
 *.dsc
diff --git a/Makefile b/Makefile
index 6134104ae6..c552312d3f 100644
--- a/Makefile
+++ b/Makefile
@@ -815,6 +815,7 @@ LIB_FILE = libgit.a
 XDIFF_LIB = xdiff/lib.a
 VCSSVN_LIB = vcs-svn/lib.a
 
+GENERATED_H += config-list.h
 GENERATED_H += command-list.h
 
 LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
@@ -2132,7 +2133,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 
 help.sp help.s help.o: command-list.h
 
-builtin/help.sp builtin/help.s builtin/help.o: command-list.h GIT-PREFIX
+builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 	'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
@@ -2152,6 +2153,12 @@ $(BUILT_INS): git$X
 	ln -s $< $@ 2>/dev/null || \
 	cp $< $@
 
+config-list.h: generate-configlist.sh
+
+config-list.h:
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \
+		>$@+ && mv $@+ $@
+
 command-list.h: generate-cmdlist.sh command-list.txt
 
 command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
@@ -2785,7 +2792,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
 .PHONY: sparse $(SP_OBJ)
 sparse: $(SP_OBJ)
 
-EXCEPT_HDRS := command-list.h unicode-width.h compat/% xdiff/%
+EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%
 ifndef GCRYPT_SHA256
 	EXCEPT_HDRS += sha256/gcrypt.h
 endif
@@ -2807,7 +2814,7 @@ hdr-check: $(HCO)
 style:
 	git clang-format --style file --diff --extensions c,h
 
-check: command-list.h
+check: config-list.h command-list.h
 	@if sparse; \
 	then \
 		echo >&2 "Use 'make sparse' instead"; \
diff --git a/builtin/help.c b/builtin/help.c
index e5590d7787..1c5f2b9255 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -8,6 +8,7 @@
 #include "parse-options.h"
 #include "run-command.h"
 #include "column.h"
+#include "config-list.h"
 #include "help.h"
 #include "alias.h"
 
@@ -62,6 +63,91 @@ static const char * const builtin_help_usage[] = {
 	NULL
 };
 
+struct slot_expansion {
+	const char *prefix;
+	const char *placeholder;
+	void (*fn)(struct string_list *list, const char *prefix);
+	int found;
+};
+
+static void list_config_help(int for_human)
+{
+	struct slot_expansion slot_expansions[] = {
+		{ "advice", "*", list_config_advices },
+		{ "color.branch", "<slot>", list_config_color_branch_slots },
+		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
+		{ "color.diff", "<slot>", list_config_color_diff_slots },
+		{ "color.grep", "<slot>", list_config_color_grep_slots },
+		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
+		{ "color.remote", "<slot>", list_config_color_sideband_slots },
+		{ "color.status", "<slot>", list_config_color_status_slots },
+		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
+		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
+		{ NULL, NULL, NULL }
+	};
+	const char **p;
+	struct slot_expansion *e;
+	struct string_list keys = STRING_LIST_INIT_DUP;
+	int i;
+
+	for (p = config_name_list; *p; p++) {
+		const char *var = *p;
+		struct strbuf sb = STRBUF_INIT;
+
+		for (e = slot_expansions; e->prefix; e++) {
+
+			strbuf_reset(&sb);
+			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
+			if (!strcasecmp(var, sb.buf)) {
+				e->fn(&keys, e->prefix);
+				e->found++;
+				break;
+			}
+		}
+		strbuf_release(&sb);
+		if (!e->prefix)
+			string_list_append(&keys, var);
+	}
+
+	for (e = slot_expansions; e->prefix; e++)
+		if (!e->found)
+			BUG("slot_expansion %s.%s is not used",
+			    e->prefix, e->placeholder);
+
+	string_list_sort(&keys);
+	for (i = 0; i < keys.nr; i++) {
+		const char *var = keys.items[i].string;
+		const char *wildcard, *tag, *cut;
+
+		if (for_human) {
+			puts(var);
+			continue;
+		}
+
+		wildcard = strchr(var, '*');
+		tag = strchr(var, '<');
+
+		if (!wildcard && !tag) {
+			puts(var);
+			continue;
+		}
+
+		if (wildcard && !tag)
+			cut = wildcard;
+		else if (!wildcard && tag)
+			cut = tag;
+		else
+			cut = wildcard < tag ? wildcard : tag;
+
+		/*
+		 * We may produce duplicates, but that's up to
+		 * git-completion.bash to handle
+		 */
+		printf("%.*s\n", (int)(cut - var), var);
+	}
+	string_list_clear(&keys, 0);
+}
+
 static enum help_format parse_help_format(const char *format)
 {
 	if (!strcmp(format, "man"))
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 71158f7d8b..45fecf8bdf 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -76,23 +76,6 @@ print_command_list () {
 	echo "};"
 }
 
-print_config_list () {
-	cat <<EOF
-static const char *config_name_list[] = {
-EOF
-	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
-	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
-	sort |
-	while read line
-	do
-		echo "	\"$line\","
-	done
-	cat <<EOF
-	NULL,
-};
-EOF
-}
-
 exclude_programs=
 while test "--exclude-program" = "$1"
 do
@@ -113,5 +96,3 @@ echo
 define_category_names "$1"
 echo
 print_command_list "$1"
-echo
-print_config_list
diff --git a/generate-configlist.sh b/generate-configlist.sh
new file mode 100755
index 0000000000..8692fe5cf4
--- /dev/null
+++ b/generate-configlist.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+echo "/* Automatically generated by generate-configlist.sh */"
+echo
+
+print_config_list () {
+	cat <<EOF
+static const char *config_name_list[] = {
+EOF
+	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
+	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
+	sort |
+	sed 's/^.*$/	"&",/'
+	cat <<EOF
+	NULL,
+};
+EOF
+}
+
+echo
+print_config_list
diff --git a/help.c b/help.c
index cf67624a94..a21487db77 100644
--- a/help.c
+++ b/help.c
@@ -407,91 +407,6 @@ void list_common_guides_help(void)
 	putchar('\n');
 }
 
-struct slot_expansion {
-	const char *prefix;
-	const char *placeholder;
-	void (*fn)(struct string_list *list, const char *prefix);
-	int found;
-};
-
-void list_config_help(int for_human)
-{
-	struct slot_expansion slot_expansions[] = {
-		{ "advice", "*", list_config_advices },
-		{ "color.branch", "<slot>", list_config_color_branch_slots },
-		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
-		{ "color.diff", "<slot>", list_config_color_diff_slots },
-		{ "color.grep", "<slot>", list_config_color_grep_slots },
-		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
-		{ "color.remote", "<slot>", list_config_color_sideband_slots },
-		{ "color.status", "<slot>", list_config_color_status_slots },
-		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
-		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
-		{ NULL, NULL, NULL }
-	};
-	const char **p;
-	struct slot_expansion *e;
-	struct string_list keys = STRING_LIST_INIT_DUP;
-	int i;
-
-	for (p = config_name_list; *p; p++) {
-		const char *var = *p;
-		struct strbuf sb = STRBUF_INIT;
-
-		for (e = slot_expansions; e->prefix; e++) {
-
-			strbuf_reset(&sb);
-			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
-			if (!strcasecmp(var, sb.buf)) {
-				e->fn(&keys, e->prefix);
-				e->found++;
-				break;
-			}
-		}
-		strbuf_release(&sb);
-		if (!e->prefix)
-			string_list_append(&keys, var);
-	}
-
-	for (e = slot_expansions; e->prefix; e++)
-		if (!e->found)
-			BUG("slot_expansion %s.%s is not used",
-			    e->prefix, e->placeholder);
-
-	string_list_sort(&keys);
-	for (i = 0; i < keys.nr; i++) {
-		const char *var = keys.items[i].string;
-		const char *wildcard, *tag, *cut;
-
-		if (for_human) {
-			puts(var);
-			continue;
-		}
-
-		wildcard = strchr(var, '*');
-		tag = strchr(var, '<');
-
-		if (!wildcard && !tag) {
-			puts(var);
-			continue;
-		}
-
-		if (wildcard && !tag)
-			cut = wildcard;
-		else if (!wildcard && tag)
-			cut = tag;
-		else
-			cut = wildcard < tag ? wildcard : tag;
-
-		/*
-		 * We may produce duplicates, but that's up to
-		 * git-completion.bash to handle
-		 */
-		printf("%.*s\n", (int)(cut - var), var);
-	}
-	string_list_clear(&keys, 0);
-}
-
 static int get_alias(const char *var, const char *value, void *data)
 {
 	struct string_list *list = data;
diff --git a/help.h b/help.h
index 7a455beeb7..9071894e8c 100644
--- a/help.h
+++ b/help.h
@@ -22,7 +22,6 @@ static inline void mput_char(char c, unsigned int num)
 void list_common_cmds_help(void);
 void list_all_cmds_help(void);
 void list_common_guides_help(void);
-void list_config_help(int for_human);
 
 void list_all_main_cmds(struct string_list *list);
 void list_all_other_cmds(struct string_list *list);
-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v10 2/5] bugreport: add tool to generate debugging info
  2020-03-23 21:43 [PATCH v10 0/5] add git-bugreport tool Emily Shaffer
  2020-03-23 21:43 ` [PATCH v10 1/5] help: move list_config_help to builtin/help Emily Shaffer
@ 2020-03-23 21:43 ` Emily Shaffer
  2020-03-23 21:43 ` [PATCH v10 3/5] bugreport: gather git version and build info Emily Shaffer
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-23 21:43 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

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

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..f6f53a5e8e
--- /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 = "%Y-%m-%d-%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.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v10 3/5] bugreport: gather git version and build info
  2020-03-23 21:43 [PATCH v10 0/5] add git-bugreport tool Emily Shaffer
  2020-03-23 21:43 ` [PATCH v10 1/5] help: move list_config_help to builtin/help Emily Shaffer
  2020-03-23 21:43 ` [PATCH v10 2/5] bugreport: add tool to generate debugging info Emily Shaffer
@ 2020-03-23 21:43 ` Emily Shaffer
  2020-03-23 21:43 ` [PATCH v10 4/5] bugreport: add uname info Emily Shaffer
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-23 21:43 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

Knowing which version of Git a user has and how it was built allows us
to more precisely pin down the circumstances when a certain issue
occurs, so teach bugreport how to tell us the same output as 'git
version --build-options'.

It's not ideal to directly call 'git version --build-options' because
that output goes to stdout. Instead, wrap the version string in a helper
within help.[ch] library, and call that helper from within the bugreport
library.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-bugreport.txt |  4 +++
 bugreport.c                     | 19 +++++++++++++-
 help.c                          | 46 ++++++++++++++++++++-------------
 help.h                          |  1 +
 4 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index 1f9fde5cde..f44ae8cbe7 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -23,6 +23,10 @@ The following information is requested from the user:
  - Expected behavior
  - Actual behavior
 
+The following information is captured automatically:
+
+ - 'git version --build-options'
+
 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
diff --git a/bugreport.c b/bugreport.c
index f6f53a5e8e..4cdb58bbaa 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -1,8 +1,17 @@
-#include "builtin.h"
+#include "cache.h"
 #include "parse-options.h"
 #include "stdio.h"
 #include "strbuf.h"
 #include "time.h"
+#include "help.h"
+
+static void get_system_info(struct strbuf *sys_info)
+{
+	/* get git version from native cmd */
+	strbuf_addstr(sys_info, _("git version:\n"));
+	get_version_info(sys_info, 1);
+	strbuf_complete_line(sys_info);
+}
 
 static const char * const bugreport_usage[] = {
 	N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
@@ -32,6 +41,11 @@ static int get_bug_template(struct strbuf *template)
 	return 0;
 }
 
+static void get_header(struct strbuf *buf, const char *title)
+{
+	strbuf_addf(buf, "\n\n[%s]\n", title);
+}
+
 int cmd_main(int argc, const char **argv)
 {
 	struct strbuf buffer = STRBUF_INIT;
@@ -79,6 +93,9 @@ int cmd_main(int argc, const char **argv)
 	/* Prepare the report contents */
 	get_bug_template(&buffer);
 
+	get_header(&buffer, _("System Info"));
+	get_system_info(&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);
 
diff --git a/help.c b/help.c
index a21487db77..1de9c0d589 100644
--- a/help.c
+++ b/help.c
@@ -622,8 +622,32 @@ const char *help_unknown_cmd(const char *cmd)
 	exit(1);
 }
 
+void get_version_info(struct strbuf *buf, int show_build_options)
+{
+	/*
+	 * The format of this string should be kept stable for compatibility
+	 * with external projects that rely on the output of "git version".
+	 *
+	 * Always show the version, even if other options are given.
+	 */
+	strbuf_addf(buf, "git version %s\n", git_version_string);
+
+	if (show_build_options) {
+		strbuf_addf(buf, "cpu: %s\n", GIT_HOST_CPU);
+		if (git_built_from_commit_string[0])
+			strbuf_addf(buf, "built from commit: %s\n",
+			       git_built_from_commit_string);
+		else
+			strbuf_addstr(buf, "no commit associated with this build\n");
+		strbuf_addf(buf, "sizeof-long: %d\n", (int)sizeof(long));
+		strbuf_addf(buf, "sizeof-size_t: %d\n", (int)sizeof(size_t));
+		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
+	}
+}
+
 int cmd_version(int argc, const char **argv, const char *prefix)
 {
+	struct strbuf buf = STRBUF_INIT;
 	int build_options = 0;
 	const char * const usage[] = {
 		N_("git version [<options>]"),
@@ -637,25 +661,11 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
-	/*
-	 * The format of this string should be kept stable for compatibility
-	 * with external projects that rely on the output of "git version".
-	 *
-	 * Always show the version, even if other options are given.
-	 */
-	printf("git version %s\n", git_version_string);
+	get_version_info(&buf, build_options);
+	printf("%s", buf.buf);
+
+	strbuf_release(&buf);
 
-	if (build_options) {
-		printf("cpu: %s\n", GIT_HOST_CPU);
-		if (git_built_from_commit_string[0])
-			printf("built from commit: %s\n",
-			       git_built_from_commit_string);
-		else
-			printf("no commit associated with this build\n");
-		printf("sizeof-long: %d\n", (int)sizeof(long));
-		printf("sizeof-size_t: %d\n", (int)sizeof(size_t));
-		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
-	}
 	return 0;
 }
 
diff --git a/help.h b/help.h
index 9071894e8c..500521b908 100644
--- a/help.h
+++ b/help.h
@@ -37,6 +37,7 @@ void add_cmdname(struct cmdnames *cmds, const char *name, int len);
 void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes);
 int is_in_cmdlist(struct cmdnames *cmds, const char *name);
 void list_commands(unsigned int colopts, struct cmdnames *main_cmds, struct cmdnames *other_cmds);
+void get_version_info(struct strbuf *buf, int show_build_options);
 
 /*
  * call this to die(), when it is suspected that the user mistyped a
-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v10 4/5] bugreport: add uname info
  2020-03-23 21:43 [PATCH v10 0/5] add git-bugreport tool Emily Shaffer
                   ` (2 preceding siblings ...)
  2020-03-23 21:43 ` [PATCH v10 3/5] bugreport: gather git version and build info Emily Shaffer
@ 2020-03-23 21:43 ` Emily Shaffer
  2020-03-23 21:43 ` [PATCH v10 5/5] bugreport: add compiler info Emily Shaffer
  2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-23 21:43 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

The contents of uname() can give us some insight into what sort of
system the user is running on, and help us replicate their setup if need
be. The domainname field is not guaranteed to be available, so don't
collect it.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-bugreport.txt |  1 +
 bugreport.c                     | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index f44ae8cbe7..17b0d14e8d 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -26,6 +26,7 @@ The following information is requested from the user:
 The following information is captured automatically:
 
  - 'git version --build-options'
+ - uname sysname, release, version, and machine strings
 
 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
diff --git a/bugreport.c b/bugreport.c
index 4cdb58bbaa..1a3172bcec 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -7,10 +7,24 @@
 
 static void get_system_info(struct strbuf *sys_info)
 {
+	struct utsname uname_info;
+
 	/* get git version from native cmd */
 	strbuf_addstr(sys_info, _("git version:\n"));
 	get_version_info(sys_info, 1);
-	strbuf_complete_line(sys_info);
+
+	/* system call for other version info */
+	strbuf_addstr(sys_info, "uname: ");
+	if (uname(&uname_info))
+		strbuf_addf(sys_info, _("uname() failed with error '%s' (%d)\n"),
+			    strerror(errno),
+			    errno);
+	else
+		strbuf_addf(sys_info, "%s %s %s %s\n",
+			    uname_info.sysname,
+			    uname_info.release,
+			    uname_info.version,
+			    uname_info.machine);
 }
 
 static const char * const bugreport_usage[] = {
-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v10 5/5] bugreport: add compiler info
  2020-03-23 21:43 [PATCH v10 0/5] add git-bugreport tool Emily Shaffer
                   ` (3 preceding siblings ...)
  2020-03-23 21:43 ` [PATCH v10 4/5] bugreport: add uname info Emily Shaffer
@ 2020-03-23 21:43 ` Emily Shaffer
  2020-03-23 23:10   ` Junio C Hamano
  2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
  5 siblings, 1 reply; 32+ messages in thread
From: Emily Shaffer @ 2020-03-23 21:43 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

To help pinpoint the source of a regression, it is useful to know some
info about the compiler which the user's Git client was built with. By
adding a generic get_compiler_info() in 'compat/' we can choose which
relevant information to share per compiler; to get started, let's
demonstrate the version of glibc if the user built with 'gcc'.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-bugreport.txt |  1 +
 bugreport.c                     |  6 ++++++
 compat/compiler.h               | 38 +++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 compat/compiler.h

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index 17b0d14e8d..643d1b2884 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -27,6 +27,7 @@ The following information is captured automatically:
 
  - 'git version --build-options'
  - uname sysname, release, version, and machine strings
+ - Compiler-specific info string
 
 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
diff --git a/bugreport.c b/bugreport.c
index 1a3172bcec..089b939a87 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -4,6 +4,7 @@
 #include "strbuf.h"
 #include "time.h"
 #include "help.h"
+#include "compat/compiler.h"
 
 static void get_system_info(struct strbuf *sys_info)
 {
@@ -25,6 +26,11 @@ static void get_system_info(struct strbuf *sys_info)
 			    uname_info.release,
 			    uname_info.version,
 			    uname_info.machine);
+
+	strbuf_addstr(sys_info, _("compiler info: "));
+	get_compiler_info(sys_info);
+	strbuf_addstr(sys_info, _("libc info: "));
+	get_libc_info(sys_info);
 }
 
 static const char * const bugreport_usage[] = {
diff --git a/compat/compiler.h b/compat/compiler.h
new file mode 100644
index 0000000000..ce6a7f6de9
--- /dev/null
+++ b/compat/compiler.h
@@ -0,0 +1,38 @@
+#ifndef COMPILER_H
+#define COMPILER_H
+
+#include "git-compat-util.h"
+#include "strbuf.h"
+
+#ifdef __GLIBC__
+#include <gnu/libc-version.h>
+#endif
+
+static inline void get_compiler_info(struct strbuf *info)
+{
+	int len = info->len;
+#ifdef __GNUC__
+	strbuf_addf(info, "gnuc: %d.%d\n", __GNUC__, __GNUC_MINOR__);
+#endif
+
+#ifdef _MSC_VER
+	strbuf_addf(info, "MSVC version: %s\n", _MSC_FULL_VER);
+#endif
+
+	if (len == info->len)
+		strbuf_addstr(info, _("no compiler information available\n"));
+}
+
+static inline void get_libc_info(struct strbuf *info)
+{
+	int len = info->len;
+
+#ifdef __GLIBC__
+	strbuf_addf(info, "glibc: %s\n", gnu_get_libc_version());
+#endif
+
+	if (len == info->len)
+		strbuf_addstr(info, _("no libc information available\n"));
+}
+
+#endif /* COMPILER_H */
-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v10 5/5] bugreport: add compiler info
  2020-03-23 21:43 ` [PATCH v10 5/5] bugreport: add compiler info Emily Shaffer
@ 2020-03-23 23:10   ` Junio C Hamano
  2020-03-25 21:10     ` Emily Shaffer
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2020-03-23 23:10 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: git

Emily Shaffer <emilyshaffer@google.com> writes:

> +#ifdef _MSC_VER
> +	strbuf_addf(info, "MSVC version: %s\n", _MSC_FULL_VER);
> +#endif

This part is a bit different from what Dscho suggested to be
squashed (and I have had on top of v9 series).

Below is a diff between (v9 + SQUASH???) and v10.  I think the
change in the strftime format is a strict improvement.

Thanks.

 bugreport.c       | 2 +-
 compat/compiler.h | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/bugreport.c b/bugreport.c
index 5073c901cd..089b939a87 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -73,7 +73,7 @@ int cmd_main(int argc, const char **argv)
 	int report = -1;
 	time_t now = time(NULL);
 	char *option_output = NULL;
-	char *option_suffix = "%F-%H%M";
+	char *option_suffix = "%Y-%m-%d-%H%M";
 	int nongit_ok = 0;
 	const char *prefix = NULL;
 	const char *user_relative_path = NULL;
diff --git a/compat/compiler.h b/compat/compiler.h
index ac90fa051d..ce6a7f6de9 100644
--- a/compat/compiler.h
+++ b/compat/compiler.h
@@ -16,8 +16,7 @@ static inline void get_compiler_info(struct strbuf *info)
 #endif
 
 #ifdef _MSC_VER
-	strbuf_addf(info, "MSVC version: %02d.%02d.%05d\n",
-		    _MSC_VER / 100, _MSC_VER % 100, _MSC_FULL_VER % 100000);
+	strbuf_addf(info, "MSVC version: %s\n", _MSC_FULL_VER);
 #endif
 
 	if (len == info->len)

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v10 5/5] bugreport: add compiler info
  2020-03-23 23:10   ` Junio C Hamano
@ 2020-03-25 21:10     ` Emily Shaffer
  0 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-25 21:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Mar 23, 2020 at 04:10:59PM -0700, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > +#ifdef _MSC_VER
> > +	strbuf_addf(info, "MSVC version: %s\n", _MSC_FULL_VER);
> > +#endif
> 
> This part is a bit different from what Dscho suggested to be
> squashed (and I have had on top of v9 series).
> 
> Below is a diff between (v9 + SQUASH???) and v10.  I think the
> change in the strftime format is a strict improvement.

Hmph. This is different than what I have in local. Please ignore this
entire v10, I've made a mistake in my tooling. I'm sorry about the
noise and for not checking over my patches better before mailing.

 - Emily

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v11 0/5] add git-bugreport tool
  2020-03-23 21:43 [PATCH v10 0/5] add git-bugreport tool Emily Shaffer
                   ` (4 preceding siblings ...)
  2020-03-23 21:43 ` [PATCH v10 5/5] bugreport: add compiler info Emily Shaffer
@ 2020-03-25 21:24 ` Emily Shaffer
  2020-03-25 21:24   ` [PATCH v11 1/5] help: move list_config_help to builtin/help Emily Shaffer
                     ` (5 more replies)
  5 siblings, 6 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-25 21:24 UTC (permalink / raw)
  To: git
  Cc: Emily Shaffer, Derrick Stolee, Johannes Schindelin,
	Junio C Hamano, Martin Ågren, Aaron Schrab, Danh Doan,
	Eric Sunshine, SZEDER Gábor, Andreas Schwab

Please ignore v10; it seemed that I had a ref .git/bugreport as well as
a ref .git/refs/heads/bugreport and the former was pointing somewhere
stale, so v10 was identical at least to v9 if not also to v8. :|

This hopefully really does include the MSVC changes as well as the clang
changes and addstr noise fixes. I'm really sorry for the noise.

 - Emily

Emily Shaffer (5):
  help: move list_config_help to builtin/help
  bugreport: add tool to generate debugging info
  bugreport: gather git version and build info
  bugreport: add uname info
  bugreport: add compiler info

 .gitignore                      |   2 +
 Documentation/git-bugreport.txt |  52 ++++++++++++
 Makefile                        |  18 +++-
 bugreport.c                     | 142 ++++++++++++++++++++++++++++++++
 builtin/help.c                  |  86 +++++++++++++++++++
 command-list.txt                |   1 +
 compat/compiler.h               |  41 +++++++++
 generate-cmdlist.sh             |  19 -----
 generate-configlist.sh          |  21 +++++
 help.c                          | 131 +++++++----------------------
 help.h                          |   2 +-
 strbuf.c                        |   4 +
 strbuf.h                        |   1 +
 t/t0091-bugreport.sh            |  61 ++++++++++++++
 14 files changed, 455 insertions(+), 126 deletions(-)
 create mode 100644 Documentation/git-bugreport.txt
 create mode 100644 bugreport.c
 create mode 100644 compat/compiler.h
 create mode 100755 generate-configlist.sh
 create mode 100755 t/t0091-bugreport.sh

-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v11 1/5] help: move list_config_help to builtin/help
  2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
@ 2020-03-25 21:24   ` Emily Shaffer
  2020-03-25 21:24   ` [PATCH v11 2/5] bugreport: add tool to generate debugging info Emily Shaffer
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-25 21:24 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

Starting in 3ac68a93fd2, help.o began to depend on builtin/branch.o,
builtin/clean.o, and builtin/config.o. This meant that help.o was
unusable outside of the context of the main Git executable.

To make help.o usable by other commands again, move list_config_help()
into builtin/help.c (where it makes sense to assume other builtin libraries
are present).

When command-list.h is included but a member is not used, we start to
hear a compiler warning. Since the config list is generated in a fairly
different way than the command list, and since commands and config
options are semantically different, move the config list into its own
header and move the generator into its own script and build rule.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 .gitignore             |  1 +
 Makefile               | 13 +++++--
 builtin/help.c         | 86 ++++++++++++++++++++++++++++++++++++++++++
 generate-cmdlist.sh    | 19 ----------
 generate-configlist.sh | 21 +++++++++++
 help.c                 | 85 -----------------------------------------
 help.h                 |  1 -
 7 files changed, 118 insertions(+), 108 deletions(-)
 create mode 100755 generate-configlist.sh

diff --git a/.gitignore b/.gitignore
index aebe7c0908..ea97de83f3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -189,6 +189,7 @@
 /gitweb/gitweb.cgi
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
+/config-list.h
 /command-list.h
 *.tar.gz
 *.dsc
diff --git a/Makefile b/Makefile
index 6134104ae6..c552312d3f 100644
--- a/Makefile
+++ b/Makefile
@@ -815,6 +815,7 @@ LIB_FILE = libgit.a
 XDIFF_LIB = xdiff/lib.a
 VCSSVN_LIB = vcs-svn/lib.a
 
+GENERATED_H += config-list.h
 GENERATED_H += command-list.h
 
 LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
@@ -2132,7 +2133,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 
 help.sp help.s help.o: command-list.h
 
-builtin/help.sp builtin/help.s builtin/help.o: command-list.h GIT-PREFIX
+builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 	'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
@@ -2152,6 +2153,12 @@ $(BUILT_INS): git$X
 	ln -s $< $@ 2>/dev/null || \
 	cp $< $@
 
+config-list.h: generate-configlist.sh
+
+config-list.h:
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \
+		>$@+ && mv $@+ $@
+
 command-list.h: generate-cmdlist.sh command-list.txt
 
 command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
@@ -2785,7 +2792,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
 .PHONY: sparse $(SP_OBJ)
 sparse: $(SP_OBJ)
 
-EXCEPT_HDRS := command-list.h unicode-width.h compat/% xdiff/%
+EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%
 ifndef GCRYPT_SHA256
 	EXCEPT_HDRS += sha256/gcrypt.h
 endif
@@ -2807,7 +2814,7 @@ hdr-check: $(HCO)
 style:
 	git clang-format --style file --diff --extensions c,h
 
-check: command-list.h
+check: config-list.h command-list.h
 	@if sparse; \
 	then \
 		echo >&2 "Use 'make sparse' instead"; \
diff --git a/builtin/help.c b/builtin/help.c
index e5590d7787..1c5f2b9255 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -8,6 +8,7 @@
 #include "parse-options.h"
 #include "run-command.h"
 #include "column.h"
+#include "config-list.h"
 #include "help.h"
 #include "alias.h"
 
@@ -62,6 +63,91 @@ static const char * const builtin_help_usage[] = {
 	NULL
 };
 
+struct slot_expansion {
+	const char *prefix;
+	const char *placeholder;
+	void (*fn)(struct string_list *list, const char *prefix);
+	int found;
+};
+
+static void list_config_help(int for_human)
+{
+	struct slot_expansion slot_expansions[] = {
+		{ "advice", "*", list_config_advices },
+		{ "color.branch", "<slot>", list_config_color_branch_slots },
+		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
+		{ "color.diff", "<slot>", list_config_color_diff_slots },
+		{ "color.grep", "<slot>", list_config_color_grep_slots },
+		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
+		{ "color.remote", "<slot>", list_config_color_sideband_slots },
+		{ "color.status", "<slot>", list_config_color_status_slots },
+		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
+		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
+		{ NULL, NULL, NULL }
+	};
+	const char **p;
+	struct slot_expansion *e;
+	struct string_list keys = STRING_LIST_INIT_DUP;
+	int i;
+
+	for (p = config_name_list; *p; p++) {
+		const char *var = *p;
+		struct strbuf sb = STRBUF_INIT;
+
+		for (e = slot_expansions; e->prefix; e++) {
+
+			strbuf_reset(&sb);
+			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
+			if (!strcasecmp(var, sb.buf)) {
+				e->fn(&keys, e->prefix);
+				e->found++;
+				break;
+			}
+		}
+		strbuf_release(&sb);
+		if (!e->prefix)
+			string_list_append(&keys, var);
+	}
+
+	for (e = slot_expansions; e->prefix; e++)
+		if (!e->found)
+			BUG("slot_expansion %s.%s is not used",
+			    e->prefix, e->placeholder);
+
+	string_list_sort(&keys);
+	for (i = 0; i < keys.nr; i++) {
+		const char *var = keys.items[i].string;
+		const char *wildcard, *tag, *cut;
+
+		if (for_human) {
+			puts(var);
+			continue;
+		}
+
+		wildcard = strchr(var, '*');
+		tag = strchr(var, '<');
+
+		if (!wildcard && !tag) {
+			puts(var);
+			continue;
+		}
+
+		if (wildcard && !tag)
+			cut = wildcard;
+		else if (!wildcard && tag)
+			cut = tag;
+		else
+			cut = wildcard < tag ? wildcard : tag;
+
+		/*
+		 * We may produce duplicates, but that's up to
+		 * git-completion.bash to handle
+		 */
+		printf("%.*s\n", (int)(cut - var), var);
+	}
+	string_list_clear(&keys, 0);
+}
+
 static enum help_format parse_help_format(const char *format)
 {
 	if (!strcmp(format, "man"))
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 71158f7d8b..45fecf8bdf 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -76,23 +76,6 @@ print_command_list () {
 	echo "};"
 }
 
-print_config_list () {
-	cat <<EOF
-static const char *config_name_list[] = {
-EOF
-	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
-	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
-	sort |
-	while read line
-	do
-		echo "	\"$line\","
-	done
-	cat <<EOF
-	NULL,
-};
-EOF
-}
-
 exclude_programs=
 while test "--exclude-program" = "$1"
 do
@@ -113,5 +96,3 @@ echo
 define_category_names "$1"
 echo
 print_command_list "$1"
-echo
-print_config_list
diff --git a/generate-configlist.sh b/generate-configlist.sh
new file mode 100755
index 0000000000..8692fe5cf4
--- /dev/null
+++ b/generate-configlist.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+echo "/* Automatically generated by generate-configlist.sh */"
+echo
+
+print_config_list () {
+	cat <<EOF
+static const char *config_name_list[] = {
+EOF
+	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
+	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
+	sort |
+	sed 's/^.*$/	"&",/'
+	cat <<EOF
+	NULL,
+};
+EOF
+}
+
+echo
+print_config_list
diff --git a/help.c b/help.c
index cf67624a94..a21487db77 100644
--- a/help.c
+++ b/help.c
@@ -407,91 +407,6 @@ void list_common_guides_help(void)
 	putchar('\n');
 }
 
-struct slot_expansion {
-	const char *prefix;
-	const char *placeholder;
-	void (*fn)(struct string_list *list, const char *prefix);
-	int found;
-};
-
-void list_config_help(int for_human)
-{
-	struct slot_expansion slot_expansions[] = {
-		{ "advice", "*", list_config_advices },
-		{ "color.branch", "<slot>", list_config_color_branch_slots },
-		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
-		{ "color.diff", "<slot>", list_config_color_diff_slots },
-		{ "color.grep", "<slot>", list_config_color_grep_slots },
-		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
-		{ "color.remote", "<slot>", list_config_color_sideband_slots },
-		{ "color.status", "<slot>", list_config_color_status_slots },
-		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
-		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
-		{ NULL, NULL, NULL }
-	};
-	const char **p;
-	struct slot_expansion *e;
-	struct string_list keys = STRING_LIST_INIT_DUP;
-	int i;
-
-	for (p = config_name_list; *p; p++) {
-		const char *var = *p;
-		struct strbuf sb = STRBUF_INIT;
-
-		for (e = slot_expansions; e->prefix; e++) {
-
-			strbuf_reset(&sb);
-			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
-			if (!strcasecmp(var, sb.buf)) {
-				e->fn(&keys, e->prefix);
-				e->found++;
-				break;
-			}
-		}
-		strbuf_release(&sb);
-		if (!e->prefix)
-			string_list_append(&keys, var);
-	}
-
-	for (e = slot_expansions; e->prefix; e++)
-		if (!e->found)
-			BUG("slot_expansion %s.%s is not used",
-			    e->prefix, e->placeholder);
-
-	string_list_sort(&keys);
-	for (i = 0; i < keys.nr; i++) {
-		const char *var = keys.items[i].string;
-		const char *wildcard, *tag, *cut;
-
-		if (for_human) {
-			puts(var);
-			continue;
-		}
-
-		wildcard = strchr(var, '*');
-		tag = strchr(var, '<');
-
-		if (!wildcard && !tag) {
-			puts(var);
-			continue;
-		}
-
-		if (wildcard && !tag)
-			cut = wildcard;
-		else if (!wildcard && tag)
-			cut = tag;
-		else
-			cut = wildcard < tag ? wildcard : tag;
-
-		/*
-		 * We may produce duplicates, but that's up to
-		 * git-completion.bash to handle
-		 */
-		printf("%.*s\n", (int)(cut - var), var);
-	}
-	string_list_clear(&keys, 0);
-}
-
 static int get_alias(const char *var, const char *value, void *data)
 {
 	struct string_list *list = data;
diff --git a/help.h b/help.h
index 7a455beeb7..9071894e8c 100644
--- a/help.h
+++ b/help.h
@@ -22,7 +22,6 @@ static inline void mput_char(char c, unsigned int num)
 void list_common_cmds_help(void);
 void list_all_cmds_help(void);
 void list_common_guides_help(void);
-void list_config_help(int for_human);
 
 void list_all_main_cmds(struct string_list *list);
 void list_all_other_cmds(struct string_list *list);
-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v11 2/5] bugreport: add tool to generate debugging info
  2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
  2020-03-25 21:24   ` [PATCH v11 1/5] help: move list_config_help to builtin/help Emily Shaffer
@ 2020-03-25 21:24   ` Emily Shaffer
  2020-04-01 20:17     ` SZEDER Gábor
  2020-03-25 21:24   ` [PATCH v11 3/5] bugreport: gather git version and build info Emily Shaffer
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 32+ messages in thread
From: Emily Shaffer @ 2020-03-25 21:24 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

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

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..f6f53a5e8e
--- /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 = "%Y-%m-%d-%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.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v11 3/5] bugreport: gather git version and build info
  2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
  2020-03-25 21:24   ` [PATCH v11 1/5] help: move list_config_help to builtin/help Emily Shaffer
  2020-03-25 21:24   ` [PATCH v11 2/5] bugreport: add tool to generate debugging info Emily Shaffer
@ 2020-03-25 21:24   ` Emily Shaffer
  2020-03-25 21:24   ` [PATCH v11 4/5] bugreport: add uname info Emily Shaffer
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-25 21:24 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

Knowing which version of Git a user has and how it was built allows us
to more precisely pin down the circumstances when a certain issue
occurs, so teach bugreport how to tell us the same output as 'git
version --build-options'.

It's not ideal to directly call 'git version --build-options' because
that output goes to stdout. Instead, wrap the version string in a helper
within help.[ch] library, and call that helper from within the bugreport
library.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-bugreport.txt |  4 +++
 bugreport.c                     | 19 +++++++++++++-
 help.c                          | 46 ++++++++++++++++++++-------------
 help.h                          |  1 +
 4 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index 1f9fde5cde..f44ae8cbe7 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -23,6 +23,10 @@ The following information is requested from the user:
  - Expected behavior
  - Actual behavior
 
+The following information is captured automatically:
+
+ - 'git version --build-options'
+
 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
diff --git a/bugreport.c b/bugreport.c
index f6f53a5e8e..4cdb58bbaa 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -1,8 +1,17 @@
-#include "builtin.h"
+#include "cache.h"
 #include "parse-options.h"
 #include "stdio.h"
 #include "strbuf.h"
 #include "time.h"
+#include "help.h"
+
+static void get_system_info(struct strbuf *sys_info)
+{
+	/* get git version from native cmd */
+	strbuf_addstr(sys_info, _("git version:\n"));
+	get_version_info(sys_info, 1);
+	strbuf_complete_line(sys_info);
+}
 
 static const char * const bugreport_usage[] = {
 	N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
@@ -32,6 +41,11 @@ static int get_bug_template(struct strbuf *template)
 	return 0;
 }
 
+static void get_header(struct strbuf *buf, const char *title)
+{
+	strbuf_addf(buf, "\n\n[%s]\n", title);
+}
+
 int cmd_main(int argc, const char **argv)
 {
 	struct strbuf buffer = STRBUF_INIT;
@@ -79,6 +93,9 @@ int cmd_main(int argc, const char **argv)
 	/* Prepare the report contents */
 	get_bug_template(&buffer);
 
+	get_header(&buffer, _("System Info"));
+	get_system_info(&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);
 
diff --git a/help.c b/help.c
index a21487db77..1de9c0d589 100644
--- a/help.c
+++ b/help.c
@@ -622,8 +622,32 @@ const char *help_unknown_cmd(const char *cmd)
 	exit(1);
 }
 
+void get_version_info(struct strbuf *buf, int show_build_options)
+{
+	/*
+	 * The format of this string should be kept stable for compatibility
+	 * with external projects that rely on the output of "git version".
+	 *
+	 * Always show the version, even if other options are given.
+	 */
+	strbuf_addf(buf, "git version %s\n", git_version_string);
+
+	if (show_build_options) {
+		strbuf_addf(buf, "cpu: %s\n", GIT_HOST_CPU);
+		if (git_built_from_commit_string[0])
+			strbuf_addf(buf, "built from commit: %s\n",
+			       git_built_from_commit_string);
+		else
+			strbuf_addstr(buf, "no commit associated with this build\n");
+		strbuf_addf(buf, "sizeof-long: %d\n", (int)sizeof(long));
+		strbuf_addf(buf, "sizeof-size_t: %d\n", (int)sizeof(size_t));
+		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
+	}
+}
+
 int cmd_version(int argc, const char **argv, const char *prefix)
 {
+	struct strbuf buf = STRBUF_INIT;
 	int build_options = 0;
 	const char * const usage[] = {
 		N_("git version [<options>]"),
@@ -637,25 +661,11 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
-	/*
-	 * The format of this string should be kept stable for compatibility
-	 * with external projects that rely on the output of "git version".
-	 *
-	 * Always show the version, even if other options are given.
-	 */
-	printf("git version %s\n", git_version_string);
+	get_version_info(&buf, build_options);
+	printf("%s", buf.buf);
+
+	strbuf_release(&buf);
 
-	if (build_options) {
-		printf("cpu: %s\n", GIT_HOST_CPU);
-		if (git_built_from_commit_string[0])
-			printf("built from commit: %s\n",
-			       git_built_from_commit_string);
-		else
-			printf("no commit associated with this build\n");
-		printf("sizeof-long: %d\n", (int)sizeof(long));
-		printf("sizeof-size_t: %d\n", (int)sizeof(size_t));
-		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
-	}
 	return 0;
 }
 
diff --git a/help.h b/help.h
index 9071894e8c..500521b908 100644
--- a/help.h
+++ b/help.h
@@ -37,6 +37,7 @@ void add_cmdname(struct cmdnames *cmds, const char *name, int len);
 void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes);
 int is_in_cmdlist(struct cmdnames *cmds, const char *name);
 void list_commands(unsigned int colopts, struct cmdnames *main_cmds, struct cmdnames *other_cmds);
+void get_version_info(struct strbuf *buf, int show_build_options);
 
 /*
  * call this to die(), when it is suspected that the user mistyped a
-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v11 4/5] bugreport: add uname info
  2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
                     ` (2 preceding siblings ...)
  2020-03-25 21:24   ` [PATCH v11 3/5] bugreport: gather git version and build info Emily Shaffer
@ 2020-03-25 21:24   ` Emily Shaffer
  2020-03-25 21:25   ` [PATCH v11 5/5] bugreport: add compiler info Emily Shaffer
  2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-25 21:24 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

The contents of uname() can give us some insight into what sort of
system the user is running on, and help us replicate their setup if need
be. The domainname field is not guaranteed to be available, so don't
collect it.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-bugreport.txt |  1 +
 bugreport.c                     | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index f44ae8cbe7..17b0d14e8d 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -26,6 +26,7 @@ The following information is requested from the user:
 The following information is captured automatically:
 
  - 'git version --build-options'
+ - uname sysname, release, version, and machine strings
 
 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
diff --git a/bugreport.c b/bugreport.c
index 4cdb58bbaa..1a3172bcec 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -7,10 +7,24 @@
 
 static void get_system_info(struct strbuf *sys_info)
 {
+	struct utsname uname_info;
+
 	/* get git version from native cmd */
 	strbuf_addstr(sys_info, _("git version:\n"));
 	get_version_info(sys_info, 1);
-	strbuf_complete_line(sys_info);
+
+	/* system call for other version info */
+	strbuf_addstr(sys_info, "uname: ");
+	if (uname(&uname_info))
+		strbuf_addf(sys_info, _("uname() failed with error '%s' (%d)\n"),
+			    strerror(errno),
+			    errno);
+	else
+		strbuf_addf(sys_info, "%s %s %s %s\n",
+			    uname_info.sysname,
+			    uname_info.release,
+			    uname_info.version,
+			    uname_info.machine);
 }
 
 static const char * const bugreport_usage[] = {
-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v11 5/5] bugreport: add compiler info
  2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
                     ` (3 preceding siblings ...)
  2020-03-25 21:24   ` [PATCH v11 4/5] bugreport: add uname info Emily Shaffer
@ 2020-03-25 21:25   ` Emily Shaffer
  2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-03-25 21:25 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer, Danh Doan, Johannes Schindelin

To help pinpoint the source of a regression, it is useful to know some
info about the compiler which the user's Git client was built with. By
adding a generic get_compiler_info() in 'compat/' we can choose which
relevant information to share per compiler; to get started, let's
demonstrate the version of glibc if the user built with 'gcc'.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Helped-by: Danh Doan <congdanhqx@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 Documentation/git-bugreport.txt |  1 +
 bugreport.c                     |  6 +++++
 compat/compiler.h               | 41 +++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 compat/compiler.h

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index 17b0d14e8d..643d1b2884 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -27,6 +27,7 @@ The following information is captured automatically:
 
  - 'git version --build-options'
  - uname sysname, release, version, and machine strings
+ - Compiler-specific info string
 
 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
diff --git a/bugreport.c b/bugreport.c
index 1a3172bcec..089b939a87 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -4,6 +4,7 @@
 #include "strbuf.h"
 #include "time.h"
 #include "help.h"
+#include "compat/compiler.h"
 
 static void get_system_info(struct strbuf *sys_info)
 {
@@ -25,6 +26,11 @@ static void get_system_info(struct strbuf *sys_info)
 			    uname_info.release,
 			    uname_info.version,
 			    uname_info.machine);
+
+	strbuf_addstr(sys_info, _("compiler info: "));
+	get_compiler_info(sys_info);
+	strbuf_addstr(sys_info, _("libc info: "));
+	get_libc_info(sys_info);
 }
 
 static const char * const bugreport_usage[] = {
diff --git a/compat/compiler.h b/compat/compiler.h
new file mode 100644
index 0000000000..10dbb65937
--- /dev/null
+++ b/compat/compiler.h
@@ -0,0 +1,41 @@
+#ifndef COMPILER_H
+#define COMPILER_H
+
+#include "git-compat-util.h"
+#include "strbuf.h"
+
+#ifdef __GLIBC__
+#include <gnu/libc-version.h>
+#endif
+
+static inline void get_compiler_info(struct strbuf *info)
+{
+	int len = info->len;
+#ifdef __clang__
+	strbuf_addf(info, "clang: %s\n", __clang_version__);
+#elif defined(__GNUC__)
+	strbuf_addf(info, "gnuc: %d.%d\n", __GNUC__, __GNUC_MINOR__);
+#endif
+
+#ifdef _MSC_VER
+	strbuf_addf(info, "MSVC version: %02d.%02d.%05d\n",
+		    _MSC_VER / 100, _MSC_VER % 100, _MSC_FULL_VER % 100000);
+#endif
+
+	if (len == info->len)
+		strbuf_addstr(info, _("no compiler information available\n"));
+}
+
+static inline void get_libc_info(struct strbuf *info)
+{
+	int len = info->len;
+
+#ifdef __GLIBC__
+	strbuf_addf(info, "glibc: %s\n", gnu_get_libc_version());
+#endif
+
+	if (len == info->len)
+		strbuf_addstr(info, _("no libc information available\n"));
+}
+
+#endif /* COMPILER_H */
-- 
2.25.1.696.g5e7596f4ac-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v11 2/5] bugreport: add tool to generate debugging info
  2020-03-25 21:24   ` [PATCH v11 2/5] bugreport: add tool to generate debugging info Emily Shaffer
@ 2020-04-01 20:17     ` SZEDER Gábor
  0 siblings, 0 replies; 32+ messages in thread
From: SZEDER Gábor @ 2020-04-01 20:17 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: git

On Wed, Mar 25, 2020 at 02:24:57PM -0700, Emily Shaffer wrote:
> 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_when_finished' should be at the beginning of the test, otherwise
it will have no effect if a command before it were to fail.

> +'
> +
> +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.1.696.g5e7596f4ac-goog
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 0/5] bugreport: add tool to generate debugging info
  2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
                     ` (4 preceding siblings ...)
  2020-03-25 21:25   ` [PATCH v11 5/5] bugreport: add compiler info Emily Shaffer
@ 2020-04-06 22:45   ` Emily Shaffer
  2020-04-06 22:45     ` [PATCH v12 1/5] help: move list_config_help to builtin/help Emily Shaffer
                       ` (5 more replies)
  5 siblings, 6 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-04-06 22:45 UTC (permalink / raw)
  To: git
  Cc: Emily Shaffer, Derrick Stolee, Johannes Schindelin,
	Junio C Hamano, Martin Ågren, Aaron Schrab, Danh Doan,
	Eric Sunshine, SZEDER Gábor, Andreas Schwab

Since v11, reordered tests per SZEDER's comment so that the
test_when_finished lines come first in each test.

Emily Shaffer (5):
  help: move list_config_help to builtin/help
  bugreport: add tool to generate debugging info
  bugreport: gather git version and build info
  bugreport: add uname info
  bugreport: add compiler info

 .gitignore                      |   2 +
 Documentation/git-bugreport.txt |  52 ++++++++++++
 Makefile                        |  18 +++-
 bugreport.c                     | 142 ++++++++++++++++++++++++++++++++
 builtin/help.c                  |  86 +++++++++++++++++++
 command-list.txt                |   1 +
 compat/compiler.h               |  41 +++++++++
 generate-cmdlist.sh             |  19 -----
 generate-configlist.sh          |  21 +++++
 help.c                          | 131 +++++++----------------------
 help.h                          |   2 +-
 strbuf.c                        |   4 +
 strbuf.h                        |   1 +
 t/t0091-bugreport.sh            |  61 ++++++++++++++
 14 files changed, 455 insertions(+), 126 deletions(-)
 create mode 100644 Documentation/git-bugreport.txt
 create mode 100644 bugreport.c
 create mode 100644 compat/compiler.h
 create mode 100755 generate-configlist.sh
 create mode 100755 t/t0091-bugreport.sh

-- 
2.26.0.292.g33ef6b2f38-goog


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 1/5] help: move list_config_help to builtin/help
  2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
@ 2020-04-06 22:45     ` Emily Shaffer
  2020-04-06 23:04       ` Junio C Hamano
  2020-04-06 22:45     ` [PATCH v12 2/5] bugreport: add tool to generate debugging info Emily Shaffer
                       ` (4 subsequent siblings)
  5 siblings, 1 reply; 32+ messages in thread
From: Emily Shaffer @ 2020-04-06 22:45 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

Starting in 3ac68a93fd2, help.o began to depend on builtin/branch.o,
builtin/clean.o, and builtin/config.o. This meant that help.o was
unusable outside of the context of the main Git executable.

To make help.o usable by other commands again, move list_config_help()
into builtin/help.c (where it makes sense to assume other builtin libraries
are present).

When command-list.h is included but a member is not used, we start to
hear a compiler warning. Since the config list is generated in a fairly
different way than the command list, and since commands and config
options are semantically different, move the config list into its own
header and move the generator into its own script and build rule.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 .gitignore             |  1 +
 Makefile               | 13 +++++--
 builtin/help.c         | 86 ++++++++++++++++++++++++++++++++++++++++++
 generate-cmdlist.sh    | 19 ----------
 generate-configlist.sh | 21 +++++++++++
 help.c                 | 85 -----------------------------------------
 help.h                 |  1 -
 7 files changed, 118 insertions(+), 108 deletions(-)
 create mode 100755 generate-configlist.sh

diff --git a/.gitignore b/.gitignore
index 188bd1c3de..61bf5142a9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -188,6 +188,7 @@
 /gitweb/gitweb.cgi
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
+/config-list.h
 /command-list.h
 *.tar.gz
 *.dsc
diff --git a/Makefile b/Makefile
index ef1ff2228f..d4aff7f9b5 100644
--- a/Makefile
+++ b/Makefile
@@ -815,6 +815,7 @@ LIB_FILE = libgit.a
 XDIFF_LIB = xdiff/lib.a
 VCSSVN_LIB = vcs-svn/lib.a
 
+GENERATED_H += config-list.h
 GENERATED_H += command-list.h
 
 LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
@@ -2133,7 +2134,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 
 help.sp help.s help.o: command-list.h
 
-builtin/help.sp builtin/help.s builtin/help.o: command-list.h GIT-PREFIX
+builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 	'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
@@ -2153,6 +2154,12 @@ $(BUILT_INS): git$X
 	ln -s $< $@ 2>/dev/null || \
 	cp $< $@
 
+config-list.h: generate-configlist.sh
+
+config-list.h:
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \
+		>$@+ && mv $@+ $@
+
 command-list.h: generate-cmdlist.sh command-list.txt
 
 command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
@@ -2786,7 +2793,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
 .PHONY: sparse $(SP_OBJ)
 sparse: $(SP_OBJ)
 
-EXCEPT_HDRS := command-list.h unicode-width.h compat/% xdiff/%
+EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%
 ifndef GCRYPT_SHA256
 	EXCEPT_HDRS += sha256/gcrypt.h
 endif
@@ -2808,7 +2815,7 @@ hdr-check: $(HCO)
 style:
 	git clang-format --style file --diff --extensions c,h
 
-check: command-list.h
+check: config-list.h command-list.h
 	@if sparse; \
 	then \
 		echo >&2 "Use 'make sparse' instead"; \
diff --git a/builtin/help.c b/builtin/help.c
index e5590d7787..1c5f2b9255 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -8,6 +8,7 @@
 #include "parse-options.h"
 #include "run-command.h"
 #include "column.h"
+#include "config-list.h"
 #include "help.h"
 #include "alias.h"
 
@@ -62,6 +63,91 @@ static const char * const builtin_help_usage[] = {
 	NULL
 };
 
+struct slot_expansion {
+	const char *prefix;
+	const char *placeholder;
+	void (*fn)(struct string_list *list, const char *prefix);
+	int found;
+};
+
+static void list_config_help(int for_human)
+{
+	struct slot_expansion slot_expansions[] = {
+		{ "advice", "*", list_config_advices },
+		{ "color.branch", "<slot>", list_config_color_branch_slots },
+		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
+		{ "color.diff", "<slot>", list_config_color_diff_slots },
+		{ "color.grep", "<slot>", list_config_color_grep_slots },
+		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
+		{ "color.remote", "<slot>", list_config_color_sideband_slots },
+		{ "color.status", "<slot>", list_config_color_status_slots },
+		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
+		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
+		{ NULL, NULL, NULL }
+	};
+	const char **p;
+	struct slot_expansion *e;
+	struct string_list keys = STRING_LIST_INIT_DUP;
+	int i;
+
+	for (p = config_name_list; *p; p++) {
+		const char *var = *p;
+		struct strbuf sb = STRBUF_INIT;
+
+		for (e = slot_expansions; e->prefix; e++) {
+
+			strbuf_reset(&sb);
+			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
+			if (!strcasecmp(var, sb.buf)) {
+				e->fn(&keys, e->prefix);
+				e->found++;
+				break;
+			}
+		}
+		strbuf_release(&sb);
+		if (!e->prefix)
+			string_list_append(&keys, var);
+	}
+
+	for (e = slot_expansions; e->prefix; e++)
+		if (!e->found)
+			BUG("slot_expansion %s.%s is not used",
+			    e->prefix, e->placeholder);
+
+	string_list_sort(&keys);
+	for (i = 0; i < keys.nr; i++) {
+		const char *var = keys.items[i].string;
+		const char *wildcard, *tag, *cut;
+
+		if (for_human) {
+			puts(var);
+			continue;
+		}
+
+		wildcard = strchr(var, '*');
+		tag = strchr(var, '<');
+
+		if (!wildcard && !tag) {
+			puts(var);
+			continue;
+		}
+
+		if (wildcard && !tag)
+			cut = wildcard;
+		else if (!wildcard && tag)
+			cut = tag;
+		else
+			cut = wildcard < tag ? wildcard : tag;
+
+		/*
+		 * We may produce duplicates, but that's up to
+		 * git-completion.bash to handle
+		 */
+		printf("%.*s\n", (int)(cut - var), var);
+	}
+	string_list_clear(&keys, 0);
+}
+
 static enum help_format parse_help_format(const char *format)
 {
 	if (!strcmp(format, "man"))
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 71158f7d8b..45fecf8bdf 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -76,23 +76,6 @@ print_command_list () {
 	echo "};"
 }
 
-print_config_list () {
-	cat <<EOF
-static const char *config_name_list[] = {
-EOF
-	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
-	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
-	sort |
-	while read line
-	do
-		echo "	\"$line\","
-	done
-	cat <<EOF
-	NULL,
-};
-EOF
-}
-
 exclude_programs=
 while test "--exclude-program" = "$1"
 do
@@ -113,5 +96,3 @@ echo
 define_category_names "$1"
 echo
 print_command_list "$1"
-echo
-print_config_list
diff --git a/generate-configlist.sh b/generate-configlist.sh
new file mode 100755
index 0000000000..8692fe5cf4
--- /dev/null
+++ b/generate-configlist.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+echo "/* Automatically generated by generate-configlist.sh */"
+echo
+
+print_config_list () {
+	cat <<EOF
+static const char *config_name_list[] = {
+EOF
+	grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
+	sed '/deprecated/d; s/::$//; s/,  */\n/g' |
+	sort |
+	sed 's/^.*$/	"&",/'
+	cat <<EOF
+	NULL,
+};
+EOF
+}
+
+echo
+print_config_list
diff --git a/help.c b/help.c
index cf67624a94..a21487db77 100644
--- a/help.c
+++ b/help.c
@@ -407,91 +407,6 @@ void list_common_guides_help(void)
 	putchar('\n');
 }
 
-struct slot_expansion {
-	const char *prefix;
-	const char *placeholder;
-	void (*fn)(struct string_list *list, const char *prefix);
-	int found;
-};
-
-void list_config_help(int for_human)
-{
-	struct slot_expansion slot_expansions[] = {
-		{ "advice", "*", list_config_advices },
-		{ "color.branch", "<slot>", list_config_color_branch_slots },
-		{ "color.decorate", "<slot>", list_config_color_decorate_slots },
-		{ "color.diff", "<slot>", list_config_color_diff_slots },
-		{ "color.grep", "<slot>", list_config_color_grep_slots },
-		{ "color.interactive", "<slot>", list_config_color_interactive_slots },
-		{ "color.remote", "<slot>", list_config_color_sideband_slots },
-		{ "color.status", "<slot>", list_config_color_status_slots },
-		{ "fsck", "<msg-id>", list_config_fsck_msg_ids },
-		{ "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
-		{ NULL, NULL, NULL }
-	};
-	const char **p;
-	struct slot_expansion *e;
-	struct string_list keys = STRING_LIST_INIT_DUP;
-	int i;
-
-	for (p = config_name_list; *p; p++) {
-		const char *var = *p;
-		struct strbuf sb = STRBUF_INIT;
-
-		for (e = slot_expansions; e->prefix; e++) {
-
-			strbuf_reset(&sb);
-			strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
-			if (!strcasecmp(var, sb.buf)) {
-				e->fn(&keys, e->prefix);
-				e->found++;
-				break;
-			}
-		}
-		strbuf_release(&sb);
-		if (!e->prefix)
-			string_list_append(&keys, var);
-	}
-
-	for (e = slot_expansions; e->prefix; e++)
-		if (!e->found)
-			BUG("slot_expansion %s.%s is not used",
-			    e->prefix, e->placeholder);
-
-	string_list_sort(&keys);
-	for (i = 0; i < keys.nr; i++) {
-		const char *var = keys.items[i].string;
-		const char *wildcard, *tag, *cut;
-
-		if (for_human) {
-			puts(var);
-			continue;
-		}
-
-		wildcard = strchr(var, '*');
-		tag = strchr(var, '<');
-
-		if (!wildcard && !tag) {
-			puts(var);
-			continue;
-		}
-
-		if (wildcard && !tag)
-			cut = wildcard;
-		else if (!wildcard && tag)
-			cut = tag;
-		else
-			cut = wildcard < tag ? wildcard : tag;
-
-		/*
-		 * We may produce duplicates, but that's up to
-		 * git-completion.bash to handle
-		 */
-		printf("%.*s\n", (int)(cut - var), var);
-	}
-	string_list_clear(&keys, 0);
-}
-
 static int get_alias(const char *var, const char *value, void *data)
 {
 	struct string_list *list = data;
diff --git a/help.h b/help.h
index 7a455beeb7..9071894e8c 100644
--- a/help.h
+++ b/help.h
@@ -22,7 +22,6 @@ static inline void mput_char(char c, unsigned int num)
 void list_common_cmds_help(void);
 void list_all_cmds_help(void);
 void list_common_guides_help(void);
-void list_config_help(int for_human);
 
 void list_all_main_cmds(struct string_list *list);
 void list_all_other_cmds(struct string_list *list);
-- 
2.26.0.292.g33ef6b2f38-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 2/5] bugreport: add tool to generate debugging info
  2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
  2020-04-06 22:45     ` [PATCH v12 1/5] help: move list_config_help to builtin/help Emily Shaffer
@ 2020-04-06 22:45     ` Emily Shaffer
  2020-04-06 22:45     ` [PATCH v12 3/5] bugreport: gather git version and build info Emily Shaffer
                       ` (3 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-04-06 22:45 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

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

diff --git a/.gitignore b/.gitignore
index 61bf5142a9..ee509a2ad2 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 d4aff7f9b5..b0a9073788 100644
--- a/Makefile
+++ b/Makefile
@@ -680,6 +680,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
@@ -2462,6 +2463,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..f6f53a5e8e
--- /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 = "%Y-%m-%d-%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 bb0065ccaf..3bfcaababb 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -554,6 +554,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 ce8e49c0b2..801a3694ec 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -468,6 +468,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..2e73658a5c
--- /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' '
+	test_when_finished rm git-bugreport-check-headers.txt &&
+	git bugreport -s check-headers &&
+	check_all_headers_populated <git-bugreport-check-headers.txt
+'
+
+test_expect_success 'dies if file with same name as report already exists' '
+	test_when_finished rm git-bugreport-duplicate.txt &&
+	>>git-bugreport-duplicate.txt &&
+	test_must_fail git bugreport --suffix duplicate
+'
+
+test_expect_success '--output-directory puts the report in the provided dir' '
+	test_when_finished rm -fr foo/ &&
+	git bugreport -o foo/ &&
+	test_path_is_file foo/git-bugreport-*
+'
+
+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' '
+	test_when_finished rm non-repo/git-bugreport-* &&
+	nongit git bugreport
+'
+
+test_expect_success 'can create leading directories outside of a git dir' '
+	test_when_finished rm -fr foo/bar/baz &&
+	nongit git bugreport -o foo/bar/baz
+'
+
+
+test_done
-- 
2.26.0.292.g33ef6b2f38-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 3/5] bugreport: gather git version and build info
  2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
  2020-04-06 22:45     ` [PATCH v12 1/5] help: move list_config_help to builtin/help Emily Shaffer
  2020-04-06 22:45     ` [PATCH v12 2/5] bugreport: add tool to generate debugging info Emily Shaffer
@ 2020-04-06 22:45     ` Emily Shaffer
  2020-04-06 23:17       ` Junio C Hamano
  2020-04-06 22:45     ` [PATCH v12 4/5] bugreport: add uname info Emily Shaffer
                       ` (2 subsequent siblings)
  5 siblings, 1 reply; 32+ messages in thread
From: Emily Shaffer @ 2020-04-06 22:45 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

Knowing which version of Git a user has and how it was built allows us
to more precisely pin down the circumstances when a certain issue
occurs, so teach bugreport how to tell us the same output as 'git
version --build-options'.

It's not ideal to directly call 'git version --build-options' because
that output goes to stdout. Instead, wrap the version string in a helper
within help.[ch] library, and call that helper from within the bugreport
library.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-bugreport.txt |  4 +++
 bugreport.c                     | 19 +++++++++++++-
 help.c                          | 46 ++++++++++++++++++++-------------
 help.h                          |  1 +
 4 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index 1f9fde5cde..f44ae8cbe7 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -23,6 +23,10 @@ The following information is requested from the user:
  - Expected behavior
  - Actual behavior
 
+The following information is captured automatically:
+
+ - 'git version --build-options'
+
 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
diff --git a/bugreport.c b/bugreport.c
index f6f53a5e8e..4cdb58bbaa 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -1,8 +1,17 @@
-#include "builtin.h"
+#include "cache.h"
 #include "parse-options.h"
 #include "stdio.h"
 #include "strbuf.h"
 #include "time.h"
+#include "help.h"
+
+static void get_system_info(struct strbuf *sys_info)
+{
+	/* get git version from native cmd */
+	strbuf_addstr(sys_info, _("git version:\n"));
+	get_version_info(sys_info, 1);
+	strbuf_complete_line(sys_info);
+}
 
 static const char * const bugreport_usage[] = {
 	N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
@@ -32,6 +41,11 @@ static int get_bug_template(struct strbuf *template)
 	return 0;
 }
 
+static void get_header(struct strbuf *buf, const char *title)
+{
+	strbuf_addf(buf, "\n\n[%s]\n", title);
+}
+
 int cmd_main(int argc, const char **argv)
 {
 	struct strbuf buffer = STRBUF_INIT;
@@ -79,6 +93,9 @@ int cmd_main(int argc, const char **argv)
 	/* Prepare the report contents */
 	get_bug_template(&buffer);
 
+	get_header(&buffer, _("System Info"));
+	get_system_info(&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);
 
diff --git a/help.c b/help.c
index a21487db77..1de9c0d589 100644
--- a/help.c
+++ b/help.c
@@ -622,8 +622,32 @@ const char *help_unknown_cmd(const char *cmd)
 	exit(1);
 }
 
+void get_version_info(struct strbuf *buf, int show_build_options)
+{
+	/*
+	 * The format of this string should be kept stable for compatibility
+	 * with external projects that rely on the output of "git version".
+	 *
+	 * Always show the version, even if other options are given.
+	 */
+	strbuf_addf(buf, "git version %s\n", git_version_string);
+
+	if (show_build_options) {
+		strbuf_addf(buf, "cpu: %s\n", GIT_HOST_CPU);
+		if (git_built_from_commit_string[0])
+			strbuf_addf(buf, "built from commit: %s\n",
+			       git_built_from_commit_string);
+		else
+			strbuf_addstr(buf, "no commit associated with this build\n");
+		strbuf_addf(buf, "sizeof-long: %d\n", (int)sizeof(long));
+		strbuf_addf(buf, "sizeof-size_t: %d\n", (int)sizeof(size_t));
+		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
+	}
+}
+
 int cmd_version(int argc, const char **argv, const char *prefix)
 {
+	struct strbuf buf = STRBUF_INIT;
 	int build_options = 0;
 	const char * const usage[] = {
 		N_("git version [<options>]"),
@@ -637,25 +661,11 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
-	/*
-	 * The format of this string should be kept stable for compatibility
-	 * with external projects that rely on the output of "git version".
-	 *
-	 * Always show the version, even if other options are given.
-	 */
-	printf("git version %s\n", git_version_string);
+	get_version_info(&buf, build_options);
+	printf("%s", buf.buf);
+
+	strbuf_release(&buf);
 
-	if (build_options) {
-		printf("cpu: %s\n", GIT_HOST_CPU);
-		if (git_built_from_commit_string[0])
-			printf("built from commit: %s\n",
-			       git_built_from_commit_string);
-		else
-			printf("no commit associated with this build\n");
-		printf("sizeof-long: %d\n", (int)sizeof(long));
-		printf("sizeof-size_t: %d\n", (int)sizeof(size_t));
-		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
-	}
 	return 0;
 }
 
diff --git a/help.h b/help.h
index 9071894e8c..500521b908 100644
--- a/help.h
+++ b/help.h
@@ -37,6 +37,7 @@ void add_cmdname(struct cmdnames *cmds, const char *name, int len);
 void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes);
 int is_in_cmdlist(struct cmdnames *cmds, const char *name);
 void list_commands(unsigned int colopts, struct cmdnames *main_cmds, struct cmdnames *other_cmds);
+void get_version_info(struct strbuf *buf, int show_build_options);
 
 /*
  * call this to die(), when it is suspected that the user mistyped a
-- 
2.26.0.292.g33ef6b2f38-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 4/5] bugreport: add uname info
  2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
                       ` (2 preceding siblings ...)
  2020-04-06 22:45     ` [PATCH v12 3/5] bugreport: gather git version and build info Emily Shaffer
@ 2020-04-06 22:45     ` Emily Shaffer
  2020-04-06 23:20       ` Junio C Hamano
  2020-04-06 22:45     ` [PATCH v12 5/5] bugreport: add compiler info Emily Shaffer
  2020-04-07 15:24     ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Junio C Hamano
  5 siblings, 1 reply; 32+ messages in thread
From: Emily Shaffer @ 2020-04-06 22:45 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

The contents of uname() can give us some insight into what sort of
system the user is running on, and help us replicate their setup if need
be. The domainname field is not guaranteed to be available, so don't
collect it.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-bugreport.txt |  1 +
 bugreport.c                     | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index f44ae8cbe7..17b0d14e8d 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -26,6 +26,7 @@ The following information is requested from the user:
 The following information is captured automatically:
 
  - 'git version --build-options'
+ - uname sysname, release, version, and machine strings
 
 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
diff --git a/bugreport.c b/bugreport.c
index 4cdb58bbaa..1a3172bcec 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -7,10 +7,24 @@
 
 static void get_system_info(struct strbuf *sys_info)
 {
+	struct utsname uname_info;
+
 	/* get git version from native cmd */
 	strbuf_addstr(sys_info, _("git version:\n"));
 	get_version_info(sys_info, 1);
-	strbuf_complete_line(sys_info);
+
+	/* system call for other version info */
+	strbuf_addstr(sys_info, "uname: ");
+	if (uname(&uname_info))
+		strbuf_addf(sys_info, _("uname() failed with error '%s' (%d)\n"),
+			    strerror(errno),
+			    errno);
+	else
+		strbuf_addf(sys_info, "%s %s %s %s\n",
+			    uname_info.sysname,
+			    uname_info.release,
+			    uname_info.version,
+			    uname_info.machine);
 }
 
 static const char * const bugreport_usage[] = {
-- 
2.26.0.292.g33ef6b2f38-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 5/5] bugreport: add compiler info
  2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
                       ` (3 preceding siblings ...)
  2020-04-06 22:45     ` [PATCH v12 4/5] bugreport: add uname info Emily Shaffer
@ 2020-04-06 22:45     ` Emily Shaffer
  2020-04-06 23:31       ` Junio C Hamano
  2020-04-06 23:47       ` Danh Doan
  2020-04-07 15:24     ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Junio C Hamano
  5 siblings, 2 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-04-06 22:45 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer, Danh Doan, Johannes Schindelin

To help pinpoint the source of a regression, it is useful to know some
info about the compiler which the user's Git client was built with. By
adding a generic get_compiler_info() in 'compat/' we can choose which
relevant information to share per compiler; to get started, let's
demonstrate the version of glibc if the user built with 'gcc'.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Helped-by: Danh Doan <congdanhqx@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 Documentation/git-bugreport.txt |  1 +
 bugreport.c                     |  6 +++++
 compat/compiler.h               | 41 +++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 compat/compiler.h

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index 17b0d14e8d..643d1b2884 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -27,6 +27,7 @@ The following information is captured automatically:
 
  - 'git version --build-options'
  - uname sysname, release, version, and machine strings
+ - Compiler-specific info string
 
 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
diff --git a/bugreport.c b/bugreport.c
index 1a3172bcec..089b939a87 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -4,6 +4,7 @@
 #include "strbuf.h"
 #include "time.h"
 #include "help.h"
+#include "compat/compiler.h"
 
 static void get_system_info(struct strbuf *sys_info)
 {
@@ -25,6 +26,11 @@ static void get_system_info(struct strbuf *sys_info)
 			    uname_info.release,
 			    uname_info.version,
 			    uname_info.machine);
+
+	strbuf_addstr(sys_info, _("compiler info: "));
+	get_compiler_info(sys_info);
+	strbuf_addstr(sys_info, _("libc info: "));
+	get_libc_info(sys_info);
 }
 
 static const char * const bugreport_usage[] = {
diff --git a/compat/compiler.h b/compat/compiler.h
new file mode 100644
index 0000000000..10dbb65937
--- /dev/null
+++ b/compat/compiler.h
@@ -0,0 +1,41 @@
+#ifndef COMPILER_H
+#define COMPILER_H
+
+#include "git-compat-util.h"
+#include "strbuf.h"
+
+#ifdef __GLIBC__
+#include <gnu/libc-version.h>
+#endif
+
+static inline void get_compiler_info(struct strbuf *info)
+{
+	int len = info->len;
+#ifdef __clang__
+	strbuf_addf(info, "clang: %s\n", __clang_version__);
+#elif defined(__GNUC__)
+	strbuf_addf(info, "gnuc: %d.%d\n", __GNUC__, __GNUC_MINOR__);
+#endif
+
+#ifdef _MSC_VER
+	strbuf_addf(info, "MSVC version: %02d.%02d.%05d\n",
+		    _MSC_VER / 100, _MSC_VER % 100, _MSC_FULL_VER % 100000);
+#endif
+
+	if (len == info->len)
+		strbuf_addstr(info, _("no compiler information available\n"));
+}
+
+static inline void get_libc_info(struct strbuf *info)
+{
+	int len = info->len;
+
+#ifdef __GLIBC__
+	strbuf_addf(info, "glibc: %s\n", gnu_get_libc_version());
+#endif
+
+	if (len == info->len)
+		strbuf_addstr(info, _("no libc information available\n"));
+}
+
+#endif /* COMPILER_H */
-- 
2.26.0.292.g33ef6b2f38-goog


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 1/5] help: move list_config_help to builtin/help
  2020-04-06 22:45     ` [PATCH v12 1/5] help: move list_config_help to builtin/help Emily Shaffer
@ 2020-04-06 23:04       ` Junio C Hamano
  0 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2020-04-06 23:04 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: git

Emily Shaffer <emilyshaffer@google.com> writes:

> Starting in 3ac68a93fd2, help.o began to depend on builtin/branch.o,
> builtin/clean.o, and builtin/config.o. This meant that help.o was
> unusable outside of the context of the main Git executable.

Quite straight-forward.  Thanks for the clean-up.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 3/5] bugreport: gather git version and build info
  2020-04-06 22:45     ` [PATCH v12 3/5] bugreport: gather git version and build info Emily Shaffer
@ 2020-04-06 23:17       ` Junio C Hamano
  2020-04-07 18:42         ` Emily Shaffer
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2020-04-06 23:17 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: git

Emily Shaffer <emilyshaffer@google.com> writes:

> It's not ideal to directly call 'git version --build-options' because
> that output goes to stdout. Instead, wrap the version string in a helper
> within help.[ch] library, and call that helper from within the bugreport
> library.

This means that "git-bugreport" that can be a different binary from
say "git-remote-curl" (or "git-version" for that matter) would still
report whatever version string that was compiled into "bugreport".

Reporting the version of "bugreport" is *not* useless, and what this
step adds to the report is good.  

But the version number of "bugreport" may not have much to do with
the binary the end user is having trouble with, so we'd also want
the version of the main part of "git", and other standalone "git"
subprograms like "git-remote-curl", reported separately, probably
together with $PATH, $GIT_EXEC_PATH and what appears in the
directories listed on these environment variables.

If "git version --build-options" writes to its standard output
stream, that is a good thing, as it makes it easy to spawn and read
what it says via the run_command() API, and there is one less thing
to worry about (it would be a bit more cumbersome if the output goes
to the standard error stream).

As "git-remote-curl" would also be a separate binary, we'd have to
use the same technique to report version number (and perhaps curl's
library version and its configuration?), perhaps by teaching the
subcommand a new option to dump such details to its standard output.

Using the same technique to report the version about the "git"
itself would be consistent thing to do, as a preparation for future
steps that reports the details about "git-remote-curl".

Thanks.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/5] bugreport: add uname info
  2020-04-06 22:45     ` [PATCH v12 4/5] bugreport: add uname info Emily Shaffer
@ 2020-04-06 23:20       ` Junio C Hamano
  0 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2020-04-06 23:20 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: git

Emily Shaffer <emilyshaffer@google.com> writes:

> The contents of uname() can give us some insight into what sort of
> system the user is running on, and help us replicate their setup if need
> be. The domainname field is not guaranteed to be available, so don't
> collect it.

Unlike "what's the verison of 'git'?" and "with what versions of
which libraries does the 'git' the user is having trouble with links
with?" I mentioned earlier in my review of [3/5], what this step
deals with, i.e. "what are these properties of the running system?",
is perfectly good to report from "git bugreport" itself.

Looking good.

Thanks.


>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>  Documentation/git-bugreport.txt |  1 +
>  bugreport.c                     | 16 +++++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
> index f44ae8cbe7..17b0d14e8d 100644
> --- a/Documentation/git-bugreport.txt
> +++ b/Documentation/git-bugreport.txt
> @@ -26,6 +26,7 @@ The following information is requested from the user:
>  The following information is captured automatically:
>  
>   - 'git version --build-options'
> + - uname sysname, release, version, and machine strings
>  
>  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
> diff --git a/bugreport.c b/bugreport.c
> index 4cdb58bbaa..1a3172bcec 100644
> --- a/bugreport.c
> +++ b/bugreport.c
> @@ -7,10 +7,24 @@
>  
>  static void get_system_info(struct strbuf *sys_info)
>  {
> +	struct utsname uname_info;
> +
>  	/* get git version from native cmd */
>  	strbuf_addstr(sys_info, _("git version:\n"));
>  	get_version_info(sys_info, 1);
> -	strbuf_complete_line(sys_info);
> +
> +	/* system call for other version info */
> +	strbuf_addstr(sys_info, "uname: ");
> +	if (uname(&uname_info))
> +		strbuf_addf(sys_info, _("uname() failed with error '%s' (%d)\n"),
> +			    strerror(errno),
> +			    errno);
> +	else
> +		strbuf_addf(sys_info, "%s %s %s %s\n",
> +			    uname_info.sysname,
> +			    uname_info.release,
> +			    uname_info.version,
> +			    uname_info.machine);
>  }
>  
>  static const char * const bugreport_usage[] = {

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 5/5] bugreport: add compiler info
  2020-04-06 22:45     ` [PATCH v12 5/5] bugreport: add compiler info Emily Shaffer
@ 2020-04-06 23:31       ` Junio C Hamano
  2020-04-06 23:47       ` Danh Doan
  1 sibling, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2020-04-06 23:31 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: git, Danh Doan, Johannes Schindelin

Emily Shaffer <emilyshaffer@google.com> writes:

> To help pinpoint the source of a regression, it is useful to know some
> info about the compiler which the user's Git client was built with. By
> adding a generic get_compiler_info() in 'compat/' we can choose which
> relevant information to share per compiler; to get started, let's
> demonstrate the version of glibc if the user built with 'gcc'.

This one is not about "what system are we running on", but more
about "how were the various pieces of Git binaries the user has
trouble running built?".  While it may be useful to learn how "git
bugreport" was compiled, the same question about the other binaries
of Git (e.g. "git" itself, or "git ls-files" that is builtin) may
have different answers.  It should be fairly trivial to teach the
"git" binaries (i.e. "git" itself plus builtins, and standalone
subcommands like "git-remote-curl" and "git-bugreport") to report
how they were built (e.g. with what library and compiler) by calling
the helpers you added in [3/5] and [5/5] and once that is done, it
would be straight-forward to teach "git bugreport" to collect
properties of them by invoking them and reading their output via the
run_command() interface, as I mentioned in my review on [3/5]
earlire.

Thanks.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 5/5] bugreport: add compiler info
  2020-04-06 22:45     ` [PATCH v12 5/5] bugreport: add compiler info Emily Shaffer
  2020-04-06 23:31       ` Junio C Hamano
@ 2020-04-06 23:47       ` Danh Doan
  2020-04-07 15:09         ` Junio C Hamano
  1 sibling, 1 reply; 32+ messages in thread
From: Danh Doan @ 2020-04-06 23:47 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: git, Johannes Schindelin

On 2020-04-06 15:45:26-0700, Emily Shaffer <emilyshaffer@google.com> wrote:
> Helped-by: Danh Doan <congdanhqx@gmail.com>

If you ever send another version of this series, my Git identity is:

Đoàn Trần Công Danh <congdanhqx@gmail.com>

Thanks,

-- 
Danh

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 5/5] bugreport: add compiler info
  2020-04-06 23:47       ` Danh Doan
@ 2020-04-07 15:09         ` Junio C Hamano
  2020-04-07 18:35           ` Emily Shaffer
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2020-04-07 15:09 UTC (permalink / raw)
  To: Danh Doan; +Cc: Emily Shaffer, git, Johannes Schindelin

Danh Doan <congdanhqx@gmail.com> writes:

> On 2020-04-06 15:45:26-0700, Emily Shaffer <emilyshaffer@google.com> wrote:
>> Helped-by: Danh Doan <congdanhqx@gmail.com>
>
> If you ever send another version of this series, my Git identity is:
>
> Đoàn Trần Công Danh <congdanhqx@gmail.com>
>
> Thanks,

Thanks.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 0/5] bugreport: add tool to generate debugging info
  2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
                       ` (4 preceding siblings ...)
  2020-04-06 22:45     ` [PATCH v12 5/5] bugreport: add compiler info Emily Shaffer
@ 2020-04-07 15:24     ` Junio C Hamano
  5 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2020-04-07 15:24 UTC (permalink / raw)
  To: Emily Shaffer
  Cc: git, Derrick Stolee, Johannes Schindelin, Martin Ågren,
	Aaron Schrab, Danh Doan, Eric Sunshine, SZEDER Gábor,
	Andreas Schwab

Emily Shaffer <emilyshaffer@google.com> writes:

> Since v11, reordered tests per SZEDER's comment so that the
> test_when_finished lines come first in each test.
>
> Emily Shaffer (5):
>   help: move list_config_help to builtin/help
>   bugreport: add tool to generate debugging info
>   bugreport: gather git version and build info
>   bugreport: add uname info
>   bugreport: add compiler info

Overall it was a pleasnt read.

Just to avoid misunderstanding, those comments in which I envisioned
a way to report versions and attributes on different parts of "git"
(and I expect we'd be reporting versions and add-ons to tools like
'wish' that runs gitk and git-gui, 'perl' and 'python' in a similar
way---by finding them on the $PATH and asking them who they are) are
NOT "without these, the bugreport tool would not be useful", but "we
will in time need these, so let's keep them in mind" issues [*1*].

So, I am hoping that this is ready to move to 'next'.

Thanks.


[Footnote]

*1* What we saw here in the review showed us a very good starting
point to report the version of the source one binary was built from,
and the compiler and library that binary was built with, and various
attributes of the system that is running that binary.  Thanks to
step [1/5], these helper functions can also be linked to help other
parts of "git" (like the transport) report the first two (some code
may further need to be moved so that they can be called from the
built-ins and standalones).  The system that the binary is running
on would be the same anyway, so there is no need to duplicate that
part of the report.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 5/5] bugreport: add compiler info
  2020-04-07 15:09         ` Junio C Hamano
@ 2020-04-07 18:35           ` Emily Shaffer
  0 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-04-07 18:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Danh Doan, git, Johannes Schindelin

On Tue, Apr 07, 2020 at 08:09:02AM -0700, Junio C Hamano wrote:
> Danh Doan <congdanhqx@gmail.com> writes:
> 
> > On 2020-04-06 15:45:26-0700, Emily Shaffer <emilyshaffer@google.com> wrote:
> >> Helped-by: Danh Doan <congdanhqx@gmail.com>
> >
> > If you ever send another version of this series, my Git identity is:
> >
> > Đoàn Trần Công Danh <congdanhqx@gmail.com>
> >
> > Thanks,
> 
> Thanks.
> 
Thanks Danh - I copied it from the email header, my apologies. Junio,
since you replied as well does that mean you updated your copy and I
don't need to re-send? (I haven't read the other comments yet, but your
comment on the cover letter makes me think you don't want a v13.)

 - Emily

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 3/5] bugreport: gather git version and build info
  2020-04-06 23:17       ` Junio C Hamano
@ 2020-04-07 18:42         ` Emily Shaffer
  2020-04-07 20:05           ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Emily Shaffer @ 2020-04-07 18:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Apr 06, 2020 at 04:17:51PM -0700, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > It's not ideal to directly call 'git version --build-options' because
> > that output goes to stdout. Instead, wrap the version string in a helper
> > within help.[ch] library, and call that helper from within the bugreport
> > library.
> 
> This means that "git-bugreport" that can be a different binary from
> say "git-remote-curl" (or "git-version" for that matter) would still
> report whatever version string that was compiled into "bugreport".
> 
> Reporting the version of "bugreport" is *not* useless, and what this
> step adds to the report is good.  
> 
> But the version number of "bugreport" may not have much to do with
> the binary the end user is having trouble with, so we'd also want
> the version of the main part of "git", and other standalone "git"
> subprograms like "git-remote-curl", reported separately, probably
> together with $PATH, $GIT_EXEC_PATH and what appears in the
> directories listed on these environment variables.
> 
> If "git version --build-options" writes to its standard output
> stream, that is a good thing, as it makes it easy to spawn and read
> what it says via the run_command() API, and there is one less thing
> to worry about (it would be a bit more cumbersome if the output goes
> to the standard error stream).
> 
> As "git-remote-curl" would also be a separate binary, we'd have to
> use the same technique to report version number (and perhaps curl's
> library version and its configuration?), perhaps by teaching the
> subcommand a new option to dump such details to its standard output.
> 
> Using the same technique to report the version about the "git"
> itself would be consistent thing to do, as a preparation for future
> steps that reports the details about "git-remote-curl".

Sure. I will include a run_command() call to "git version
--build-options" (or is it --build-info?) in the same series where I
re-introduce the call to "git-remote-curl", since they will look pretty
similar; around that time I will clean up what "git-bugreport" reports
about its own version/build info, too.

To be clear, do you want me to include the output of get_compiler_info()
in "git version --build-options" when I do that change, too?

 - Emily

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 3/5] bugreport: gather git version and build info
  2020-04-07 18:42         ` Emily Shaffer
@ 2020-04-07 20:05           ` Junio C Hamano
  2020-04-07 20:34             ` Emily Shaffer
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2020-04-07 20:05 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: git

Emily Shaffer <emilyshaffer@google.com> writes:

> To be clear, do you want me to include the output of get_compiler_info()
> in "git version --build-options" when I do that change, too?

In the endgame, there are two kinds of information we'd want to
gather and report, I would think.

Ones that can be different per-binary are things like:

 - what version of the source code the binary was compiled from
 - with what compiler options
 - using which compiler and
 - linking with what libraries,
 - where in the filesystem is the binary located

The others are various properties of the system the user is using,
and having trouble using, Git on:

 - how many CPUs do we have,
 - how much free memory,
 - is the repository's filesystem case sensitive,
 - what version of 'wish' is being used.

We'd want the former to be reported for each binary that matters, so
"git version --build-options" would want to say it, "git remote-curl
--build-options" would want to say it, and being different binaries,
they may say different things.

There is not much point in duplicating the latter that are not
binary specific, so it probably makes sense to gather them inside,
and report them from, "git bugreport" itself.

Thanks.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 3/5] bugreport: gather git version and build info
  2020-04-07 20:05           ` Junio C Hamano
@ 2020-04-07 20:34             ` Emily Shaffer
  0 siblings, 0 replies; 32+ messages in thread
From: Emily Shaffer @ 2020-04-07 20:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Tue, Apr 07, 2020 at 01:05:02PM -0700, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > To be clear, do you want me to include the output of get_compiler_info()
> > in "git version --build-options" when I do that change, too?
> 
> In the endgame, there are two kinds of information we'd want to
> gather and report, I would think.
> 
> Ones that can be different per-binary are things like:
> 
>  - what version of the source code the binary was compiled from
>  - with what compiler options
>  - using which compiler and
>  - linking with what libraries,
>  - where in the filesystem is the binary located
> 
> The others are various properties of the system the user is using,
> and having trouble using, Git on:
> 
>  - how many CPUs do we have,
>  - how much free memory,
>  - is the repository's filesystem case sensitive,
>  - what version of 'wish' is being used.
> 
> We'd want the former to be reported for each binary that matters, so
> "git version --build-options" would want to say it, "git remote-curl
> --build-options" would want to say it, and being different binaries,
> they may say different things.
> 
> There is not much point in duplicating the latter that are not
> binary specific, so it probably makes sense to gather them inside,
> and report them from, "git bugreport" itself.

Sure. Sounds great. Thanks for laying it out, this is very clear.

 - Emily

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2020-04-07 20:34 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-23 21:43 [PATCH v10 0/5] add git-bugreport tool Emily Shaffer
2020-03-23 21:43 ` [PATCH v10 1/5] help: move list_config_help to builtin/help Emily Shaffer
2020-03-23 21:43 ` [PATCH v10 2/5] bugreport: add tool to generate debugging info Emily Shaffer
2020-03-23 21:43 ` [PATCH v10 3/5] bugreport: gather git version and build info Emily Shaffer
2020-03-23 21:43 ` [PATCH v10 4/5] bugreport: add uname info Emily Shaffer
2020-03-23 21:43 ` [PATCH v10 5/5] bugreport: add compiler info Emily Shaffer
2020-03-23 23:10   ` Junio C Hamano
2020-03-25 21:10     ` Emily Shaffer
2020-03-25 21:24 ` [PATCH v11 0/5] add git-bugreport tool Emily Shaffer
2020-03-25 21:24   ` [PATCH v11 1/5] help: move list_config_help to builtin/help Emily Shaffer
2020-03-25 21:24   ` [PATCH v11 2/5] bugreport: add tool to generate debugging info Emily Shaffer
2020-04-01 20:17     ` SZEDER Gábor
2020-03-25 21:24   ` [PATCH v11 3/5] bugreport: gather git version and build info Emily Shaffer
2020-03-25 21:24   ` [PATCH v11 4/5] bugreport: add uname info Emily Shaffer
2020-03-25 21:25   ` [PATCH v11 5/5] bugreport: add compiler info Emily Shaffer
2020-04-06 22:45   ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Emily Shaffer
2020-04-06 22:45     ` [PATCH v12 1/5] help: move list_config_help to builtin/help Emily Shaffer
2020-04-06 23:04       ` Junio C Hamano
2020-04-06 22:45     ` [PATCH v12 2/5] bugreport: add tool to generate debugging info Emily Shaffer
2020-04-06 22:45     ` [PATCH v12 3/5] bugreport: gather git version and build info Emily Shaffer
2020-04-06 23:17       ` Junio C Hamano
2020-04-07 18:42         ` Emily Shaffer
2020-04-07 20:05           ` Junio C Hamano
2020-04-07 20:34             ` Emily Shaffer
2020-04-06 22:45     ` [PATCH v12 4/5] bugreport: add uname info Emily Shaffer
2020-04-06 23:20       ` Junio C Hamano
2020-04-06 22:45     ` [PATCH v12 5/5] bugreport: add compiler info Emily Shaffer
2020-04-06 23:31       ` Junio C Hamano
2020-04-06 23:47       ` Danh Doan
2020-04-07 15:09         ` Junio C Hamano
2020-04-07 18:35           ` Emily Shaffer
2020-04-07 15:24     ` [PATCH v12 0/5] bugreport: add tool to generate debugging info Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).