[4/7] docs: exclude documentation for commands that have been excluded

Commit Message

Kazuhiro Kato via GitGitGadget April 12, 2019, noon UTC

From: Johannes Schindelin <[e-mail]>

When building with certain build options, some commands are excluded
from the build. For example, `git-credential-cache` is skipped when
building with `NO_UNIX_SOCKETS`.

Let's not build or package documentation for those excluded commands.

This issue was pointed out rightfully when running `make check-docs` on
Windows, where we do not yet have Unix sockets, and therefore the
`credential-cache` command is excluded (yet its documentation was built
and shipped).

Note: building the documentation via `make -C Documentation` leaves the
build system with no way to determine which commands have been
excluded. If called thusly, we gracefully fail to exclude their
documentation. Only when building the documentation via the top-level
Makefile will it get excluded properly, or after building
`Documentation/GIT-EXCLUDED-PROGRAMS` manually.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
 Documentation/.gitignore |  1 +
 Documentation/Makefile   |  3 +++
 Makefile                 | 36 ++++++++++++++++++++----------------
 3 files changed, 24 insertions(+), 16 deletions(-)

Comments

Eric Sunshine April 12, 2019, 6:46 p.m. UTC | #1

On Fri, Apr 12, 2019 at 8:00 AM Johannes Schindelin via GitGitGadget
<[e-mail]> wrote:
> When building with certain build options, some commands are excluded
> from the build. For example, `git-credential-cache` is skipped when
> building with `NO_UNIX_SOCKETS`.
>
> Let's not build or package documentation for those excluded commands.
> [...]
> Signed-off-by: Johannes Schindelin <[e-mail]>
> ---
> diff --git a/Makefile b/Makefile
> @@ -2455,22 +2455,25 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
> +Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
> +       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@

Should this rule also have a dependency upon "config.mak.autogen"?
Perhaps like this:

Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)

Junio C Hamano April 15, 2019, 3:09 a.m. UTC | #2

Eric Sunshine <[e-mail]> writes:

>> diff --git a/Makefile b/Makefile
>> @@ -2455,22 +2455,25 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
>> +Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
>> +       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@
>
> Should this rule also have a dependency upon "config.mak.autogen"?

That is probably a good point.

> Perhaps like this:
>
> Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)

I'd rather not let changes to "config.mak-", which I keep in my
working tree (untracked and disabled copy of config.mak, that can be
readily activated by renaming), be part of dependency rules.

If we know 'autogen' is the only dependency that optionally can
exist, then depending explicitly on $(wildcard config.mak.autogen)
would be a better alternative.  To those who do not use the autoconf,
the macro will expand to an empty string, and to those who do have the
path on the filesystem, the macro parrots the constant string it was
given, which is exactly what we want.

Thanks.

Eric Sunshine April 15, 2019, 4:16 a.m. UTC | #3

On Sun, Apr 14, 2019 at 11:10 PM Junio C Hamano <[e-mail]> wrote:
> Eric Sunshine <[e-mail]> writes:
> >> +Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
> >> +       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@
> >
> > Should this rule also have a dependency upon "config.mak.autogen"?
>
> That is probably a good point.
>
> > Perhaps like this:
> >
> > Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)
>
> I'd rather not let changes to "config.mak-", which I keep in my
> working tree (untracked and disabled copy of config.mak, that can be
> readily activated by renaming), be part of dependency rules.
>
> If we know 'autogen' is the only dependency that optionally can
> exist, then depending explicitly on $(wildcard config.mak.autogen)
> would be a better alternative.

When composing that email, I originally wrote $(wildcard
config.mak.autogen) as the suggestion but changed it to the looser
$(wildcard config.mak*) when I realized that the developer's own
config.mak probably ought to be a dependency, as well. Taking your
objection into consideration, we could mention both explicitly:

    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
        $(wildcard config.mak) $(wildcard config.mak.autogen)

Eric Sunshine April 15, 2019, 4:18 a.m. UTC | #4

On Mon, Apr 15, 2019 at 12:16 AM Eric Sunshine <[e-mail]> wrote:
> When composing that email, I originally wrote $(wildcard
> config.mak.autogen) as the suggestion but changed it to the looser
> $(wildcard config.mak*) when I realized that the developer's own
> config.mak probably ought to be a dependency, as well. Taking your
> objection into consideration, we could mention both explicitly:
>
>     Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
>         $(wildcard config.mak) $(wildcard config.mak.autogen)

Bleh, I forgot config.mak.uname from Dscho's original patch.

    Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname \
        $(wildcard config.mak) $(wildcard config.mak.autogen)

Jeff King April 15, 2019, 2:50 p.m. UTC | #5

On Mon, Apr 15, 2019 at 12:16:51AM -0400, Eric Sunshine wrote:

