threads / patch / 50727

patchFix make check-docs on Windows

Subject: [PATCH 0/1] Fix make check-docs on Windows

## tl;dr

17 messages between Mar 13, 2019 and Mar 29, 2019. Diffs are folded; open one to read it.

replies: 16people: 4as markdown or json

Johannes Schindelin via GitGitGadget· Mar 13, 2019, 11:56 UTC · lore

I stumbled across this when investigating one of Duy's bugs in the CI builds.

Johannes Schindelin (1):
  check-docs: fix for setups where executables have an extension
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
base-commit: e902e9bcae2010bc42648c80ab6adc6c5a16a4a5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-162%2Fdscho%2Fcheck-docs-on-windows-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-162/dscho/check-docs-on-windows-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/162
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Mar 13, 2019, 11:56 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 1/1] check-docs: fix for setups where executables have an extension

From: Johannes Schindelin <johannes.schindelin@gmx.de>

On Windows, for example, executables (must) have the extension `.exe`. Our `check-docs` target was not prepared for that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to Makefile +2 −2
diff --git a/Makefile b/Makefile
index 537493822b..df56bf0cd1 100644
--- a/Makefile
+++ b/Makefile
@@ -3074,7 +3074,7 @@ ALL_COMMANDS += git-gui git-citool
 .PHONY: check-docs
 check-docs::
 	$(MAKE) -C Documentation lint-docs
-	@(for v in $(ALL_COMMANDS); \
+	@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \
 	do \
 		case "$$v" in \
 		git-merge-octopus | git-merge-ours | git-merge-recursive | \
@@ -3103,7 +3103,7 @@ check-docs::
 		    -e 's/\.txt//'; \
 	) | while read how cmd; \
 	do \
-		case " $(ALL_COMMANDS) " in \
+		case "  $(patsubst %$X,%,$(ALL_COMMANDS)) " in \
 		*" $$cmd "*)	;; \
 		*) echo "removed but $$how: $$cmd" ;; \
 		esac; \

-- 
gitgitgadget
Junio C Hamano· Mar 14, 2019, 01:54 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 1/1] check-docs: fix for setups where executables have an extension

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 7 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> On Windows, for example, executables (must) have the extension `.exe`.
> Our `check-docs` target was not prepared for that.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

My writing this loop certainly predates increased upstream activities for Windows port we see recently, and we saw little updates ever since the loop was written; I am kind of surprised it took this long for us to find the missing .exe support here.

Thanks.
Show 25 quoted lines
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 537493822b..df56bf0cd1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3074,7 +3074,7 @@ ALL_COMMANDS += git-gui git-citool
>  .PHONY: check-docs
>  check-docs::
>  	$(MAKE) -C Documentation lint-docs
> -	@(for v in $(ALL_COMMANDS); \
> +	@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \
>  	do \
>  		case "$$v" in \
>  		git-merge-octopus | git-merge-ours | git-merge-recursive | \
> @@ -3103,7 +3103,7 @@ check-docs::
>  		    -e 's/\.txt//'; \
>  	) | while read how cmd; \
>  	do \
> -		case " $(ALL_COMMANDS) " in \
> +		case "  $(patsubst %$X,%,$(ALL_COMMANDS)) " in \
>  		*" $$cmd "*)	;; \
>  		*) echo "removed but $$how: $$cmd" ;; \
>  		esac; \
Taylor Blau· Mar 22, 2019, 18:43 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 1/1] check-docs: fix for setups where executables have an extension

Hi Johannes,
On Wed, Mar 13, 2019 at 04:56:53AM -0700, Johannes Schindelin via GitGitGadget wrote:
Show 29 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> On Windows, for example, executables (must) have the extension `.exe`.
> Our `check-docs` target was not prepared for that.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 537493822b..df56bf0cd1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3074,7 +3074,7 @@ ALL_COMMANDS += git-gui git-citool
>  .PHONY: check-docs
>  check-docs::
>  	$(MAKE) -C Documentation lint-docs
> -	@(for v in $(ALL_COMMANDS); \
> +	@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \
>  	do \
>  		case "$$v" in \
>  		git-merge-octopus | git-merge-ours | git-merge-recursive | \
> @@ -3103,7 +3103,7 @@ check-docs::
>  		    -e 's/\.txt//'; \
>  	) | while read how cmd; \
>  	do \
> -		case " $(ALL_COMMANDS) " in \
> +		case "  $(patsubst %$X,%,$(ALL_COMMANDS)) " in \

