From 69f54afc6bd4aef6f42dc4a96227cfd69f8f8e8e Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Tue, 19 Nov 2019 16:39:54 -0800 Subject: [PATCH 01/15] help: move list_config_help to builtin/help 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 --- .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 aebe7c0908f168..ea97de83f39500 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 6134104ae65b71..c552312d3f30d1 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 e5590d7787c50a..1c5f2b9255fc2e 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", "", list_config_color_branch_slots }, + { "color.decorate", "", list_config_color_decorate_slots }, + { "color.diff", "", list_config_color_diff_slots }, + { "color.grep", "", list_config_color_grep_slots }, + { "color.interactive", "", list_config_color_interactive_slots }, + { "color.remote", "", list_config_color_sideband_slots }, + { "color.status", "", list_config_color_status_slots }, + { "fsck", "", list_config_fsck_msg_ids }, + { "receive.fsck", "", 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 71158f7d8ba9c7..45fecf8bdfb21a 100755 --- a/generate-cmdlist.sh +++ b/generate-cmdlist.sh @@ -76,23 +76,6 @@ print_command_list () { echo "};" } -print_config_list () { - cat <", list_config_color_branch_slots }, - { "color.decorate", "", list_config_color_decorate_slots }, - { "color.diff", "", list_config_color_diff_slots }, - { "color.grep", "", list_config_color_grep_slots }, - { "color.interactive", "", list_config_color_interactive_slots }, - { "color.remote", "", list_config_color_sideband_slots }, - { "color.status", "", list_config_color_status_slots }, - { "fsck", "", list_config_fsck_msg_ids }, - { "receive.fsck", "", 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 7a455beeb725e2..9071894e8cf4a4 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); From 9ea1c3ab340d38fb6935f9ae4a558bc6c4b36057 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 11 Dec 2019 15:05:30 -0800 Subject: [PATCH 02/15] help: add shell-path to --build-options It may be useful to know which shell Git was built to try to point to, in the event that shell-based Git commands are failing. $SHELL_PATH is set during the build and used to launch the manpage viewer, as well as by git-compat-util.h, and it's used during tests. 'git version --build-options' is encouraged for use in bug reports, so it makes sense to include this information there. Signed-off-by: Emily Shaffer --- help.c | 1 + 1 file changed, 1 insertion(+) diff --git a/help.c b/help.c index a21487db773c2d..190722fb0aae90 100644 --- a/help.c +++ b/help.c @@ -654,6 +654,7 @@ int cmd_version(int argc, const char **argv, const char *prefix) 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)); + printf("shell-path: %s\n", SHELL_PATH); /* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */ } return 0; From 0b1fb7e9c64b2798585832c17e553a69e9296210 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 9 Oct 2019 17:17:58 -0700 Subject: [PATCH 03/15] bugreport: add tool to generate debugging info 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 --- .gitignore | 1 + Documentation/git-bugreport.txt | 46 ++++++++++++++++ Makefile | 5 ++ bugreport.c | 95 +++++++++++++++++++++++++++++++++ command-list.txt | 1 + strbuf.c | 4 ++ strbuf.h | 1 + t/t0091-bugreport.sh | 61 +++++++++++++++++++++ 8 files changed, 214 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 ea97de83f39500..d89bf9e11eec6c 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 00000000000000..1f9fde5cde0a71 --- /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) ] [(-s | --suffix) ] + +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 :: +--output-directory :: + Place the resulting bug report file in `` instead of the root of + the Git repository. + +-s :: +--suffix :: + Specify an alternate suffix for the bugreport name, to create a file + named 'git-bugreport-'. 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 c552312d3f30d1..9e6705061df083 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 00000000000000..3e9e2f50b1c542 --- /dev/null +++ b/bugreport.c @@ -0,0 +1,95 @@ +#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 ] [-s|--suffix ]"), + NULL +}; + +static int get_bug_template(struct strbuf *template) +{ + const char template_text[] = N_( +"Thank you for filling out a Git bug report!\n" +"Please answer the following questions to help us understand your issue.\n" +"\n" +"What did you do before the bug happened? (Steps to reproduce your issue)\n" +"\n" +"What did you expect to happen? (Expected behavior)\n" +"\n" +"What happened instead? (Actual behavior)\n" +"\n" +"What's different between what you expected and what actually happened?\n" +"\n" +"Anything else you want to add:\n" +"\n" +"Please review the rest of the bug report below.\n" +"You can delete any lines you don't wish to share.\n"); + + strbuf_addstr(template, template_text); + return 0; +} + +int cmd_main(int argc, const char **argv) +{ + struct strbuf buffer = STRBUF_INIT; + struct strbuf report_path = STRBUF_INIT; + int report = -1; + time_t now = time(NULL); + char *option_output = NULL; + char *option_suffix = "%F-%H%M"; + int nongit_ok = 0; + + const 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() + }; + + /* Prerequisite for hooks and config checks */ + setup_git_directory_gently(&nongit_ok); + + argc = parse_options(argc, argv, NULL, bugreport_options, + bugreport_usage, 0); + + if (option_output) { + strbuf_addstr(&report_path, 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); + } + + 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); + + fprintf(stderr, _("Created new report at '%s'.\n"), report_path.buf); + + UNLEAK(buffer); + UNLEAK(report_path); + return !!launch_editor(report_path.buf, NULL, NULL); +} diff --git a/command-list.txt b/command-list.txt index 20878946558847..185e5e3f05fbc4 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 f19da55b0783dc..f1d66c7848e4dc 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 aae7ac3a82da1a..bbf6204de726cb 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 00000000000000..65f664fdac25a2 --- /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-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 From c6a3abf1a561da89739810468f1afd5b36985ede Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 8 Nov 2019 11:28:41 -0800 Subject: [PATCH 04/15] bugreport: gather git version and build info 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 --- Documentation/git-bugreport.txt | 4 +++ bugreport.c | 19 ++++++++++++- help.c | 48 ++++++++++++++++++++------------- help.h | 1 + 4 files changed, 52 insertions(+), 20 deletions(-) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index 1f9fde5cde0a71..f44ae8cbe7544b 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 3e9e2f50b1c542..4e2efd9c2fbc08 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 ] [-s|--suffix ]"), @@ -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; @@ -76,6 +90,9 @@ int cmd_main(int argc, const char **argv) 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 190722fb0aae90..44cee69c11c683 100644 --- a/help.c +++ b/help.c @@ -622,8 +622,33 @@ 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)); + strbuf_addf(buf, "shell-path: %s\n", SHELL_PATH); + /* 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 []"), @@ -637,26 +662,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)); - printf("shell-path: %s\n", SHELL_PATH); - /* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */ - } return 0; } diff --git a/help.h b/help.h index 9071894e8cf4a4..500521b9081c37 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 From 1e9df68da283f38375a1448f80429f59a24fee88 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Thu, 14 Nov 2019 14:28:44 -0800 Subject: [PATCH 05/15] bugreport: add uname info 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 --- Documentation/git-bugreport.txt | 1 + bugreport.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index f44ae8cbe7544b..17b0d14e8d5134 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 4e2efd9c2fbc08..694806a8c5fab7 100644 --- a/bugreport.c +++ b/bugreport.c @@ -7,10 +7,25 @@ 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[] = { From 240511032d4a859420b9269763ebd67e3a36ccf1 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Thu, 14 Nov 2019 14:38:10 -0800 Subject: [PATCH 06/15] bugreport: add compiler info 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 --- Documentation/git-bugreport.txt | 1 + bugreport.c | 5 +++++ compat/compiler.h | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 compat/compiler.h diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index 17b0d14e8d5134..643d1b288420f6 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 694806a8c5fab7..38047e3d8e2582 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) { @@ -26,6 +27,10 @@ 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_complete_line(sys_info); } static const char * const bugreport_usage[] = { diff --git a/compat/compiler.h b/compat/compiler.h new file mode 100644 index 00000000000000..309cd3830bcce0 --- /dev/null +++ b/compat/compiler.h @@ -0,0 +1,22 @@ +#ifndef COMPILER_H +#define COMPILER_H + +#include "git-compat-util.h" +#include "strbuf.h" + +#ifdef __GLIBC__ +#include +#endif + +static inline void get_compiler_info(struct strbuf *info) +{ +#ifdef __GLIBC__ + strbuf_addf(info, "glibc: %s\n", gnu_get_libc_version()); +#endif + +#ifdef __GNUC__ + strbuf_addf(info, "gnuc: %d.%d\n", __GNUC__, __GNUC_MINOR__); +#endif +} + +#endif /* COMPILER_H */ From b6d936a756e078af2442815a9c33d205bff731b3 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Thu, 14 Nov 2019 14:31:13 -0800 Subject: [PATCH 07/15] bugreport: add git-remote-https version It's possible for git-remote-curl to be built separately from git; in that case we want to know what version of cURL is used by git-remote-curl, not necessarily which version was present at git-bugreport's build time. So instead, ask git-remote-curl for the version information it knows about. Today, "git-remote-http" and "git-remote-https" are aliased to "git-remote-curl"; but in case we rely on a different library than cURL in the future, let's not explicitly reference cURL from bugreport. For longevity purposes, invoke the alias "git-remote-https" instead of "git-remote-http". Since it could have been built at a different time, also report the version and built-from commit of git-remote-curl alongside the cURL info. Signed-off-by: Emily Shaffer --- Documentation/git-bugreport.txt | 1 + bugreport.c | 16 ++++++++++++++++ remote-curl.c | 8 ++++++++ 3 files changed, 25 insertions(+) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index 643d1b288420f6..8c7cb9a5d4c6a9 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -28,6 +28,7 @@ The following information is captured automatically: - 'git version --build-options' - uname sysname, release, version, and machine strings - Compiler-specific info string + - 'git remote-https --build-info' 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 38047e3d8e2582..f26b51a83cbc7d 100644 --- a/bugreport.c +++ b/bugreport.c @@ -5,6 +5,18 @@ #include "time.h" #include "help.h" #include "compat/compiler.h" +#include "run-command.h" + +static void get_git_remote_https_version_info(struct strbuf *version_info) +{ + struct child_process cp = CHILD_PROCESS_INIT; + + cp.git_cmd = 1; + argv_array_push(&cp.args, "remote-https"); + argv_array_push(&cp.args, "--build-info"); + if (capture_command(&cp, version_info, 0)) + strbuf_addstr(version_info, "'git-remote-https --build-info' not supported\n"); +} static void get_system_info(struct strbuf *sys_info) { @@ -31,6 +43,10 @@ static void get_system_info(struct strbuf *sys_info) strbuf_addstr(sys_info, "compiler info: "); get_compiler_info(sys_info); strbuf_complete_line(sys_info); + + strbuf_addstr(sys_info, "git-remote-https --build-info:\n"); + get_git_remote_https_version_info(sys_info); + strbuf_complete_line(sys_info); } static const char * const bugreport_usage[] = { diff --git a/remote-curl.c b/remote-curl.c index 8eb96152f51b64..73e52175c05c3e 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -17,6 +17,7 @@ #include "protocol.h" #include "quote.h" #include "transport.h" +#include "version.h" static struct remote *remote; /* always ends with a trailing slash */ @@ -1375,6 +1376,13 @@ int cmd_main(int argc, const char **argv) string_list_init(&options.deepen_not, 1); string_list_init(&options.push_options, 1); + if (!strcmp("--build-info", argv[1])) { + printf("git-http-fetch version: %s\n", git_version_string); + printf("built from commit: %s\n", git_built_from_commit_string); + printf("curl version: %s\n", curl_version()); + return 0; + } + /* * Just report "remote-curl" here (folding all the various aliases * ("git-remote-http", "git-remote-https", and etc.) here since they From 4d9d8b1b8ad543121a0d0a9d7774397143e17af7 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 11 Dec 2019 13:34:53 -0800 Subject: [PATCH 08/15] bugreport: include user interactive shell It's possible a user may complain about the way that Git interacts with their interactive shell, e.g. autocompletion or shell prompt. In that case, it's useful for us to know which shell they're using interactively. Signed-off-by: Emily Shaffer --- Documentation/git-bugreport.txt | 1 + bugreport.c | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index 8c7cb9a5d4c6a9..f7a13c0374488b 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -29,6 +29,7 @@ The following information is captured automatically: - uname sysname, release, version, and machine strings - Compiler-specific info string - 'git remote-https --build-info' + - $SHELL 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 f26b51a83cbc7d..80eb41f14d7232 100644 --- a/bugreport.c +++ b/bugreport.c @@ -21,6 +21,7 @@ static void get_git_remote_https_version_info(struct strbuf *version_info) static void get_system_info(struct strbuf *sys_info) { struct utsname uname_info; + char *shell = NULL; /* get git version from native cmd */ strbuf_addstr(sys_info, "git version:\n"); @@ -44,6 +45,10 @@ static void get_system_info(struct strbuf *sys_info) get_compiler_info(sys_info); strbuf_complete_line(sys_info); + shell = getenv("SHELL"); + strbuf_addf(sys_info, "$SHELL (typically, interactive shell): %s\n", + shell ? shell : ""); + strbuf_addstr(sys_info, "git-remote-https --build-info:\n"); get_git_remote_https_version_info(sys_info); strbuf_complete_line(sys_info); From 1f5c2d1eb649bf8b64a78ea422e2756e5b6ed7af Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 16 Aug 2019 16:24:35 -0700 Subject: [PATCH 09/15] bugreport: generate config safelist based on docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new step to the build to generate a safelist of git-config variables which are appropriate to include in the output of git-bugreport. New variables can be added to the safelist by annotating their documentation in Documentation/config with the "annotate" macro, which is a no-op in AsciiDoc and AsciiDoctor. Some configs are private in nature, and can contain remote URLs, passwords, or other sensitive information. In the event that a user doesn't notice their information while reviewing a bugreport, that user may leak their credentials to other individuals, mailing lists, or bug tracking tools inadvertently. Heuristic blocklisting of configuration keys is imperfect and prone to false negatives; given the nature of the information which can be leaked, a safelist is more reliable. However, it's possible that in some situations, an organization may be less concerned with privacy of things like remote URLs and branch names, and more concerned with ease of diagnosis for their support staff. In those cases, it may make more sense for that organization to modify the code to use a blocklist. To that end, we should try to mark configs which are definitely safe, and configs which are definitely unsafe, and leave blank configs which are somewhere in between. To mark a config as safe, add "annotate:bugreport[include]" to the corresponding line in the config documentation; to mark it as unsafe, add "annotate:bugreport[exclude]" instead. Generating bugreport-config-safelist.h at build time by grepping the documentation for this new macro helps us prevent staleness. The macro itself is a no-op and should not alter the appearance of the documentation in either AsciiDoc or AsciiDoctor, confirmable by running: cd Documentation ./doc-diff --asciidoctor HEAD^ HEAD ./doc-diff --asciidoc HEAD^ HEAD Diffing the rendered HTML shows that only inline comments were added, which shouldn't be a problem. Additionally, add annotations to the sendemail config documentation in order to demonstrate a proof of concept. Helped-by: Martin Ågren Helped-by: Johannes Schindelin Signed-off-by: Emily Shaffer --- .gitignore | 1 + Documentation/asciidoc.conf | 9 ++++ Documentation/asciidoctor-extensions.rb | 7 ++++ Documentation/config/sendemail.txt | 56 ++++++++++++------------- Makefile | 7 ++++ generate-bugreport-config-safelist.sh | 18 ++++++++ 6 files changed, 70 insertions(+), 28 deletions(-) create mode 100755 generate-bugreport-config-safelist.sh diff --git a/.gitignore b/.gitignore index d89bf9e11eec6c..bd2f49b99620f2 100644 --- a/.gitignore +++ b/.gitignore @@ -192,6 +192,7 @@ /gitweb/static/gitweb.min.* /config-list.h /command-list.h +/bugreport-config-safelist.h *.tar.gz *.dsc *.deb diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf index 8fc4b67081a649..663e06481fc5db 100644 --- a/Documentation/asciidoc.conf +++ b/Documentation/asciidoc.conf @@ -6,9 +6,14 @@ # # Show Git link as: (
); if section is defined, else just show # the command. +# +# The annotate macro does nothing as far as rendering is +# concerned -- we just grep for it in the sources to populate +# things like the bugreport safelist. [macros] (?su)[\\]?(?Plinkgit):(?P\S*?)\[(?P.*?)\]= +(?su)[\\]?(?Pannotate):(?P\S*?)\[(?P.*?)\]= [attributes] asterisk=* @@ -28,6 +33,8 @@ ifdef::backend-docbook[] {0#} {0#{target}{0}} {0#} +[annotate-inlinemacro] +{0#} endif::backend-docbook[] ifdef::backend-docbook[] @@ -94,4 +101,6 @@ ifdef::backend-xhtml11[] git-relative-html-prefix= [linkgit-inlinemacro] {target}{0?({0})} +[annotate-inlinemacro] + endif::backend-xhtml11[] diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb index d906a008039cf5..03c80af0e5463b 100644 --- a/Documentation/asciidoctor-extensions.rb +++ b/Documentation/asciidoctor-extensions.rb @@ -39,10 +39,17 @@ def process document, output output end end + + class AnnotateProcessor < Asciidoctor::Extensions::InlineMacroProcessor + def process(parent, target, attrs) + "" + end + end end end Asciidoctor::Extensions.register do inline_macro Git::Documentation::LinkGitProcessor, :linkgit postprocessor Git::Documentation::DocumentPostProcessor + inline_macro Git::Documentation::AnnotateProcessor, :annotate end diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt index 0006faf800ae69..fe27473e44948e 100644 --- a/Documentation/config/sendemail.txt +++ b/Documentation/config/sendemail.txt @@ -4,7 +4,7 @@ sendemail.identity:: values in the 'sendemail' section. The default identity is the value of `sendemail.identity`. -sendemail.smtpEncryption:: +sendemail.smtpEncryption annotate:bugreport[include] :: See linkgit:git-send-email[1] for description. Note that this setting is not subject to the 'identity' mechanism. @@ -15,7 +15,7 @@ sendemail.smtpsslcertpath:: Path to ca-certificates (either a directory or a single file). Set it to an empty string to disable certificate verification. -sendemail..*:: +sendemail..* annotate:bugreport[exclude] :: Identity-specific versions of the 'sendemail.*' parameters found below, taking precedence over those when this identity is selected, through either the command-line or @@ -23,41 +23,41 @@ sendemail..*:: sendemail.aliasesFile:: sendemail.aliasFileType:: -sendemail.annotate:: -sendemail.bcc:: -sendemail.cc:: -sendemail.ccCmd:: -sendemail.chainReplyTo:: -sendemail.confirm:: -sendemail.envelopeSender:: -sendemail.from:: -sendemail.multiEdit:: -sendemail.signedoffbycc:: -sendemail.smtpPass:: -sendemail.suppresscc:: -sendemail.suppressFrom:: -sendemail.to:: -sendemail.tocmd:: -sendemail.smtpDomain:: -sendemail.smtpServer:: -sendemail.smtpServerPort:: -sendemail.smtpServerOption:: -sendemail.smtpUser:: -sendemail.thread:: -sendemail.transferEncoding:: -sendemail.validate:: -sendemail.xmailer:: +sendemail.annotate annotate:bugreport[include] :: +sendemail.bcc annotate:bugreport[include] :: +sendemail.cc annotate:bugreport[include] :: +sendemail.ccCmd annotate:bugreport[include] :: +sendemail.chainReplyTo annotate:bugreport[include] :: +sendemail.confirm annotate:bugreport[include] :: +sendemail.envelopeSender annotate:bugreport[include] :: +sendemail.from annotate:bugreport[include] :: +sendemail.multiEdit annotate:bugreport[include] :: +sendemail.signedoffbycc annotate:bugreport[include] :: +sendemail.smtpPass annotate:bugreport[exclude] :: +sendemail.suppresscc annotate:bugreport[include] :: +sendemail.suppressFrom annotate:bugreport[include] :: +sendemail.to annotate:bugreport[include] :: +sendemail.tocmd annotate:bugreport[include] :: +sendemail.smtpDomain annotate:bugreport[include] :: +sendemail.smtpServer annotate:bugreport[include] :: +sendemail.smtpServerPort annotate:bugreport[include] :: +sendemail.smtpServerOption annotate:bugreport[include] :: +sendemail.smtpUser annotate:bugreport[exclude] :: +sendemail.thread annotate:bugreport[include] :: +sendemail.transferEncoding annotate:bugreport[include] :: +sendemail.validate annotate:bugreport[include] :: +sendemail.xmailer annotate:bugreport[include] :: See linkgit:git-send-email[1] for description. sendemail.signedoffcc (deprecated):: Deprecated alias for `sendemail.signedoffbycc`. -sendemail.smtpBatchSize:: +sendemail.smtpBatchSize annotate:bugreport[include] :: Number of messages to be sent per connection, after that a relogin will happen. If the value is 0 or undefined, send all messages in one connection. See also the `--batch-size` option of linkgit:git-send-email[1]. -sendemail.smtpReloginDelay:: +sendemail.smtpReloginDelay annotate:bugreport[include] :: Seconds wait before reconnecting to smtp server. See also the `--relogin-delay` option of linkgit:git-send-email[1]. diff --git a/Makefile b/Makefile index 9e6705061df083..6bdd3b93377afd 100644 --- a/Makefile +++ b/Makefile @@ -818,6 +818,7 @@ VCSSVN_LIB = vcs-svn/lib.a GENERATED_H += config-list.h GENERATED_H += command-list.h +GENERATED_H += bugreport-config-safelist.h LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \ $(FIND) . \ @@ -2167,6 +2168,12 @@ command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Doc $(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \ command-list.txt >$@+ && mv $@+ $@ +bugreport-config-safelist.h: generate-bugreport-config-safelist.sh + +bugreport-config-safelist.h: Documentation/config/*.txt + $(QUIET_GEN)$(SHELL_PATH) ./generate-bugreport-config-safelist.sh \ + >$@+ && mv $@+ $@ + SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\ $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\ $(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\ diff --git a/generate-bugreport-config-safelist.sh b/generate-bugreport-config-safelist.sh new file mode 100755 index 00000000000000..57ebf3bb57ab94 --- /dev/null +++ b/generate-bugreport-config-safelist.sh @@ -0,0 +1,18 @@ +#!/bin/sh + +cat <<"EOF" +/* Automatically generated by bugreport-generate-config-safelist.sh */ + + +static const char *bugreport_config_safelist[] = { +EOF + +# cat all regular files in Documentation/config +find Documentation/config -type f -exec cat {} \; | +# print the command name which matches the annotate-bugreport macro +sed -n 's/^\([^ ]*\) *annotate:bugreport\[include\].* ::$/ "\1",/p' | + sort + +cat < Date: Wed, 9 Oct 2019 17:28:56 -0700 Subject: [PATCH 10/15] bugreport: add config values from safelist Teach bugreport to gather the values of config options which are present in 'bugreport-config-safelist.h', and show their origin scope. Many config options are sensitive, and many Git add-ons use config options which git-core does not know about; it is better only to gather config options which we know to be safe, rather than excluding options which we know to be unsafe. Rather than including the path to someone's config, which can reveal filesystem layout and project names, just name the scope (e.g. system, global, local) of the config source. Signed-off-by: Emily Shaffer --- Documentation/git-bugreport.txt | 1 + Makefile | 2 ++ bugreport.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index f7a13c0374488b..efe9719ea4fb8b 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -30,6 +30,7 @@ The following information is captured automatically: - Compiler-specific info string - 'git remote-https --build-info' - $SHELL + - Selected config values 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/Makefile b/Makefile index 6bdd3b93377afd..bf4f3ede2aae14 100644 --- a/Makefile +++ b/Makefile @@ -2135,6 +2135,8 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) help.sp help.s help.o: command-list.h +bugreport.sp bugreport.s bugreport.o: bugreport-config-safelist.h + 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)"' \ diff --git a/bugreport.c b/bugreport.c index 80eb41f14d7232..a3d3004b4e82d1 100644 --- a/bugreport.c +++ b/bugreport.c @@ -6,6 +6,9 @@ #include "help.h" #include "compat/compiler.h" #include "run-command.h" +#include "config.h" +#include "bugreport-config-safelist.h" +#include "khash.h" static void get_git_remote_https_version_info(struct strbuf *version_info) { @@ -54,6 +57,29 @@ static void get_system_info(struct strbuf *sys_info) strbuf_complete_line(sys_info); } +static void get_safelisted_config(struct strbuf *config_info) +{ + size_t idx; + struct string_list_item *it = NULL; + struct key_value_info *kv_info = NULL; + + for (idx = 0; idx < ARRAY_SIZE(bugreport_config_safelist); idx++) { + const struct string_list *list = + git_config_get_value_multi(bugreport_config_safelist[idx]); + + if (!list) + continue; + + strbuf_addf(config_info, "%s:\n", bugreport_config_safelist[idx]); + for_each_string_list_item(it, list) { + kv_info = it->util; + strbuf_addf(config_info, " %s (%s)\n", it->string, + kv_info ? config_scope_name(kv_info->scope) + : "source unknown"); + } + } +} + static const char * const bugreport_usage[] = { N_("git bugreport [-o|--output-directory ] [-s|--suffix ]"), NULL @@ -134,6 +160,9 @@ int cmd_main(int argc, const char **argv) get_header(&buffer, "System Info"); get_system_info(&buffer); + get_header(&buffer, "Safelisted Config Info"); + get_safelisted_config(&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); From 195f29d25406173bc08283f9fd36bbfc5351c2b2 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Mon, 14 Oct 2019 16:36:44 -0700 Subject: [PATCH 11/15] bugreport: collect list of populated hooks Occasionally a failure a user is seeing may be related to a specific hook which is being run, perhaps without the user realizing. While the contents of hooks can be sensitive - containing user data or process information specific to the user's organization - simply knowing that a hook is being run at a certain stage can help us to understand whether something is going wrong. Without a definitive list of hook names within the code, we compile our own list from the documentation. This is likely prone to bitrot. To reduce the amount of code humans need to read, we turn the list into a string_list and iterate over it (as we are calling the same find_hook operation on each string). However, since bugreport should primarily be called by the user, the performance loss from massaging the string seems acceptable. Signed-off-by: Emily Shaffer --- Documentation/git-bugreport.txt | 1 + bugreport.c | 53 +++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index efe9719ea4fb8b..4d015285406cd1 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -31,6 +31,7 @@ The following information is captured automatically: - 'git remote-https --build-info' - $SHELL - Selected config values + - A list of enabled hooks 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 a3d3004b4e82d1..82becd563d2c69 100644 --- a/bugreport.c +++ b/bugreport.c @@ -9,6 +9,7 @@ #include "config.h" #include "bugreport-config-safelist.h" #include "khash.h" +#include "run-command.h" static void get_git_remote_https_version_info(struct strbuf *version_info) { @@ -80,6 +81,55 @@ static void get_safelisted_config(struct strbuf *config_info) } } +static void get_populated_hooks(struct strbuf *hook_info, int nongit) +{ + /* + * Doesn't look like there is a list of all possible hooks; so below is + * a transcription of `git help hooks`. + */ + const char *hooks = "applypatch-msg," + "pre-applypatch," + "post-applypatch," + "pre-commit," + "pre-merge-commit," + "prepare-commit-msg," + "commit-msg," + "post-commit," + "pre-rebase," + "post-checkout," + "post-merge," + "pre-push," + "pre-receive," + "update," + "post-receive," + "post-update," + "push-to-checkout," + "pre-auto-gc," + "post-rewrite," + "sendemail-validate," + "fsmonitor-watchman," + "p4-pre-submit," + "post-index-changex"; + struct string_list hooks_list = STRING_LIST_INIT_DUP; + struct string_list_item *iter = NULL; + + + if (nongit) { + strbuf_addstr(hook_info, + "not run from a git repository - no hooks to show\n"); + return; + } + + string_list_split(&hooks_list, hooks, ',', -1); + + for_each_string_list_item(iter, &hooks_list) { + if (find_hook(iter->string)) { + strbuf_addstr(hook_info, iter->string); + strbuf_complete_line(hook_info); + } + } +} + static const char * const bugreport_usage[] = { N_("git bugreport [-o|--output-directory ] [-s|--suffix ]"), NULL @@ -163,6 +213,9 @@ int cmd_main(int argc, const char **argv) get_header(&buffer, "Safelisted Config Info"); get_safelisted_config(&buffer); + get_header(&buffer, "Enabled Hooks"); + get_populated_hooks(&buffer, nongit_ok); + /* fopen doesn't offer us an O_EXCL alternative, except with glibc. */ report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666); From 63981ec71aef94e4ebc17886ac713ec06ddfd35e Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Tue, 10 Dec 2019 16:00:07 -0800 Subject: [PATCH 12/15] bugreport: count loose objects The number of unpacked objects in a user's repository may help us understand the root of the problem they're seeing, especially if a command is running unusually slowly. Helped-by: Johannes Schindelin Signed-off-by: Emily Shaffer --- Documentation/git-bugreport.txt | 1 + bugreport.c | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index 4d015285406cd1..4fe1c605061726 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -32,6 +32,7 @@ The following information is captured automatically: - $SHELL - Selected config values - A list of enabled hooks + - The number of loose objects in the repository 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 82becd563d2c69..d030d7a92c1d3b 100644 --- a/bugreport.c +++ b/bugreport.c @@ -10,6 +10,7 @@ #include "bugreport-config-safelist.h" #include "khash.h" #include "run-command.h" +#include "object-store.h" static void get_git_remote_https_version_info(struct strbuf *version_info) { @@ -130,6 +131,54 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit) } } +static int loose_object_cb(const struct object_id *oid, const char *path, + void *data) { + int *loose_object_count = data; + + if (loose_object_count) { + (*loose_object_count)++; + return 0; + } + + return 1; +} + +static void get_loose_object_summary(struct strbuf *obj_info, int nongit) { + + int local_loose_object_count = 0, total_loose_object_count = 0; + int local_count_questionable = 0, total_count_questionable = 0; + + if (nongit) { + strbuf_addstr(obj_info, + "not run from a git repository - no objects to show\n"); + return; + } + + local_count_questionable = for_each_loose_object( + loose_object_cb, + &local_loose_object_count, + FOR_EACH_OBJECT_LOCAL_ONLY); + + total_count_questionable = for_each_loose_object( + loose_object_cb, + &total_loose_object_count, + 0); + + strbuf_addf(obj_info, "%d local loose objects%s\n", + local_loose_object_count, + local_count_questionable ? " (problem during count)" : ""); + + strbuf_addf(obj_info, "%d alternate loose objects%s\n", + total_loose_object_count - local_loose_object_count, + (local_count_questionable || total_count_questionable) + ? " (problem during count)" + : ""); + + strbuf_addf(obj_info, "%d total loose objects%s\n", + total_loose_object_count, + total_count_questionable ? " (problem during count)" : ""); +} + static const char * const bugreport_usage[] = { N_("git bugreport [-o|--output-directory ] [-s|--suffix ]"), NULL @@ -216,6 +265,9 @@ int cmd_main(int argc, const char **argv) get_header(&buffer, "Enabled Hooks"); get_populated_hooks(&buffer, nongit_ok); + get_header(&buffer, "Loose Object Counts"); + get_loose_object_summary(&buffer, nongit_ok); + /* fopen doesn't offer us an O_EXCL alternative, except with glibc. */ report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666); From 54cb6bb9fcb094cdc67b0925eba9ca5e415b2587 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Mon, 21 Oct 2019 19:52:34 -0700 Subject: [PATCH 13/15] bugreport: add packed object summary Alongside the loose object counts, it can be useful to show the number of packs and packed objects. This way we can check whether the repo has an appropriate ratio of packed to loose objects to help determine whether it's behaving correctly. Add a utility to easily traverse all packfiles in a given repository. Use it in packfile.c and remove a redundant call to prepare_packed_git(), which is already called in get_all_packs(). Signed-off-by: Emily Shaffer --- Documentation/git-bugreport.txt | 1 + bugreport.c | 30 ++++++++++++++++++++++++++++++ object-store.h | 6 ++++++ packfile.c | 3 +-- 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index 4fe1c605061726..eb41f0677fce7f 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -33,6 +33,7 @@ The following information is captured automatically: - Selected config values - A list of enabled hooks - The number of loose objects in the repository + - The number of packs and packed objects in the repository 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 d030d7a92c1d3b..be063060c04266 100644 --- a/bugreport.c +++ b/bugreport.c @@ -179,6 +179,33 @@ static void get_loose_object_summary(struct strbuf *obj_info, int nongit) { total_count_questionable ? " (problem during count)" : ""); } +static void get_packed_object_summary(struct strbuf *obj_info, int nongit) +{ + struct packed_git *pack = NULL; + int pack_count = 0; + int object_count = 0; + + if (nongit) { + strbuf_addstr(obj_info, + "not run from a git repository - no objects to show\n"); + return; + } + + for_each_pack(the_repository, pack) { + pack_count++; + /* + * To accurately count how many objects are packed, look inside + * the packfile's index. + */ + open_pack_index(pack); + object_count += pack->num_objects; + } + + strbuf_addf(obj_info, "%d total packs (%d objects)\n", pack_count, + object_count); + +} + static const char * const bugreport_usage[] = { N_("git bugreport [-o|--output-directory ] [-s|--suffix ]"), NULL @@ -268,6 +295,9 @@ int cmd_main(int argc, const char **argv) get_header(&buffer, "Loose Object Counts"); get_loose_object_summary(&buffer, nongit_ok); + get_header(&buffer, "Packed Object Summary"); + get_packed_object_summary(&buffer, nongit_ok); + /* 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/object-store.h b/object-store.h index 5b047637e3e02e..f881a1756ed0da 100644 --- a/object-store.h +++ b/object-store.h @@ -7,6 +7,7 @@ #include "sha1-array.h" #include "strbuf.h" #include "thread-utils.h" +#include "packfile.h" struct object_directory { struct object_directory *next; @@ -447,4 +448,9 @@ int for_each_object_in_pack(struct packed_git *p, int for_each_packed_object(each_packed_object_fn, void *, enum for_each_object_flags flags); +#define for_each_pack(repo, pack) \ + for (pack = get_all_packs(repo);\ + pack; \ + pack = pack->next) + #endif /* OBJECT_STORE_H */ diff --git a/packfile.c b/packfile.c index 99dd1a7d094fe8..95afcc1187bc81 100644 --- a/packfile.c +++ b/packfile.c @@ -2095,8 +2095,7 @@ int for_each_packed_object(each_packed_object_fn cb, void *data, int r = 0; int pack_errors = 0; - prepare_packed_git(the_repository); - for (p = get_all_packs(the_repository); p; p = p->next) { + for_each_pack(the_repository, p) { if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local) continue; if ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) && From 239faf2c0eeae99ed23cd7269a4f8fe1fde99dd8 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Mon, 21 Oct 2019 21:22:22 -0700 Subject: [PATCH 14/15] bugreport: list contents of $OBJDIR/info Miscellaneous information used about the object store can end up in .git/objects/info; this can help us understand what may be going on with the object store when the user is reporting a bug. Otherwise, it could be difficult to track down what is going wrong with an object which isn't kept locally to .git/objects/ or .git/objects/pack. Having some understanding of where the user's objects may be kept can save us some hops during the bug reporting process. Signed-off-by: Emily Shaffer --- Documentation/git-bugreport.txt | 1 + bugreport.c | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index eb41f0677fce7f..a7ef3360d29e3e 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -34,6 +34,7 @@ The following information is captured automatically: - A list of enabled hooks - The number of loose objects in the repository - The number of packs and packed objects in the repository + - A list of the contents of .git/objects/info (or equivalent) 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 be063060c04266..9e6d8772d2abe1 100644 --- a/bugreport.c +++ b/bugreport.c @@ -206,6 +206,57 @@ static void get_packed_object_summary(struct strbuf *obj_info, int nongit) } +static void list_contents_of_dir_recursively(struct strbuf *contents, + struct strbuf *dirpath) +{ + struct dirent *d; + DIR *dir; + size_t path_len; + + dir = opendir(dirpath->buf); + if (!dir) + return; + + strbuf_complete(dirpath, '/'); + path_len = dirpath->len; + + while ((d = readdir(dir))) { + if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, "..")) + continue; + + strbuf_addbuf(contents, dirpath); + strbuf_addstr(contents, d->d_name); + strbuf_complete_line(contents); + + if (d->d_type == DT_DIR) { + strbuf_addstr(dirpath, d->d_name); + list_contents_of_dir_recursively(contents, dirpath); + } + strbuf_setlen(dirpath, path_len); + } + + closedir(dir); +} + +static void get_object_info_summary(struct strbuf *obj_info, int nongit) +{ + struct strbuf dirpath = STRBUF_INIT; + + if (nongit) { + strbuf_addstr(obj_info, + "not run from a git repository - object info unavailable\n"); + return; + } + + strbuf_addstr(&dirpath, get_object_directory()); + strbuf_complete(&dirpath, '/'); + strbuf_addstr(&dirpath, "info/"); + + list_contents_of_dir_recursively(obj_info, &dirpath); + + strbuf_release(&dirpath); +} + static const char * const bugreport_usage[] = { N_("git bugreport [-o|--output-directory ] [-s|--suffix ]"), NULL @@ -298,6 +349,9 @@ int cmd_main(int argc, const char **argv) get_header(&buffer, "Packed Object Summary"); get_packed_object_summary(&buffer, nongit_ok); + get_header(&buffer, "Object Info Summary"); + get_object_info_summary(&buffer, nongit_ok); + /* fopen doesn't offer us an O_EXCL alternative, except with glibc. */ report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666); From c301beb46d5d7a7b9795ecc7a90390ab22a7d825 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Thu, 24 Oct 2019 17:15:50 -0700 Subject: [PATCH 15/15] bugreport: summarize contents of alternates file In some cases, it could be that the user is having a problem with an object which isn't present in their normal object directory. We can get a hint that that might be the case by examining the list of alternates where their object may be stored instead. Since paths to alternates may be sensitive, we'll instead count how many alternates have been specified and note how many of them exist or are broken. While object-cache.h describes a function "foreach_alt_odb()", this function does not provide information on broken alternates, which are skipped over in "link_alt_odb_entry()". Since the goal is to identify missing alternates, we can gather the contents of .git/objects/info/alternates manually. Signed-off-by: Emily Shaffer --- Documentation/git-bugreport.txt | 1 + bugreport.c | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt index a7ef3360d29e3e..1f60fb9456e60a 100644 --- a/Documentation/git-bugreport.txt +++ b/Documentation/git-bugreport.txt @@ -35,6 +35,7 @@ The following information is captured automatically: - The number of loose objects in the repository - The number of packs and packed objects in the repository - A list of the contents of .git/objects/info (or equivalent) + - The number of valid and invalid alternates 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 9e6d8772d2abe1..5687a952a91ad2 100644 --- a/bugreport.c +++ b/bugreport.c @@ -257,6 +257,48 @@ static void get_object_info_summary(struct strbuf *obj_info, int nongit) strbuf_release(&dirpath); } +static void get_alternates_summary(struct strbuf *alternates_info, int nongit) +{ + struct strbuf alternates_path = STRBUF_INIT; + struct strbuf alternate = STRBUF_INIT; + FILE *file; + size_t exists = 0, broken = 0; + + if (nongit) { + strbuf_addstr(alternates_info, + "not run from a git repository - alternates unavailable\n"); + return; + } + + strbuf_addstr(&alternates_path, get_object_directory()); + strbuf_complete(&alternates_path, '/'); + strbuf_addstr(&alternates_path, "info/alternates"); + + file = fopen(alternates_path.buf, "r"); + if (!file) { + strbuf_addstr(alternates_info, "No alternates file found.\n"); + strbuf_release(&alternates_path); + return; + } + + while (strbuf_getline(&alternate, file) != EOF) { + if (!access(alternate.buf, F_OK)) + exists++; + else + broken++; + } + + strbuf_addf(alternates_info, + "%zd alternates found (%zd working, %zd broken)\n", + exists + broken, + exists, + broken); + + fclose(file); + strbuf_release(&alternate); + strbuf_release(&alternates_path); +} + static const char * const bugreport_usage[] = { N_("git bugreport [-o|--output-directory ] [-s|--suffix ]"), NULL @@ -352,6 +394,9 @@ int cmd_main(int argc, const char **argv) get_header(&buffer, "Object Info Summary"); get_object_info_summary(&buffer, nongit_ok); + get_header(&buffer, "Alternates"); + get_alternates_summary(&buffer, nongit_ok); + /* fopen doesn't offer us an O_EXCL alternative, except with glibc. */ report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);