> On Sun, Apr 14, 2019 at 11:10 PM Junio C Hamano <[e-mail]> wrote:
> > Eric Sunshine <[e-mail]> writes:
> > >> +Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
> > >> +       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@
> > >
> > > Should this rule also have a dependency upon "config.mak.autogen"?
> >
> > That is probably a good point.
> >
> > > Perhaps like this:
> > >
> > > Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)
> >
> > I'd rather not let changes to "config.mak-", which I keep in my
> > working tree (untracked and disabled copy of config.mak, that can be
> > readily activated by renaming), be part of dependency rules.
> >
> > If we know 'autogen' is the only dependency that optionally can
> > exist, then depending explicitly on $(wildcard config.mak.autogen)
> > would be a better alternative.
> 
> When composing that email, I originally wrote $(wildcard
> config.mak.autogen) as the suggestion but changed it to the looser
> $(wildcard config.mak*) when I realized that the developer's own
> config.mak probably ought to be a dependency, as well. Taking your
> objection into consideration, we could mention both explicitly:
> 
>     Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
>         $(wildcard config.mak) $(wildcard config.mak.autogen)

What about command-line options that influence the outcome? It sounds
like this is the same problem we have in lots of other places (like say,
compiler flags being updated), that we solve by generating the proposed
file output unconditionally and comparing it to what's on disk.  E.g.,
see the way GIT-CFLAGS or GIT-BUILD-OPTIONS is generated.

-Peff

Junio C Hamano April 16, 2019, 4:12 a.m. UTC | #6

Jeff King <[e-mail]> writes:

> What about command-line options that influence the outcome? It sounds
> like this is the same problem we have in lots of other places (like say,
> compiler flags being updated), that we solve by generating the proposed
> file output unconditionally and comparing it to what's on disk.  E.g.,
> see the way GIT-CFLAGS or GIT-BUILD-OPTIONS is generated.

;-).

Johannes Schindelin April 18, 2019, 1:06 p.m. UTC | #7

Hi Peff,

On Mon, 15 Apr 2019, Jeff King wrote:

> On Mon, Apr 15, 2019 at 12:16:51AM -0400, Eric Sunshine wrote:
>
> > On Sun, Apr 14, 2019 at 11:10 PM Junio C Hamano <[e-mail]> wrote:
> > > Eric Sunshine <[e-mail]> writes:
> > > >> +Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
> > > >> +       $(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@
> > > >
> > > > Should this rule also have a dependency upon "config.mak.autogen"?
> > >
> > > That is probably a good point.
> > >
> > > > Perhaps like this:
> > > >
> > > > Documentation/GIT-EXCLUDED-PROGRAMS: Makefile $(wildcard config.mak*)
> > >
> > > I'd rather not let changes to "config.mak-", which I keep in my
> > > working tree (untracked and disabled copy of config.mak, that can be
> > > readily activated by renaming), be part of dependency rules.
> > >
> > > If we know 'autogen' is the only dependency that optionally can
> > > exist, then depending explicitly on $(wildcard config.mak.autogen)
> > > would be a better alternative.
> >
> > When composing that email, I originally wrote $(wildcard
> > config.mak.autogen) as the suggestion but changed it to the looser
> > $(wildcard config.mak*) when I realized that the developer's own
> > config.mak probably ought to be a dependency, as well. Taking your
> > objection into consideration, we could mention both explicitly:
> >
> >     Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
> >         $(wildcard config.mak) $(wildcard config.mak.autogen)
>
> What about command-line options that influence the outcome? It sounds
> like this is the same problem we have in lots of other places (like say,
> compiler flags being updated), that we solve by generating the proposed
> file output unconditionally and comparing it to what's on disk.  E.g.,
> see the way GIT-CFLAGS or GIT-BUILD-OPTIONS is generated.

*Sigh*

I really did not want to do that because I thought it would be tedious and
more complicated and result in a longer patch.

Well, don't you know. The patch is actually *a lot* shorter now. So much
so that range-diff thinks it is a different commit ;-)

Thanks for the reality check/prod,
Dscho

Johannes Schindelin April 18, 2019, 1:08 p.m. UTC | #8

Hi Eric & Junio,

On Mon, 15 Apr 2019, Eric Sunshine wrote:

> On Mon, Apr 15, 2019 at 12:16 AM Eric Sunshine <[e-mail]> wrote:
> > When composing that email, I originally wrote $(wildcard
> > config.mak.autogen) as the suggestion but changed it to the looser
> > $(wildcard config.mak*) when I realized that the developer's own
> > config.mak probably ought to be a dependency, as well. Taking your
> > objection into consideration, we could mention both explicitly:
> >
> >     Documentation/GIT-EXCLUDED-PROGRAMS: Makefile \
> >         $(wildcard config.mak) $(wildcard config.mak.autogen)
>
> Bleh, I forgot config.mak.uname from Dscho's original patch.
>
>     Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname \
>         $(wildcard config.mak) $(wildcard config.mak.autogen)

Right.

I had that locally in the meantime, but based on Peff's suggestion went
for a GIT-CFLAGS like approach instead.

Ciao,
Dscho

Jeff King April 18, 2019, 4:01 p.m. UTC | #9