I'm a little late to reading this thread, but I was curious why there are now two spaces around the patsubst as opposed to the one around ALL_COMMANDS?

Show 5 quoted lines
>  		*" $$cmd "*)	;; \
>  		*) echo "removed but $$how: $$cmd" ;; \
>  		esac; \
> --
> gitgitgadget

Thanks, Taylor

Johannes Schindelin· Mar 24, 2019, 10:02 UTC · re: Taylor Blau · lore

Re: [PATCH 1/1] check-docs: fix for setups where executables have an extension

Hi Taylor,
On Fri, 22 Mar 2019, Taylor Blau wrote:
Show 34 quoted lines
> On Wed, Mar 13, 2019 at 04:56:53AM -0700, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > On Windows, for example, executables (must) have the extension `.exe`.
> > Our `check-docs` target was not prepared for that.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  Makefile | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 537493822b..df56bf0cd1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -3074,7 +3074,7 @@ ALL_COMMANDS += git-gui git-citool
> >  .PHONY: check-docs
> >  check-docs::
> >  	$(MAKE) -C Documentation lint-docs
> > -	@(for v in $(ALL_COMMANDS); \
> > +	@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \
> >  	do \
> >  		case "$$v" in \
> >  		git-merge-octopus | git-merge-ours | git-merge-recursive | \
> > @@ -3103,7 +3103,7 @@ check-docs::
> >  		    -e 's/\.txt//'; \
> >  	) | while read how cmd; \
> >  	do \
> > -		case " $(ALL_COMMANDS) " in \
> > +		case "  $(patsubst %$X,%,$(ALL_COMMANDS)) " in \
>
> I'm a little late to reading this thread, but I was curious why there
> are now two spaces around the patsubst as opposed to the one around
> ALL_COMMANDS?

Sharp eyes, and a *very* good point. The double space is actually required for this patch to work as intended. I added the following explanation to the commit message:

    Note that `$(ALL_COMMANDS)` starts with a space, and that is rather
    crucial for the `while read how cmd` loop: some of the input lines do
    not actually have the form `<how> <cmd>`, but only `<cmd>`, therefore
    `$cmd` evaluates to the empty string, and when we are looking for
    `* $cmd *` in ` $(ALL_COMMANDS)`, we do find it because the latter
    starts with a double space.
    In other words, the double space helps us skip the lines that list
    only a command.
    But in this patch, we want to transform `$(ALL_COMMANDS)` via
    `$(patsubst ...)` which swallows that double space. To help that, we
    prefix the result with *two* spaces instead of just one.
Do you think that is good enough?
Oh, and can I ask you to review *all* of my patches in the future?

Thanks, Dscho

Junio C Hamano· Mar 24, 2019, 12:54 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/1] check-docs: fix for setups where executables have an extension

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 12 quoted lines
> Sharp eyes, and a *very* good point. The double space is actually required
> for this patch to work as intended. I added the following explanation to
> the commit message:
>
>     Note that `$(ALL_COMMANDS)` starts with a space, and that is rather
>     crucial for the `while read how cmd` loop: some of the input lines do
>     not actually have the form `<how> <cmd>`, but only `<cmd>`, therefore
>     `$cmd` evaluates to the empty string, and when we are looking for
>     `* $cmd *` in ` $(ALL_COMMANDS)`, we do find it because the latter
>     starts with a double space.
>     In other words, the double space helps us skip the lines that list
>     only a command.

That sounds more like a bug in the existing set-up even before your patch (in fact, these lines are probably from 2007 or so, long before you started touching our top-level Makefile heavily). If we want to ignore lines with only one token, I'd rather see it explicitly done, e.g.

	... generate data ... |
	while read how cmd
	do
		# skip a line with a single token
		case "$cmd" in "") continue ;; esac
		# is $cmd part of the known command list?
		case " $(ALL_COMMANDS) " in
		*" $cmd "*)	;; # skip
		*)	echo ... warning ... ;;
		esac
	done

