From 7a76f5c611c25785cbb49374c04d366c274a3936 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 21 Aug 2018 15:23:22 -0400 Subject: [PATCH 1/6] SubmittingPatches: mention doc-diff We already advise people to make sure their documentation formats correctly. Let's point them at the doc-diff script, which can help with that. Let's also put a brief note in the script about its purpose, since that otherwise can only be found in the original commit message. Along with the existing -h/usage text, that's hopefully enough for developers to make use of it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/SubmittingPatches | 4 +++- Documentation/doc-diff | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index b44fd51f27..ec8b205145 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -80,7 +80,9 @@ GitHub-Travis CI hints section for details. Do not forget to update the documentation to describe the updated behavior and make sure that the resulting documentation set formats -well. It is currently a liberal mixture of US and UK English norms for +well (try the Documentation/doc-diff script). + +We currently have a liberal mixture of US and UK English norms for spelling and grammar, which is somewhat unfortunate. A huge patch that touches the files all over the place only to correct the inconsistency is not welcome, though. Potential clashes with other changes that can diff --git a/Documentation/doc-diff b/Documentation/doc-diff index f483fe427c..6e285e648c 100755 --- a/Documentation/doc-diff +++ b/Documentation/doc-diff @@ -1,4 +1,12 @@ #!/bin/sh +# +# Build two documentation trees and diff the resulting formatted output. +# Compared to a source diff, this can reveal mistakes in the formatting. +# For example: +# +# ./doc-diff origin/master HEAD +# +# would show the differences introduced by a branch based on master. OPTIONS_SPEC="\ doc-diff [options] [-- ] From 27064fb7fb82d607f3e2ccc5e9c93e0161bae134 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 30 Aug 2018 04:12:03 -0400 Subject: [PATCH 2/6] doc-diff: always use oids inside worktree The doc-diff script immediately resolves its two endpoints to actual object ids, so that we can reuse cached results even if they appear under a different name. But we still use the original name the user fed us when running "git checkout" in our temporary worktree. This can lead to confusing results: - the namespace inside the worktree is different than the one outside. In particular, "./doc-diff origin HEAD" will resolve HEAD inside the worktree, whose detached HEAD will be pointing at origin! As a result, such a diff would always be empty. - worse, we will store this result under the oid we got by resolving HEAD in the main worktree, thus polluting our cache - we didn't pass --detach, which meant that using a branch name would cause us to actually check out that branch, making it unavailable to other worktrees. We can solve this by feeding the already-resolved object id to git-checkout. That naturally forces a detached HEAD, but just to make clear our expectation, let's explicitly pass --detach. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/doc-diff | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/doc-diff b/Documentation/doc-diff index 6e285e648c..c430fe7c99 100755 --- a/Documentation/doc-diff +++ b/Documentation/doc-diff @@ -82,7 +82,7 @@ generate_render_makefile () { done } -# render_tree +# render_tree render_tree () { # Skip install-man entirely if we already have an installed directory. # We can't rely on make here, since "install-man" unconditionally @@ -92,7 +92,7 @@ render_tree () { # through. if ! test -d "$tmp/installed/$1" then - git -C "$tmp/worktree" checkout "$2" && + git -C "$tmp/worktree" checkout --detach "$1" && make -j$parallel -C "$tmp/worktree" \ GIT_VERSION=omitted \ SOURCE_DATE_EPOCH=0 \ @@ -112,6 +112,6 @@ render_tree () { fi } -render_tree $from_oid "$from" && -render_tree $to_oid "$to" && +render_tree $from_oid && +render_tree $to_oid && git -C $tmp/rendered diff --no-index "$@" $from_oid $to_oid From 83d4b5ff29ee4a67e1a7269de5c9ec913a467bef Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 31 Aug 2018 02:33:16 -0400 Subject: [PATCH 3/6] doc-diff: fix non-portable 'man' invocation doc-diff invokes 'man' with the -l option to force "local" mode, however, neither MacOS nor FreeBSD recognize this option. On those platforms, if the argument to 'man' contains a slash, it is automatically interpreted as a file specification, so a "local"-like mode is not needed. And, it turns out, 'man' which does support -l falls back to enabling -l automatically if it can't otherwise find a manual entry corresponding to the argument. Since doc-diff always passes an absolute path of the nroff source file to 'man', the -l option kicks in anyhow, despite not being specified explicitly. Therefore, make the invocation portable to the various platforms by simply dropping -l. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/doc-diff | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/doc-diff b/Documentation/doc-diff index c430fe7c99..6ce040ea05 100755 --- a/Documentation/doc-diff +++ b/Documentation/doc-diff @@ -77,7 +77,7 @@ generate_render_makefile () { printf '%s: %s\n' "$dst" "$src" printf '\t@echo >&2 " RENDER $(notdir $@)" && \\\n' printf '\tmkdir -p $(dir $@) && \\\n' - printf '\tMANWIDTH=80 man -l $< >$@+ && \\\n' + printf '\tMANWIDTH=80 man $< >$@+ && \\\n' printf '\tmv $@+ $@\n' done } From ad51743007d408ba6f1f670126d57722bb397ce6 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 31 Aug 2018 02:33:17 -0400 Subject: [PATCH 4/6] doc-diff: add --clean mode to remove temporary working gunk As part of its operation, doc-diff creates a bunch of temporary working files and holds onto them in order to speed up subsequent invocations. These files are never deleted. Moreover, it creates a temporary working tree (via git-wortkree) which likewise never gets removed. Without knowing the implementation details of the tool, a user may not know how to clean up manually afterward. Worse, the user may find it surprising and alarming to discover a working tree which s/he did not create explicitly. To address these issues, add a --clean mode which removes the temporary working tree and deletes all generated files. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/doc-diff | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/Documentation/doc-diff b/Documentation/doc-diff index 6ce040ea05..cece4fd537 100755 --- a/Documentation/doc-diff +++ b/Documentation/doc-diff @@ -10,20 +10,25 @@ OPTIONS_SPEC="\ doc-diff [options] [-- ] +doc-diff (-c|--clean) -- j=n parallel argument to pass to make f force rebuild; do not rely on cached results +c,clean cleanup temporary working files " SUBDIRECTORY_OK=1 . "$(git --exec-path)/git-sh-setup" parallel= force= +clean= while test $# -gt 0 do case "$1" in -j) parallel=$2; shift ;; + -c|--clean) + clean=t ;; -f) force=t ;; --) @@ -34,6 +39,17 @@ do shift done +cd_to_toplevel +tmp=Documentation/tmp-doc-diff + +if test -n "$clean" +then + test $# -eq 0 || usage + git worktree remove --force "$tmp/worktree" 2>/dev/null + rm -rf "$tmp" + exit 0 +fi + if test -z "$parallel" then parallel=$(getconf _NPROCESSORS_ONLN 2>/dev/null) @@ -50,9 +66,6 @@ to=$1; shift from_oid=$(git rev-parse --verify "$from") || exit 1 to_oid=$(git rev-parse --verify "$to") || exit 1 -cd_to_toplevel -tmp=Documentation/tmp-doc-diff - if test -n "$force" then rm -rf "$tmp" From 6f924265a0bf6efa677e9a684cebdde958e5ba06 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 31 Aug 2018 02:33:18 -0400 Subject: [PATCH 5/6] doc/Makefile: drop doc-diff worktree and temporary files on "make clean" doc-diff creates a temporary working tree (git-worktree) and generates a bunch of temporary files which it does not remove since they act as a cache to speed up subsequent runs. Although doc-diff's working tree and generated files are not strictly build products of the Makefile (which, itself, never runs doc-diff), as a convenience, update "make clean" to clean up doc-diff's working tree and generated files along with other development detritus normally removed by "make clean". Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/Makefile b/Documentation/Makefile index d079d7c73a..623f1a866d 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -331,6 +331,7 @@ clean: $(RM) SubmittingPatches.txt $(RM) $(cmds_txt) $(mergetools_txt) *.made $(RM) manpage-base-url.xsl + '$(SHELL_PATH_SQ)' ./doc-diff --clean $(MAN_HTML): %.html : %.txt asciidoc.conf $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ From 6b6547b46f6f3d6895b61467604ee1f4d98b5a7f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 17 Sep 2018 12:46:18 -0700 Subject: [PATCH 6/6] Revert "doc/Makefile: drop doc-diff worktree and temporary files on "make clean"" This reverts commit 6f924265a0bf6efa677e9a684cebdde958e5ba06, which started to require that we have an executable git available in order to say "make clean", which gives us a chicken-and-egg problem. Having to have Git installed, or be in a repository, in order to be able to run an optional "doc-diff" tool is fine. Requiring either in order to run "make clean" is a different story. Reported by Jonathan Nieder . --- Documentation/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 623f1a866d..d079d7c73a 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -331,7 +331,6 @@ clean: $(RM) SubmittingPatches.txt $(RM) $(cmds_txt) $(mergetools_txt) *.made $(RM) manpage-base-url.xsl - '$(SHELL_PATH_SQ)' ./doc-diff --clean $(MAN_HTML): %.html : %.txt asciidoc.conf $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \