Subject: Re: [PATCH v2 1/4] Makefile: extract script to lint missing/extraneous manpages

From: Junio C Hamano <[email-masked]>
Date: Fri, 07 Jun 2024 09:51:54 -0700

Patrick Steinhardt <[email-masked]> writes:

> The "check-docs" target of our top-level Makefile fulfills two different
> roles. For one it runs the "lint-docs" target of the "Documentation/"
> Makefile. And second it performs some checks of whether there are any
> manpages that are missing or extraneous via some inline scripts.

Yes.

> The second set of checks feels quite misplaced in the top-level Makefile
> as it would fit in much better with our "lint-docs" target. Back when
> the checks were introduced in 8c989ec528 (Makefile: $(MAKE) check-docs,
> 2006-04-13), that target did not yet exist though.

Correct.

> Furthermore, the script makes use of several Makefile variables which
> are defined in the top-level Makefile, which makes it hard to access
> their contents from elsewhere. There is a trick though that we already
> use in "check-builtins.sh" to gain access: we can create an ad-hoc
> Makefile that has an extra target to print those variables.

Yes.

> Pull out the script into a separate "lint-manpages.sh" script by using
> that trick. Wire up that script via the "lint-docs" target. For one,
> normal shell scripts are way easier to reason about than those which are
> embedded in a Makefile. Second, it allows one to easily execute the
> script standalone without any of the other checks.

Nicely done.  We might want to stop doing "make print-man1" in the
lint script, remove "print-man1" target from Documentation/Makefile,
and instead extract MAN1_TXT using the same "it is unlikely to have
an output line enclosing something in between XXX and YYY" trick,
but it may not be worth it.

> +check_missing_docs () {
> +...
> +		git-merge-octopus) continue;;
> +		git-merge-ours) continue;;
> +		git-merge-recursive) continue;;
> +		git-merge-resolve) continue;;
> +		git-merge-subtree) continue;;
> +		git-fsck-objects) continue;;
> +		git-init-db) continue;;
> +		git-remote-*) continue;;
> +		git-stage) continue;;
> +		git-legacy-*) continue;;
> +		git-?*--?* ) continue ;;

This looks easier to maintain than the original by listing one
pattern per line.  Again, nicely done.