instead of relying on "if $cmd is empty, then SP + $cmd + SP makes double space, so let's have double space somewhere in the list of known commands" cuteness.

Thanks.
Johannes Schindelin· Mar 25, 2019, 21:08 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] check-docs: fix for setups where executables have an extension

Hi Junio,
On Sun, 24 Mar 2019, Junio C Hamano wrote:
Show 37 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Sharp eyes, and a *very* good point. The double space is actually required
> > for this patch to work as intended. I added the following explanation to
> > the commit message:
> >
> >     Note that `$(ALL_COMMANDS)` starts with a space, and that is rather
> >     crucial for the `while read how cmd` loop: some of the input lines do
> >     not actually have the form `<how> <cmd>`, but only `<cmd>`, therefore
> >     `$cmd` evaluates to the empty string, and when we are looking for
> >     `* $cmd *` in ` $(ALL_COMMANDS)`, we do find it because the latter
> >     starts with a double space.
> >     In other words, the double space helps us skip the lines that list
> >     only a command.
>
> That sounds more like a bug in the existing set-up even before your
> patch (in fact, these lines are probably from 2007 or so, long
> before you started touching our top-level Makefile heavily).  If we
> want to ignore lines with only one token, I'd rather see it
> explicitly done, e.g.
>
> 	... generate data ... |
> 	while read how cmd
> 	do
> 		# skip a line with a single token
> 		case "$cmd" in "") continue ;; esac
>
> 		# is $cmd part of the known command list?
> 		case " $(ALL_COMMANDS) " in
> 		*" $cmd "*)	;; # skip
> 		*)	echo ... warning ... ;;
> 		esac
> 	done
>
> instead of relying on "if $cmd is empty, then SP + $cmd + SP makes
> double space, so let's have double space somewhere in the list of
> known commands" cuteness.
You are right, I should have root-caused it, it had a funny smell to it.

Turns out that we should not just skip those lines with a single token, but instead fix the bug that prevents the `how` variable to be set to `documented` in those cases, as had been intended all along.

I fixed the commit, and accompanied it with a bug fix for this, plus for the fall-out bugs that had been hidden by this bug.

Ciao, Dscho

Johannes Schindelin via GitGitGadget· Mar 25, 2019, 21:41 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 0/5] Fix make check-docs on Windows

I stumbled across this when investigating one of Duy's bugs in the CI builds.

Changes since v1:
 * We no longer add an extra space in front of the $(patsubst ...) in the
   second loop, but fix the underlying bug that prevented check-docs' part
   from working where it discovered documented commands that are actually
   not installed.
 * As a fall-out, the gitremote-helpers page, which does not refer to a
   command, but to a concept, was moved from section 1 to section 7.
 * As a second fall-out, the documentation for git remote-testgit, which is 
   a command, but not installed, was removed.
 * As a "while at it" patch, the part of the check-docs target that tried to
   identify commands that are listed in the help, but not implemented, was
   fixed to no longer mistake guides like gitattributes and gitcli for
   commands.
Johannes Schindelin (5):
  docs: move gitremote-helpers into section 7
  docs: do not document the `git remote-testgit` command
  check-docs: really look at the documented commands again
  check-docs: do not expect guide pages to correspond to commands
  check-docs: fix for setups where executables have an extension
 Documentation/Makefile                |  2 +-
 Documentation/git-remote-ext.txt      |  2 +-
 Documentation/git-remote-fd.txt       |  2 +-
 Documentation/git-remote-helpers.txto |  2 +-
 Documentation/git-remote-testgit.txt  | 30 ---------------------------
 Documentation/gitremote-helpers.txt   |  4 +---
 Documentation/urls.txt                |  2 +-
 Makefile                              |  7 ++++---
 8 files changed, 10 insertions(+), 41 deletions(-)
 delete mode 100644 Documentation/git-remote-testgit.txt