On Thu, Apr 18, 2019 at 03:06:56PM +0200, Johannes Schindelin wrote:

> > What about command-line options that influence the outcome? It sounds
> > like this is the same problem we have in lots of other places (like say,
> > compiler flags being updated), that we solve by generating the proposed
> > file output unconditionally and comparing it to what's on disk.  E.g.,
> > see the way GIT-CFLAGS or GIT-BUILD-OPTIONS is generated.
> 
> *Sigh*
> 
> I really did not want to do that because I thought it would be tedious and
> more complicated and result in a longer patch.
> 
> Well, don't you know. The patch is actually *a lot* shorter now. So much
> so that range-diff thinks it is a different commit ;-)

:) Great. I looked at the result in your v2 and it looks good to me.

-Peff

Patch

diff --git a/Documentation/.gitignore b/Documentation/.gitignore
index 3ef54e0adb..ea27148c59 100644
--- a/Documentation/.gitignore
+++ b/Documentation/.gitignore
@@ -13,3 +13,4 @@  mergetools-*.txt
 manpage-base-url.xsl
 SubmittingPatches.txt
 tmp-doc-diff/
+/GIT-EXCLUDED-PROGRAMS
diff --git a/Documentation/Makefile b/Documentation/Makefile
index af0e2cf11a..e22ea2f57c 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -7,7 +7,10 @@  ARTICLES =
 SP_ARTICLES =
 OBSOLETE_HTML =
 
+-include GIT-EXCLUDED-PROGRAMS
+
 MAN1_TXT += $(filter-out \
+		$(patsubst %,%.txt,$(EXCLUDED_PROGRAMS)) \
 		$(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \
 		$(wildcard git-*.txt))
 MAN1_TXT += git.txt
diff --git a/Makefile b/Makefile
index 5880d4d3a9..a5212c64bf 100644
--- a/Makefile
+++ b/Makefile
@@ -2455,22 +2455,25 @@  $(VCSSVN_LIB): $(VCSSVN_OBJS)
 
 export DEFAULT_EDITOR DEFAULT_PAGER
 
+Documentation/GIT-EXCLUDED-PROGRAMS: Makefile config.mak.uname
+	$(QUIET_GEN)echo "EXCLUDED_PROGRAMS := $(EXCLUDED_PROGRAMS)" >$@
+
 .PHONY: doc man man-perl html info pdf
-doc: man-perl
+doc: man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation all
 
-man: man-perl
+man: man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation man
 
 man-perl: perl/build/man/man3/Git.3pm
 
-html:
+html: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation html
 
-info:
+info: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation info
 
-pdf:
+pdf: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation pdf
 
 XGETTEXT_FLAGS = \
@@ -2907,33 +2910,33 @@  endif
 install-gitweb:
 	$(MAKE) -C gitweb install
 
-install-doc: install-man-perl
+install-doc: install-man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install
 
-install-man: install-man-perl
+install-man: install-man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install-man
 
-install-man-perl: man-perl
+install-man-perl: man-perl Documentation/GIT-EXCLUDED-PROGRAMS
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mandir_SQ)/man3'
 	(cd perl/build/man/man3 && $(TAR) cf - .) | \
 	(cd '$(DESTDIR_SQ)$(mandir_SQ)/man3' && umask 022 && $(TAR) xof -)
 
-install-html:
+install-html: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install-html
 
-install-info:
+install-info: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install-info
 
-install-pdf:
+install-pdf: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation install-pdf
 
-quick-install-doc:
+quick-install-doc: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation quick-install
 
-quick-install-man:
+quick-install-man: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation quick-install-man
 
-quick-install-html:
+quick-install-html: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation quick-install-html
 
 
@@ -2988,7 +2991,7 @@  artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \
 htmldocs = git-htmldocs-$(GIT_VERSION)
 manpages = git-manpages-$(GIT_VERSION)
 .PHONY: dist-doc distclean
-dist-doc:
+dist-doc: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(RM) -r .doc-tmp-dir
 	mkdir .doc-tmp-dir
 	$(MAKE) -C Documentation WEBDOC_DEST=../.doc-tmp-dir install-webdoc
@@ -3035,6 +3038,7 @@  clean: profile-clean coverage-clean cocciclean
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
 	$(MAKE) -C Documentation/ clean
+	$(RM) Documentation/GIT-EXCLUDED-PROGRAMS
 ifndef NO_PERL
 	$(MAKE) -C gitweb clean
 	$(RM) -r perl/build/
@@ -3062,7 +3066,7 @@  ALL_COMMANDS += gitweb
 ALL_COMMANDS += git-gui git-citool
 
 .PHONY: check-docs
-check-docs::
+check-docs:: Documentation/GIT-EXCLUDED-PROGRAMS
 	$(MAKE) -C Documentation lint-docs
 	@(for v in $(patsubst %$X,%,$(ALL_COMMANDS)); \
 	do \