Message ID | 20191213004312.169753-10-emilyshaffer@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v4,01/15] bugreport: add tool to generate debugging info | expand |
Emily Shaffer <emilyshaffer@google.com> writes: > diff --git a/Makefile b/Makefile > index c49f55a521..76dc51e2b1 100644 > --- a/Makefile > +++ b/Makefile > @@ -651,7 +651,7 @@ install-perl-script: $(SCRIPT_PERL_GEN) > install-python-script: $(SCRIPT_PYTHON_GEN) > $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > -.PHONY: clean-perl-script clean-sh-script clean-python-script > +.PHONY: clean-perl-script clean-sh-script clean-python-script clean-script-dependencies > clean-sh-script: > $(RM) $(SCRIPT_SH_GEN) > clean-perl-script: > @@ -817,6 +817,7 @@ VCSSVN_LIB = vcs-svn/lib.a > > GENERATED_H += config-list.h > GENERATED_H += command-list.h > +GENERATED_H += bugreport-config-safelist.h OK. > LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \ > $(FIND) . \ > @@ -2161,6 +2162,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 $@+ $@ OK. > 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):\ But bugreport.o needs this *.h file generated before a compiler attempts to produce it out of bugreport.c; that dependency is missing from this update to the Makefile. > @@ -2791,7 +2798,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE > .PHONY: sparse $(SP_OBJ) > sparse: $(SP_OBJ) > > -GEN_HDRS := config-list.h command-list.h unicode-width.h > +GEN_HDRS := config-list.h command-list.h unicode-width.h bugreport-config-safelist.h > EXCEPT_HDRS := $(GEN_HDRS) compat/% xdiff/% > ifndef GCRYPT_SHA256 > EXCEPT_HDRS += sha256/gcrypt.h > @@ -3117,7 +3124,8 @@ clean: profile-clean coverage-clean cocciclean > $(RM) $(HCC) > $(RM) -r bin-wrappers $(dep_dirs) > $(RM) -r po/build/ > - $(RM) *.pyc *.pyo */*.pyc */*.pyo config-list.h command-list.h > + $(RM) *.pyc *.pyo */*.pyc */*.pyo > + $(RM) config-list.h command-list.h bugreport-config-safelist.h It is kind of sad that GEN_HDRS defines the list of build artifact that we should be able to clean, and then we manually list them to be removed here independently. Can we fix it up too? We probably clean up the build/update procedure around unicode-width.h before we can do so, probably. It may be generatable using contrib/ script, but as far as our normal build is concerned, it is a tracked source and not part of the build byproducts, so we probably would want to remove it from GEN_HDRS. When that happens, we can $(RM) all of the $(GEN_HDRS) in the "clean" target.
Hi Emily, On Thu, 12 Dec 2019, Emily Shaffer wrote: > diff --git a/generate-bugreport-config-safelist.sh b/generate-bugreport-config-safelist.sh > new file mode 100755 > index 0000000000..06b8e0c3c4 > --- /dev/null > +++ b/generate-bugreport-config-safelist.sh > @@ -0,0 +1,22 @@ > +#!/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 bugreport-include macro > +sed -n 's/^\(.*\) \+bugreport:include.* ::$/\1/p' | If you use `/ "\1",/` as replacement, you can totally avoid that ugly `while` loop (that is unfortunately quite slow in MSYS2/Cygwin). You can still pipe the result into `sort` just the same. Ciao, Dscho > +sort | > +while read line > +do > + echo " \"$line\"," > +done > + > +cat <<EOF > +}; > +EOF > -- > 2.24.1.735.g03f4e72817-goog > >
On Sun, Dec 15, 2019 at 09:17:57PM +0100, Johannes Schindelin wrote: > Hi Emily, > > On Thu, 12 Dec 2019, Emily Shaffer wrote: > > > diff --git a/generate-bugreport-config-safelist.sh b/generate-bugreport-config-safelist.sh > > new file mode 100755 > > index 0000000000..06b8e0c3c4 > > --- /dev/null > > +++ b/generate-bugreport-config-safelist.sh > > @@ -0,0 +1,22 @@ > > +#!/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 bugreport-include macro > > +sed -n 's/^\(.*\) \+bugreport:include.* ::$/\1/p' | > > If you use `/ "\1",/` as replacement, you can totally avoid that ugly > `while` loop (that is unfortunately quite slow in MSYS2/Cygwin). You can > still pipe the result into `sort` just the same. Oooof. How embarrassing - I will do that.
On Fri, Dec 13, 2019 at 02:57:17PM -0800, Junio C Hamano wrote: > Emily Shaffer <emilyshaffer@google.com> writes: > > > diff --git a/Makefile b/Makefile > > index c49f55a521..76dc51e2b1 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -651,7 +651,7 @@ install-perl-script: $(SCRIPT_PERL_GEN) > > install-python-script: $(SCRIPT_PYTHON_GEN) > > $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > > > -.PHONY: clean-perl-script clean-sh-script clean-python-script > > +.PHONY: clean-perl-script clean-sh-script clean-python-script clean-script-dependencies > > clean-sh-script: > > $(RM) $(SCRIPT_SH_GEN) > > clean-perl-script: > > @@ -817,6 +817,7 @@ VCSSVN_LIB = vcs-svn/lib.a > > > > GENERATED_H += config-list.h > > GENERATED_H += command-list.h > > +GENERATED_H += bugreport-config-safelist.h > > OK. > > > LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \ > > $(FIND) . \ > > @@ -2161,6 +2162,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 $@+ $@ > > OK. > > > 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):\ > > But bugreport.o needs this *.h file generated before a compiler > attempts to produce it out of bugreport.c; that dependency is > missing from this update to the Makefile. Hm. Somewhere I misformatted my commits, then. My goal was to add the new header here, but not add it as a dependency to anybody; then, in the next commit, add it as a dependency to git-bugreport and start to use it (commit 1: make the thing; commit 2: use the thing). But now when I look at the next commit, I don't see a Makefile change. So I missed something while I was shuffling around fixups. Thanks for noticing. > > > @@ -2791,7 +2798,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE > > .PHONY: sparse $(SP_OBJ) > > sparse: $(SP_OBJ) > > > > -GEN_HDRS := config-list.h command-list.h unicode-width.h > > +GEN_HDRS := config-list.h command-list.h unicode-width.h bugreport-config-safelist.h > > EXCEPT_HDRS := $(GEN_HDRS) compat/% xdiff/% > > ifndef GCRYPT_SHA256 > > EXCEPT_HDRS += sha256/gcrypt.h > > @@ -3117,7 +3124,8 @@ clean: profile-clean coverage-clean cocciclean > > $(RM) $(HCC) > > $(RM) -r bin-wrappers $(dep_dirs) > > $(RM) -r po/build/ > > - $(RM) *.pyc *.pyo */*.pyc */*.pyo config-list.h command-list.h > > + $(RM) *.pyc *.pyo */*.pyc */*.pyo > > + $(RM) config-list.h command-list.h bugreport-config-safelist.h > > It is kind of sad that GEN_HDRS defines the list of build artifact > that we should be able to clean, and then we manually list them to > be removed here independently. Can we fix it up too? Yeah, I can do that. I thought so too when I was updating this. I *think* what happened is that when I split config-list away (in the earleir commit in this chain) I didn't notice that command-list.h was already in GEN_HDRS, and instead just grepped Makefile for "command-list.h" and added config-list.h next to it. So I'll try to fix it much earlier, in that commit, and then simply add to the appropriate variables in this commit. > > We probably clean up the build/update procedure around unicode-width.h > before we can do so, probably. It may be generatable using contrib/ > script, but as far as our normal build is concerned, it is a tracked > source and not part of the build byproducts, so we probably would > want to remove it from GEN_HDRS. When that happens, we can $(RM) > all of the $(GEN_HDRS) in the "clean" target. Noted. Thanks. - Emily
On Mon, Dec 16, 2019 at 03:01:24PM -0800, Emily Shaffer wrote: > On Fri, Dec 13, 2019 at 02:57:17PM -0800, Junio C Hamano wrote: > > Emily Shaffer <emilyshaffer@google.com> writes: > > > > > diff --git a/Makefile b/Makefile > > > index c49f55a521..76dc51e2b1 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -651,7 +651,7 @@ install-perl-script: $(SCRIPT_PERL_GEN) > > > install-python-script: $(SCRIPT_PYTHON_GEN) > > > $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > > > > > -.PHONY: clean-perl-script clean-sh-script clean-python-script > > > +.PHONY: clean-perl-script clean-sh-script clean-python-script clean-script-dependencies > > > clean-sh-script: > > > $(RM) $(SCRIPT_SH_GEN) > > > clean-perl-script: > > > @@ -817,6 +817,7 @@ VCSSVN_LIB = vcs-svn/lib.a > > > > > > GENERATED_H += config-list.h > > > GENERATED_H += command-list.h > > > +GENERATED_H += bugreport-config-safelist.h > > > > OK. > > > > > LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \ > > > $(FIND) . \ > > > @@ -2161,6 +2162,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 $@+ $@ > > > > OK. > > > > > 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):\ > > > > But bugreport.o needs this *.h file generated before a compiler > > attempts to produce it out of bugreport.c; that dependency is > > missing from this update to the Makefile. > > Hm. Somewhere I misformatted my commits, then. My goal was to add the > new header here, but not add it as a dependency to anybody; then, in the > next commit, add it as a dependency to git-bugreport and start to use it > (commit 1: make the thing; commit 2: use the thing). But now when I look > at the next commit, I don't see a Makefile change. So I missed something > while I was shuffling around fixups. Thanks for noticing. > > > > > > @@ -2791,7 +2798,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE > > > .PHONY: sparse $(SP_OBJ) > > > sparse: $(SP_OBJ) > > > > > > -GEN_HDRS := config-list.h command-list.h unicode-width.h > > > +GEN_HDRS := config-list.h command-list.h unicode-width.h bugreport-config-safelist.h > > > EXCEPT_HDRS := $(GEN_HDRS) compat/% xdiff/% > > > ifndef GCRYPT_SHA256 > > > EXCEPT_HDRS += sha256/gcrypt.h > > > @@ -3117,7 +3124,8 @@ clean: profile-clean coverage-clean cocciclean > > > $(RM) $(HCC) > > > $(RM) -r bin-wrappers $(dep_dirs) > > > $(RM) -r po/build/ > > > - $(RM) *.pyc *.pyo */*.pyc */*.pyo config-list.h command-list.h > > > + $(RM) *.pyc *.pyo */*.pyc */*.pyo > > > + $(RM) config-list.h command-list.h bugreport-config-safelist.h > > > > It is kind of sad that GEN_HDRS defines the list of build artifact > > that we should be able to clean, and then we manually list them to > > be removed here independently. Can we fix it up too? > > Yeah, I can do that. I thought so too when I was updating this. > > I *think* what happened is that when I split config-list away (in the > earleir commit in this chain) I didn't notice that command-list.h was > already in GEN_HDRS, and instead just grepped Makefile for > "command-list.h" and added config-list.h next to it. So I'll try to fix > it much earlier, in that commit, and then simply add to the appropriate > variables in this commit. > > > > > We probably clean up the build/update procedure around unicode-width.h > > before we can do so, probably. It may be generatable using contrib/ > > script, but as far as our normal build is concerned, it is a tracked > > source and not part of the build byproducts, so we probably would > > want to remove it from GEN_HDRS. When that happens, we can $(RM) > > all of the $(GEN_HDRS) in the "clean" target. > > Noted. Thanks. Ah, after I sent this mail I saw your patch. For those playing along at home, https://lore.kernel.org/xmqq1rt7hkp6.fsf@gitster-ct.c.googlers.com Will review. - Emily
Hi Emily, On Thu, 12 Dec 2019, Emily Shaffer wrote: > diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb > index d906a00803..750bdff9af 100644 > --- a/Documentation/asciidoctor-extensions.rb > +++ b/Documentation/asciidoctor-extensions.rb > @@ -37,6 +37,10 @@ module Git > output = output.sub(/<\/refmeta>/, new_tags + "</refmeta>") > end > output > + > + class BugReportProcessor < Asciidoctor::Extensions::InlineMacroProcessor > + def process(parent, action, attrs) > + "" The Azure Pipeline fails on this hunk, saying: /usr/bin/asciidoctor: /home/vsts/work/1/s/Documentation/asciidoctor-extensions.rb:41: class definition in method body (SyntaxError) /home/vsts/work/1/s/Documentation/asciidoctor-extensions.rb:55: syntax error, unexpected end-of-input, expecting keyword_end See https://dev.azure.com/gitgitgadget/git/_build/results?buildId=23830&view=logs&jobId=253e5128-1058-5bd4-fdf1-9b556d3207f8&j=253e5128-1058-5bd4-fdf1-9b556d3207f8&t=95c65aec-3627-54dc-fc17-47625f123bb3 for details. Ciao, Dscho > end > end > end > @@ -45,4 +49,7 @@ end > Asciidoctor::Extensions.register do > inline_macro Git::Documentation::LinkGitProcessor, :linkgit > postprocessor Git::Documentation::DocumentPostProcessor > + # The bugreport macro does nothing as far as rendering is > + # concerned -- we just grep for it in the sources. > + inline_macro Git::Documentation::BugReportProcessor, :bugreport > end > diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt > index 0006faf800..92f5082013 100644 > --- a/Documentation/config/sendemail.txt > +++ b/Documentation/config/sendemail.txt > @@ -1,63 +1,63 @@ > -sendemail.identity:: > +sendemail.identity bugreport:exclude[x] :: > A configuration identity. When given, causes values in the > 'sendemail.<identity>' subsection to take precedence over > values in the 'sendemail' section. The default identity is > the value of `sendemail.identity`. > > -sendemail.smtpEncryption:: > +sendemail.smtpEncryption bugreport:include[x] :: > See linkgit:git-send-email[1] for description. Note that this > setting is not subject to the 'identity' mechanism. > > -sendemail.smtpssl (deprecated):: > +sendemail.smtpssl (deprecated) bugreport:exclude[x] :: > Deprecated alias for 'sendemail.smtpEncryption = ssl'. > > -sendemail.smtpsslcertpath:: > +sendemail.smtpsslcertpath bugreport:exclude[x] :: > Path to ca-certificates (either a directory or a single file). > Set it to an empty string to disable certificate verification. > > -sendemail.<identity>.*:: > +sendemail.<identity>.* bugreport:exclude[x] :: > Identity-specific versions of the 'sendemail.*' parameters > found below, taking precedence over those when this > identity is selected, through either the command-line or > `sendemail.identity`. > > -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.aliasesFile bugreport:exclude[x] :: > +sendemail.aliasFileType bugreport:exclude[x] :: > +sendemail.annotate bugreport:include[x] :: > +sendemail.bcc bugreport:include[x] :: > +sendemail.cc bugreport:include[x] :: > +sendemail.ccCmd bugreport:include[x] :: > +sendemail.chainReplyTo bugreport:include[x] :: > +sendemail.confirm bugreport:include[x] :: > +sendemail.envelopeSender bugreport:include[x] :: > +sendemail.from bugreport:include[x] :: > +sendemail.multiEdit bugreport:include[x] :: > +sendemail.signedoffbycc bugreport:include[x] :: > +sendemail.smtpPass bugreport:exclude[x] :: > +sendemail.suppresscc bugreport:include[x] :: > +sendemail.suppressFrom bugreport:include[x] :: > +sendemail.to bugreport:include[x] :: > +sendemail.tocmd bugreport:include[x] :: > +sendemail.smtpDomain bugreport:include[x] :: > +sendemail.smtpServer bugreport:include[x] :: > +sendemail.smtpServerPort bugreport:include[x] :: > +sendemail.smtpServerOption bugreport:include[x] :: > +sendemail.smtpUser bugreport:exclude[x] :: > +sendemail.thread bugreport:include[x] :: > +sendemail.transferEncoding bugreport:include[x] :: > +sendemail.validate bugreport:include[x] :: > +sendemail.xmailer bugreport:include[x] :: > See linkgit:git-send-email[1] for description. > > -sendemail.signedoffcc (deprecated):: > +sendemail.signedoffcc (deprecated) bugreport:exclude[x] :: > Deprecated alias for `sendemail.signedoffbycc`. > > -sendemail.smtpBatchSize:: > +sendemail.smtpBatchSize bugreport:include[x] :: > 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 bugreport:include[x] :: > 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 c49f55a521..76dc51e2b1 100644 > --- a/Makefile > +++ b/Makefile > @@ -651,7 +651,7 @@ install-perl-script: $(SCRIPT_PERL_GEN) > install-python-script: $(SCRIPT_PYTHON_GEN) > $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' > > -.PHONY: clean-perl-script clean-sh-script clean-python-script > +.PHONY: clean-perl-script clean-sh-script clean-python-script clean-script-dependencies > clean-sh-script: > $(RM) $(SCRIPT_SH_GEN) > clean-perl-script: > @@ -817,6 +817,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) . \ > @@ -2161,6 +2162,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):\ > @@ -2791,7 +2798,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE > .PHONY: sparse $(SP_OBJ) > sparse: $(SP_OBJ) > > -GEN_HDRS := config-list.h command-list.h unicode-width.h > +GEN_HDRS := config-list.h command-list.h unicode-width.h bugreport-config-safelist.h > EXCEPT_HDRS := $(GEN_HDRS) compat/% xdiff/% > ifndef GCRYPT_SHA256 > EXCEPT_HDRS += sha256/gcrypt.h > @@ -3117,7 +3124,8 @@ clean: profile-clean coverage-clean cocciclean > $(RM) $(HCC) > $(RM) -r bin-wrappers $(dep_dirs) > $(RM) -r po/build/ > - $(RM) *.pyc *.pyo */*.pyc */*.pyo config-list.h command-list.h > + $(RM) *.pyc *.pyo */*.pyc */*.pyo > + $(RM) config-list.h command-list.h bugreport-config-safelist.h > $(RM) $(ETAGS_TARGET) tags cscope* > $(RM) -r $(GIT_TARNAME) .doc-tmp-dir > $(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz > diff --git a/generate-bugreport-config-safelist.sh b/generate-bugreport-config-safelist.sh > new file mode 100755 > index 0000000000..06b8e0c3c4 > --- /dev/null > +++ b/generate-bugreport-config-safelist.sh > @@ -0,0 +1,22 @@ > +#!/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 bugreport-include macro > +sed -n 's/^\(.*\) \+bugreport:include.* ::$/\1/p' | > +sort | > +while read line > +do > + echo " \"$line\"," > +done > + > +cat <<EOF > +}; > +EOF > -- > 2.24.1.735.g03f4e72817-goog > > >
diff --git a/.gitignore b/.gitignore index 5dde2cc4c8..30935621d9 100644 --- a/.gitignore +++ b/.gitignore @@ -191,6 +191,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 8fc4b67081..5d5359fcf9 100644 --- a/Documentation/asciidoc.conf +++ b/Documentation/asciidoc.conf @@ -6,9 +6,13 @@ # # Show Git link as: <command>(<section>); if section is defined, else just show # the command. +# +# The bugreport macro does nothing as far as rendering is +# concerned -- we just grep for it in the sources. [macros] (?su)[\\]?(?P<name>linkgit):(?P<target>\S*?)\[(?P<attrlist>.*?)\]= +(?su)[\\]?(?P<name>bugreport):(?P<action>\S*?)\[(?P<attrlist>.*?)\]= [attributes] asterisk=* @@ -28,6 +32,8 @@ ifdef::backend-docbook[] {0#<citerefentry>} {0#<refentrytitle>{target}</refentrytitle><manvolnum>{0}</manvolnum>} {0#</citerefentry>} +[bugreport-inlinemacro] +{0#} endif::backend-docbook[] ifdef::backend-docbook[] @@ -94,4 +100,6 @@ ifdef::backend-xhtml11[] git-relative-html-prefix= [linkgit-inlinemacro] <a href="{git-relative-html-prefix}{target}.html">{target}{0?({0})}</a> +[bugreport-inlinemacro] +<!-- --> endif::backend-xhtml11[] diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb index d906a00803..750bdff9af 100644 --- a/Documentation/asciidoctor-extensions.rb +++ b/Documentation/asciidoctor-extensions.rb @@ -37,6 +37,10 @@ module Git output = output.sub(/<\/refmeta>/, new_tags + "</refmeta>") end output + + class BugReportProcessor < Asciidoctor::Extensions::InlineMacroProcessor + def process(parent, action, attrs) + "" end end end @@ -45,4 +49,7 @@ end Asciidoctor::Extensions.register do inline_macro Git::Documentation::LinkGitProcessor, :linkgit postprocessor Git::Documentation::DocumentPostProcessor + # The bugreport macro does nothing as far as rendering is + # concerned -- we just grep for it in the sources. + inline_macro Git::Documentation::BugReportProcessor, :bugreport end diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt index 0006faf800..92f5082013 100644 --- a/Documentation/config/sendemail.txt +++ b/Documentation/config/sendemail.txt @@ -1,63 +1,63 @@ -sendemail.identity:: +sendemail.identity bugreport:exclude[x] :: A configuration identity. When given, causes values in the 'sendemail.<identity>' subsection to take precedence over values in the 'sendemail' section. The default identity is the value of `sendemail.identity`. -sendemail.smtpEncryption:: +sendemail.smtpEncryption bugreport:include[x] :: See linkgit:git-send-email[1] for description. Note that this setting is not subject to the 'identity' mechanism. -sendemail.smtpssl (deprecated):: +sendemail.smtpssl (deprecated) bugreport:exclude[x] :: Deprecated alias for 'sendemail.smtpEncryption = ssl'. -sendemail.smtpsslcertpath:: +sendemail.smtpsslcertpath bugreport:exclude[x] :: Path to ca-certificates (either a directory or a single file). Set it to an empty string to disable certificate verification. -sendemail.<identity>.*:: +sendemail.<identity>.* bugreport:exclude[x] :: Identity-specific versions of the 'sendemail.*' parameters found below, taking precedence over those when this identity is selected, through either the command-line or `sendemail.identity`. -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.aliasesFile bugreport:exclude[x] :: +sendemail.aliasFileType bugreport:exclude[x] :: +sendemail.annotate bugreport:include[x] :: +sendemail.bcc bugreport:include[x] :: +sendemail.cc bugreport:include[x] :: +sendemail.ccCmd bugreport:include[x] :: +sendemail.chainReplyTo bugreport:include[x] :: +sendemail.confirm bugreport:include[x] :: +sendemail.envelopeSender bugreport:include[x] :: +sendemail.from bugreport:include[x] :: +sendemail.multiEdit bugreport:include[x] :: +sendemail.signedoffbycc bugreport:include[x] :: +sendemail.smtpPass bugreport:exclude[x] :: +sendemail.suppresscc bugreport:include[x] :: +sendemail.suppressFrom bugreport:include[x] :: +sendemail.to bugreport:include[x] :: +sendemail.tocmd bugreport:include[x] :: +sendemail.smtpDomain bugreport:include[x] :: +sendemail.smtpServer bugreport:include[x] :: +sendemail.smtpServerPort bugreport:include[x] :: +sendemail.smtpServerOption bugreport:include[x] :: +sendemail.smtpUser bugreport:exclude[x] :: +sendemail.thread bugreport:include[x] :: +sendemail.transferEncoding bugreport:include[x] :: +sendemail.validate bugreport:include[x] :: +sendemail.xmailer bugreport:include[x] :: See linkgit:git-send-email[1] for description. -sendemail.signedoffcc (deprecated):: +sendemail.signedoffcc (deprecated) bugreport:exclude[x] :: Deprecated alias for `sendemail.signedoffbycc`. -sendemail.smtpBatchSize:: +sendemail.smtpBatchSize bugreport:include[x] :: 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 bugreport:include[x] :: 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 c49f55a521..76dc51e2b1 100644 --- a/Makefile +++ b/Makefile @@ -651,7 +651,7 @@ install-perl-script: $(SCRIPT_PERL_GEN) install-python-script: $(SCRIPT_PYTHON_GEN) $(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' -.PHONY: clean-perl-script clean-sh-script clean-python-script +.PHONY: clean-perl-script clean-sh-script clean-python-script clean-script-dependencies clean-sh-script: $(RM) $(SCRIPT_SH_GEN) clean-perl-script: @@ -817,6 +817,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) . \ @@ -2161,6 +2162,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):\ @@ -2791,7 +2798,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE .PHONY: sparse $(SP_OBJ) sparse: $(SP_OBJ) -GEN_HDRS := config-list.h command-list.h unicode-width.h +GEN_HDRS := config-list.h command-list.h unicode-width.h bugreport-config-safelist.h EXCEPT_HDRS := $(GEN_HDRS) compat/% xdiff/% ifndef GCRYPT_SHA256 EXCEPT_HDRS += sha256/gcrypt.h @@ -3117,7 +3124,8 @@ clean: profile-clean coverage-clean cocciclean $(RM) $(HCC) $(RM) -r bin-wrappers $(dep_dirs) $(RM) -r po/build/ - $(RM) *.pyc *.pyo */*.pyc */*.pyo config-list.h command-list.h + $(RM) *.pyc *.pyo */*.pyc */*.pyo + $(RM) config-list.h command-list.h bugreport-config-safelist.h $(RM) $(ETAGS_TARGET) tags cscope* $(RM) -r $(GIT_TARNAME) .doc-tmp-dir $(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz diff --git a/generate-bugreport-config-safelist.sh b/generate-bugreport-config-safelist.sh new file mode 100755 index 0000000000..06b8e0c3c4 --- /dev/null +++ b/generate-bugreport-config-safelist.sh @@ -0,0 +1,22 @@ +#!/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 bugreport-include macro +sed -n 's/^\(.*\) \+bugreport:include.* ::$/\1/p' | +sort | +while read line +do + echo " \"$line\"," +done + +cat <<EOF +}; +EOF
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 "bugreport" macro, which is recognized by 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. In order to prevent staleness of the safelist, add a mechanism to generate the safelist from annotations in the config documentation, where contributors are already used to documenting their new config keys. Implement a new no-op "bugreport" macro for use as "bugreport:include[x]" to annotate the config keys that should be included in the automatically generated safelist. Use "exclude" for the others. With Asciidoctor, it's ok to say "bugreport:include[]", but AsciiDoc seems to want something between the brackets. A bit unfortunate, but not a huge problem -- we'll just provide an "x". "doc-diff" reports that this macro doesn't render at all. That is, these are both empty after this commit: cd Documentation ./doc-diff --asciidoctor :/"bugreport: add tool" HEAD ./doc-diff --asciidoc :/"bugreport: add tool" HEAD Diffing the rendered HTML shows that there is some small amount of whitespace and comments added. That shouldn't be a problem. We could perhaps let the implementation verify that the "action" is one of "include" and "exclude". For the Asciidoctor implementation that should be straightforward, but for AsciiDoc I don't immediately know how to do it. Anyway, if someone stumbles on the keyboard and writes "bugreport:icndule", they'll "only" miss out on the config key being included in the safelist. If this were a blocklist, the consequences of a misspelled target could be a lot more severe. Additionally, add annotations to the sendemail config documentation in order to demonstrate a proof of concept. Helped-by: Martin Ă…gren <martin.agren@gmail.com> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- .gitignore | 1 + Documentation/asciidoc.conf | 8 +++ Documentation/asciidoctor-extensions.rb | 7 +++ Documentation/config/sendemail.txt | 68 ++++++++++++------------- Makefile | 14 +++-- generate-bugreport-config-safelist.sh | 22 ++++++++ 6 files changed, 83 insertions(+), 37 deletions(-) create mode 100755 generate-bugreport-config-safelist.sh