base-commit: e902e9bcae2010bc42648c80ab6adc6c5a16a4a5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-162%2Fdscho%2Fcheck-docs-on-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-162/dscho/check-docs-on-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/162
Range-diff vs v1:
 -:  ---------- > 1:  0ad38649c0 docs: move gitremote-helpers into section 7
 -:  ---------- > 2:  810d2c5a94 docs: do not document the `git remote-testgit` command
 -:  ---------- > 3:  0dad6abd2f check-docs: really look at the documented commands again
 -:  ---------- > 4:  1097be7678 check-docs: do not expect guide pages to correspond to commands
 1:  f06126c3a1 ! 5:  b26aa40c61 check-docs: fix for setups where executables have an extension
     @@ -24,7 +24,7 @@
       	) | while read how cmd; \
       	do \
      -		case " $(ALL_COMMANDS) " in \
     -+		case "  $(patsubst %$X,%,$(ALL_COMMANDS)) " in \
     ++		case " $(patsubst %$X,%,$(ALL_COMMANDS)) " in \
       		*" $$cmd "*)	;; \
       		*) echo "removed but $$how: $$cmd" ;; \
       		esac; \
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Mar 25, 2019, 21:41 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 3/5] check-docs: really look at the documented commands again

From: Johannes Schindelin <johannes.schindelin@gmx.de>

As part of the `check-docs` target, we verify that commands that are documented are actually in the current list of commands to be built.

However, this logic broke in 5fafce0b78 (check-docs: get documented command list from Makefile, 2012-08-08), when we tried to make the logic safer by not looking at the files in the worktree, but at the list of files to be generated in `Documentation/Makefile`. While this was the right thing to do, it failed to accommodate for the fact that `make -C Documentation/ print-man1`, unlike `ls Documentation/*.txt`, would *not* print lines starting with the prefix `Documentation/`.

At long last, let's fix this.
Note: This went undetected due to a funny side effect of the
`ALL_PROGRAMS` variable starting with a space. That space, together with
the extra space we inserted before `$(ALL_PROGRAMS)` in the
	case " $(ALL_PROGRAMS)" in
	*" $$cmd ")
		[...]

construct, is responsible that this case arm is used when `cmd` is empty (which was clearly not intended to be the case).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Makefile +1 −1
diff --git a/Makefile b/Makefile
index 537493822b..76904f07fe 100644
--- a/Makefile
+++ b/Makefile
@@ -3099,7 +3099,7 @@ check-docs::
 		    -e 's/^/listed /' command-list.txt; \
 		$(MAKE) -C Documentation print-man1 | \
 		grep '\.txt$$' | \
-		sed -e 's|Documentation/|documented |' \
+		sed -e 's|^|documented |' \
 		    -e 's/\.txt//'; \
 	) | while read how cmd; \
 	do \

-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Mar 25, 2019, 21:41 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 5/5] check-docs: fix for setups where executables have an extension

From: Johannes Schindelin <johannes.schindelin@gmx.de>

On Windows, for example, executables (must) have the extension `.exe`. Our `check-docs` target was not prepared for that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to Makefile +2 −2
diff --git a/Makefile b/Makefile
index c7b884cd38..2cd650e7b9 100644
--- a/Makefile
+++ b/Makefile
@@ -3074,7 +3074,7 @@ ALL_COMMANDS += git-gui git-citool
 .PHONY: check-docs
 check-docs::
 	$(MAKE) -C Documentation lint-docs
-	@(for v in $(ALL_COMMANDS); \
+	@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \
 	do \
 		case "$$v" in \
 		git-merge-octopus | git-merge-ours | git-merge-recursive | \
@@ -3104,7 +3104,7 @@ check-docs::
 		    -e 's/\.txt//'; \
 	) | while read how cmd; \
 	do \
-		case " $(ALL_COMMANDS) " in \
+		case " $(patsubst %$X,%,$(ALL_COMMANDS)) " in \
 		*" $$cmd "*)	;; \
 		*) echo "removed but $$how: $$cmd" ;; \
 		esac; \

-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Mar 25, 2019, 21:41 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 2/5] docs: do not document the `git remote-testgit` command

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Since 7ded055401 (build: do not install git-remote-testgit, 2013-06-07), we do not install it. Therefore it makes no sense to document it, either.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-remote-testgit.txt | 30 ----------------------------
 Documentation/gitremote-helpers.txt  |  2 --
 2 files changed, 32 deletions(-)
 delete mode 100644 Documentation/git-remote-testgit.txt
Show changes to 2 files +0 −27

Documentation/git-remote-testgit.txt, Documentation/gitremote-helpers.txt

diff --git a/Documentation/git-remote-testgit.txt b/Documentation/git-remote-testgit.txt
deleted file mode 100644
index b45bfebba5..0000000000
--- a/Documentation/git-remote-testgit.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-git-remote-testgit(1)
-=====================
-
-NAME
-----
-git-remote-testgit - Example remote-helper
-
-
-SYNOPSIS
---------
-[verse]
-git clone testgit::<source-repo> [<destination>]
-
-DESCRIPTION
------------
-
-This command is a simple remote-helper, that is used both as a
-testcase for the remote-helper functionality, and as an example to
-show remote-helper authors one possible implementation.
-
-The best way to learn more is to read the comments and source code in
-'git-remote-testgit'.
-
-SEE ALSO
---------
-linkgit:gitremote-helpers[7]
-
-GIT
----
-Part of the linkgit:git[1] suite
diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index 34a3e60d08..2fc4007525 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -513,8 +513,6 @@ linkgit:git-remote-ext[1]
 
 linkgit:git-remote-fd[1]
 
-linkgit:git-remote-testgit[1]
-
 linkgit:git-fast-import[1]
 
 GIT

-- 
gitgitgadget
Ævar Arnfjörð Bjarmason· Mar 26, 2019, 14:39 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH v2 2/5] docs: do not document the `git remote-testgit` command

On Mon, Mar 25 2019, Johannes Schindelin via GitGitGadget wrote:
Show 60 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Since 7ded055401 (build: do not install git-remote-testgit, 2013-06-07),
> we do not install it. Therefore it makes no sense to document it,
> either.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/git-remote-testgit.txt | 30 ----------------------------
>  Documentation/gitremote-helpers.txt  |  2 --
>  2 files changed, 32 deletions(-)
>  delete mode 100644 Documentation/git-remote-testgit.txt
>
> diff --git a/Documentation/git-remote-testgit.txt b/Documentation/git-remote-testgit.txt
> deleted file mode 100644
> index b45bfebba5..0000000000
> --- a/Documentation/git-remote-testgit.txt
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -git-remote-testgit(1)
> -=====================
> -
> -NAME
> -----
> -git-remote-testgit - Example remote-helper
> -
> -
> -SYNOPSIS
> ---------
> -[verse]
> -git clone testgit::<source-repo> [<destination>]
> -
> -DESCRIPTION
> ------------
> -
> -This command is a simple remote-helper, that is used both as a
> -testcase for the remote-helper functionality, and as an example to
> -show remote-helper authors one possible implementation.
> -
> -The best way to learn more is to read the comments and source code in
> -'git-remote-testgit'.
> -
> -SEE ALSO
> ---------
> -linkgit:gitremote-helpers[7]
> -
> -GIT
> ----
> -Part of the linkgit:git[1] suite
> diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
> index 34a3e60d08..2fc4007525 100644
> --- a/Documentation/gitremote-helpers.txt
> +++ b/Documentation/gitremote-helpers.txt
> @@ -513,8 +513,6 @@ linkgit:git-remote-ext[1]
>
>  linkgit:git-remote-fd[1]
>
> -linkgit:git-remote-testgit[1]
> -
>  linkgit:git-fast-import[1]

I wonder if it should be moved into e.g. t/helper or somesuch at this point...

Johannes Schindelin· Mar 27, 2019, 22:23 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 2/5] docs: do not document the `git remote-testgit` command

Hi Ævar,
On Tue, 26 Mar 2019, Ævar Arnfjörð Bjarmason wrote:
Show 66 quoted lines
>
> On Mon, Mar 25 2019, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > Since 7ded055401 (build: do not install git-remote-testgit, 2013-06-07),
> > we do not install it. Therefore it makes no sense to document it,
> > either.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  Documentation/git-remote-testgit.txt | 30 ----------------------------
> >  Documentation/gitremote-helpers.txt  |  2 --
> >  2 files changed, 32 deletions(-)
> >  delete mode 100644 Documentation/git-remote-testgit.txt
> >
> > diff --git a/Documentation/git-remote-testgit.txt b/Documentation/git-remote-testgit.txt
> > deleted file mode 100644
> > index b45bfebba5..0000000000
> > --- a/Documentation/git-remote-testgit.txt
> > +++ /dev/null
> > @@ -1,30 +0,0 @@
> > -git-remote-testgit(1)
> > -=====================
> > -
> > -NAME
> > -----
> > -git-remote-testgit - Example remote-helper
> > -
> > -
> > -SYNOPSIS
> > ---------
> > -[verse]
> > -git clone testgit::<source-repo> [<destination>]
> > -
> > -DESCRIPTION
> > ------------
> > -
> > -This command is a simple remote-helper, that is used both as a
> > -testcase for the remote-helper functionality, and as an example to
> > -show remote-helper authors one possible implementation.
> > -
> > -The best way to learn more is to read the comments and source code in
> > -'git-remote-testgit'.
> > -
> > -SEE ALSO
> > ---------
> > -linkgit:gitremote-helpers[7]
> > -
> > -GIT
> > ----
> > -Part of the linkgit:git[1] suite
> > diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
> > index 34a3e60d08..2fc4007525 100644
> > --- a/Documentation/gitremote-helpers.txt
> > +++ b/Documentation/gitremote-helpers.txt
> > @@ -513,8 +513,6 @@ linkgit:git-remote-ext[1]
> >
> >  linkgit:git-remote-fd[1]
> >
> > -linkgit:git-remote-testgit[1]
> > -
> >  linkgit:git-fast-import[1]
>
> I wonder if it should be moved into e.g. t/helper or somesuch at this
> point...
Sure.

One thing to keep in mind is that we cannot change the actual name of the executable, as it needs to have the format `git-remote-*`, and cannot require a first argument `testgit`, to be able to perform its purpose.

We do have a precedent for that kind of scenario: `test-fake-ssh`. It is also not folded into `test-tool`, and it is copied while changing the name to `ssh`, as required to fulfill its mission.

I am not totally enthusiastic about this because this is what prevents Git's test suite from running in a RAM disk on Windows, IIRC: I have not been able to convince my RAM disk provider to allow executables stored on it to be executed.

Ciao, Dscho

Johannes Schindelin· Mar 29, 2019, 14:48 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2 2/5] docs: do not document the `git remote-testgit` command

Hi Ævar,
On Wed, 27 Mar 2019, Johannes Schindelin wrote:
Show 82 quoted lines
> On Tue, 26 Mar 2019, Ævar Arnfjörð Bjarmason wrote:
>
> > On Mon, Mar 25 2019, Johannes Schindelin via GitGitGadget wrote:
> >
> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > >
> > > Since 7ded055401 (build: do not install git-remote-testgit, 2013-06-07),
> > > we do not install it. Therefore it makes no sense to document it,
> > > either.
> > >
> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > ---
> > >  Documentation/git-remote-testgit.txt | 30 ----------------------------
> > >  Documentation/gitremote-helpers.txt  |  2 --
> > >  2 files changed, 32 deletions(-)
> > >  delete mode 100644 Documentation/git-remote-testgit.txt
> > >
> > > diff --git a/Documentation/git-remote-testgit.txt b/Documentation/git-remote-testgit.txt
> > > deleted file mode 100644
> > > index b45bfebba5..0000000000
> > > --- a/Documentation/git-remote-testgit.txt
> > > +++ /dev/null
> > > @@ -1,30 +0,0 @@
> > > -git-remote-testgit(1)
> > > -=====================
> > > -
> > > -NAME
> > > -----
> > > -git-remote-testgit - Example remote-helper
> > > -
> > > -
> > > -SYNOPSIS
> > > ---------
> > > -[verse]
> > > -git clone testgit::<source-repo> [<destination>]
> > > -
> > > -DESCRIPTION
> > > ------------
> > > -
> > > -This command is a simple remote-helper, that is used both as a
> > > -testcase for the remote-helper functionality, and as an example to
> > > -show remote-helper authors one possible implementation.
> > > -
> > > -The best way to learn more is to read the comments and source code in
> > > -'git-remote-testgit'.
> > > -
> > > -SEE ALSO
> > > ---------
> > > -linkgit:gitremote-helpers[7]
> > > -
> > > -GIT
> > > ----
> > > -Part of the linkgit:git[1] suite
> > > diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
> > > index 34a3e60d08..2fc4007525 100644
> > > --- a/Documentation/gitremote-helpers.txt
> > > +++ b/Documentation/gitremote-helpers.txt
> > > @@ -513,8 +513,6 @@ linkgit:git-remote-ext[1]
> > >
> > >  linkgit:git-remote-fd[1]
> > >
> > > -linkgit:git-remote-testgit[1]
> > > -
> > >  linkgit:git-fast-import[1]
> >
> > I wonder if it should be moved into e.g. t/helper or somesuch at this
> > point...
>
> Sure.
>
> One thing to keep in mind is that we cannot change the actual name of the
> executable, as it needs to have the format `git-remote-*`, and cannot
> require a first argument `testgit`, to be able to perform its purpose.
>
> We do have a precedent for that kind of scenario: `test-fake-ssh`. It is
> also not folded into `test-tool`, and it is copied while changing the name
> to `ssh`, as required to fulfill its mission.
>
> I am not totally enthusiastic about this because this is what prevents
> Git's test suite from running in a RAM disk on Windows, IIRC: I have not
> been able to convince my RAM disk provider to allow executables stored on
> it to be executed.
Oh you know what? `git-remote-testgit` is a *shell script*.
So that one was easy...

Now, the question is really, should I just add this patch *here*, or to the non-Windows-related docs fixes I have lined up?

Decisions, decisions.

Ciao, Dscho

Johannes Schindelin via GitGitGadget· Mar 25, 2019, 21:41 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 1/5] docs: move gitremote-helpers into section 7

From: Johannes Schindelin <johannes.schindelin@gmx.de>

It is currently in section 1, but that section is intended for "Executable programs or shell commands".

A more appropriate place is section 7: "Miscellaneous (including macro packages and conventions), e.g. man(7), groff(7)".

This issue should have been detected earlier by `make check-docs`, but was missed due to a bug in that Makefile target (that we are about to fix).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/Makefile                | 2 +-
 Documentation/git-remote-ext.txt      | 2 +-
 Documentation/git-remote-fd.txt       | 2 +-
 Documentation/git-remote-helpers.txto | 2 +-
 Documentation/git-remote-testgit.txt  | 2 +-
 Documentation/gitremote-helpers.txt   | 2 +-
 Documentation/urls.txt                | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)
Show changes to 7 files +7 −7

Documentation/Makefile, Documentation/git-remote-ext.txt, Documentation/git-remote-fd.txt, Documentation/git-remote-helpers.txto, Documentation/git-remote-testgit.txt, Documentation/gitremote-helpers.txt, Documentation/urls.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 26a2342bea..af0e2cf11a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -12,7 +12,6 @@ MAN1_TXT += $(filter-out \
 		$(wildcard git-*.txt))
 MAN1_TXT += git.txt
 MAN1_TXT += gitk.txt
-MAN1_TXT += gitremote-helpers.txt
 MAN1_TXT += gitweb.txt
 
 MAN5_TXT += gitattributes.txt
@@ -30,6 +29,7 @@ MAN7_TXT += gitdiffcore.txt
 MAN7_TXT += giteveryday.txt
 MAN7_TXT += gitglossary.txt
 MAN7_TXT += gitnamespaces.txt
+MAN7_TXT += gitremote-helpers.txt
 MAN7_TXT += gitrevisions.txt
 MAN7_TXT += gitsubmodules.txt
 MAN7_TXT += gittutorial-2.txt
diff --git a/Documentation/git-remote-ext.txt b/Documentation/git-remote-ext.txt
index 3fc5d94336..790e2ca3fe 100644
--- a/Documentation/git-remote-ext.txt
+++ b/Documentation/git-remote-ext.txt
@@ -118,7 +118,7 @@ begins with `ext::`.  Examples:
 
 SEE ALSO
 --------
-linkgit:gitremote-helpers[1]
+linkgit:gitremote-helpers[7]
 
 GIT
 ---
diff --git a/Documentation/git-remote-fd.txt b/Documentation/git-remote-fd.txt
index 80afca866c..0451ceb8a2 100644
--- a/Documentation/git-remote-fd.txt
+++ b/Documentation/git-remote-fd.txt
@@ -52,7 +52,7 @@ EXAMPLES
 
 SEE ALSO
 --------
-linkgit:gitremote-helpers[1]
+linkgit:gitremote-helpers[7]
 
 GIT
 ---
diff --git a/Documentation/git-remote-helpers.txto b/Documentation/git-remote-helpers.txto
index 49233f5d26..6f353ebfd3 100644
--- a/Documentation/git-remote-helpers.txto
+++ b/Documentation/git-remote-helpers.txto
@@ -1,7 +1,7 @@
 git-remote-helpers
 ==================
 
-This document has been moved to linkgit:gitremote-helpers[1].
+This document has been moved to linkgit:gitremote-helpers[7].
 
 Please let the owners of the referring site know so that they can update the
 link you clicked to get here.
diff --git a/Documentation/git-remote-testgit.txt b/Documentation/git-remote-testgit.txt
index f791d73c05..b45bfebba5 100644
--- a/Documentation/git-remote-testgit.txt
+++ b/Documentation/git-remote-testgit.txt
@@ -23,7 +23,7 @@ The best way to learn more is to read the comments and source code in
 
 SEE ALSO
 --------
-linkgit:gitremote-helpers[1]
+linkgit:gitremote-helpers[7]
 
 GIT
 ---
diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index 9d1459aac6..34a3e60d08 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -1,4 +1,4 @@
-gitremote-helpers(1)
+gitremote-helpers(7)
 ====================
 
 NAME
diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index b05da95788..bc354fe2dc 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -62,7 +62,7 @@ may be used:
 
 where <address> may be a path, a server and path, or an arbitrary
 URL-like string recognized by the specific remote helper being
-invoked. See linkgit:gitremote-helpers[1] for details.
+invoked. See linkgit:gitremote-helpers[7] for details.
 
 If there are a large number of similarly-named remote repositories and
 you want to use a different format for them (such that the URLs you

-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Mar 25, 2019, 21:41 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 4/5] check-docs: do not expect guide pages to correspond to commands

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When we want to see what commands are listed in `command-list.txt` but not installed, we currently include lines that refer to guides, e.g. `gitattributes` or `gitcli`.

Let's not include those lines, as they are not referring to commands.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)
Show changes to Makefile +1 −0
diff --git a/Makefile b/Makefile
index 76904f07fe..c7b884cd38 100644
--- a/Makefile
+++ b/Makefile
@@ -3095,6 +3095,7 @@ check-docs::
 	( \
 		sed -e '1,/^### command list/d' \
 		    -e '/^#/d' \
+		    -e '/guide$$/d' \
 		    -e 's/[ 	].*//' \
 		    -e 's/^/listed /' command-list.txt; \
 		$(MAKE) -C Documentation print-man1 | \

-- 
gitgitgadget
Johannes Schindelin· Mar 26, 2019, 14:38 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH v2 0/5] Fix make check-docs on Windows

Hi,
On Mon, 25 Mar 2019, Johannes Schindelin via GitGitGadget wrote:
Show 6 quoted lines
> Johannes Schindelin (5):
>   docs: move gitremote-helpers into section 7
>   docs: do not document the `git remote-testgit` command
>   check-docs: really look at the documented commands again
>   check-docs: do not expect guide pages to correspond to commands
>   check-docs: fix for setups where executables have an extension

FWIW I am also working on a follow-up patch series [*1*] that resolves all issues reported by `make check-docs` on Windows, but I do not want to make this here patch series even bigger (from an initial single, relatively small patch).

Ciao, Johannes

Foontote *1*: Tracked here: https://github.com/gitgitgadget/git/pull/168

← back to recent threads