threads / patch / 62955

patchMakefile: correct default docs build target

Subject: [PATCH] Makefile: correct default docs build target

## tl;dr

6 messages between Feb 14, 2025 and Feb 18, 2025. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Adam Dinwoodie· Feb 14, 2025, 21:52 UTC · lore

Put the "all" target definition near the top of Documentation/Makefile, so that attempts to run make in the documentation directory actually build the documentation.

This seems like the expected behaviour, and was the behaviour up until a38edab7c8 (Makefile: generate doc versions via GIT-VERSION-GEN, 2024-12-06). That commit added some config files as build targets, and put the configuration in a sensible place, but unfortunately that sensible place was above any other build target definitions, meaning the default goal changed to being those configuration files only.

Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
---

Sending with my apologies to anyone who receives this twice; I made an error with my sendmail configuration, meaning servers checking the DMARC records would have rejected the previous patch.

 Documentation/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to Documentation/Makefile +2 −2
diff --git a/Documentation/Makefile b/Documentation/Makefile
index aedfe99d1d..31f40b6f37 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -3,6 +3,8 @@ include ../shared.mak
 
 .PHONY: FORCE
 
+all: html man
+
 # Guard against environment variables
 MAN1_TXT =
 MAN5_TXT =
@@ -238,8 +240,6 @@ DEFAULT_EDITOR_SQ = $(subst ','\'',$(DEFAULT_EDITOR))
 ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'
 endif
 
-all: html man
-
 html: $(DOC_HTML)
 
 man: man1 man5 man7

-- 
2.47.0
Junio C Hamano· Feb 14, 2025, 22:42 UTC · re: Adam Dinwoodie · lore

Re: [PATCH] Makefile: correct default docs build target

Adam Dinwoodie <adam@dinwoodie.org> writes:
> Put the "all" target definition near the top of Documentation/Makefile,
> so that attempts to run make in the documentation directory actually
> build the documentation.

Good eyes. To make the intent even more clear, please adopt the trick (or "convention") used by t/Makefile and our main Makefile to have an empty "all::" at the very beginning of the file, instead of moving things around, to avoid this kind of mistake to ever enter the repository again.

Thanks.
[Footnote]
* If existing "all" targets are single-colon rules by mistake, they
  need to be corrected.  There is no reason why these phony targets
  should be anything but double-colon rules).
Show 40 quoted lines
>
> This seems like the expected behaviour, and was the behaviour up until
> a38edab7c8 (Makefile: generate doc versions via GIT-VERSION-GEN,
> 2024-12-06).  That commit added some config files as build targets, and
> put the configuration in a sensible place, but unfortunately that
> sensible place was above any other build target definitions, meaning the
> default goal changed to being those configuration files only.
>
> Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
> ---
>
> Sending with my apologies to anyone who receives this twice; I made an
> error with my sendmail configuration, meaning servers checking the DMARC
> records would have rejected the previous patch.
>
>  Documentation/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index aedfe99d1d..31f40b6f37 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -3,6 +3,8 @@ include ../shared.mak
>  
>  .PHONY: FORCE
>  
> +all: html man
> +
>  # Guard against environment variables
>  MAN1_TXT =
>  MAN5_TXT =
> @@ -238,8 +240,6 @@ DEFAULT_EDITOR_SQ = $(subst ','\'',$(DEFAULT_EDITOR))
>  ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'
>  endif
>  
> -all: html man
> -
>  html: $(DOC_HTML)
>  
>  man: man1 man5 man7
Junio C Hamano· Feb 14, 2025, 22:50 UTC · re: Junio C Hamano · lore

Re: [PATCH] Makefile: correct default docs build target

Junio C Hamano <gitster@pobox.com> writes:
Show 20 quoted lines
> Adam Dinwoodie <adam@dinwoodie.org> writes:
>
>> Put the "all" target definition near the top of Documentation/Makefile,
>> so that attempts to run make in the documentation directory actually
>> build the documentation.
>
> Good eyes.  To make the intent even more clear, please adopt the
> trick (or "convention") used by t/Makefile and our main Makefile to
> have an empty "all::" at the very beginning of the file, instead of
> moving things around, to avoid this kind of mistake to ever enter
> the repository again.
>
> Thanks.
>
>
> [Footnote]
>
> * If existing "all" targets are single-colon rules by mistake, they
>   need to be corrected.  There is no reason why these phony targets
>   should be anything but double-colon rules).

Yikes, it turns out this is needed, but because there is only one place right now, fixing it is easy. Something like this, perhaps.

Show changes to diff +4 −1
diff --git c/Documentation/Makefile w/Documentation/Makefile
index aedfe99d1d..ddf3aa8fac 100644
--- c/Documentation/Makefile
+++ w/Documentation/Makefile
@@ -1,3 +1,6 @@
+# The default target of this Makefile is...
+all::
+
 # Import tree-wide shared Makefile behavior and libraries
 include ../shared.mak
 
@@ -238,7 +241,7 @@ DEFAULT_EDITOR_SQ = $(subst ','\'',$(DEFAULT_EDITOR))
 ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'
 endif
 
-all: html man
+all:: html man
 
 html: $(DOC_HTML)
Adam Dinwoodie· Feb 15, 2025, 21:19 UTC · re: Junio C Hamano · lore

[PATCH v2] Makefile: set default goals in makefiles

Explicitly set the default goal at the very top of various makefiles. This is already present in some makefiles, but not all of them.

In particular, this corrects a regression introduced in a38edab7c8 (Makefile: generate doc versions via GIT-VERSION-GEN, 2024-12-06). That commit added some config files as build targets for the Documentation directory, and put the target configuration in a sensible place. Unfortunately, that sensible place was above any other build target definitions, meaning the default goal changed to being those configuration files only, rather than the HTML and man page documentation.

Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/Makefile                  | 5 ++++-
 contrib/credential/libsecret/Makefile   | 3 +++
 contrib/credential/osxkeychain/Makefile | 1 +
 contrib/credential/wincred/Makefile     | 3 ++-
 contrib/diff-highlight/Makefile         | 3 ++-
 contrib/diff-highlight/t/Makefile       | 5 ++++-
 contrib/mw-to-git/Makefile              | 5 ++++-
 contrib/mw-to-git/t/Makefile            | 3 ++-
 contrib/persistent-https/Makefile       | 5 ++++-
 contrib/subtree/t/Makefile              | 5 ++++-
 git-gui/Makefile                        | 1 +
 git-gui/po/glossary/Makefile            | 3 +++
 t/interop/Makefile                      | 5 ++++-
 t/perf/Makefile                         | 5 ++++-
 templates/Makefile                      | 5 ++++-
 15 files changed, 46 insertions(+), 11 deletions(-)
Show changes to 15 files +46 −11

Documentation/Makefile, contrib/credential/libsecret/Makefile, contrib/credential/osxkeychain/Makefile, contrib/credential/wincred/Makefile, contrib/diff-highlight/Makefile, contrib/diff-highlight/t/Makefile, contrib/mw-to-git/Makefile, contrib/mw-to-git/t/Makefile, contrib/persistent-https/Makefile, contrib/subtree/t/Makefile, git-gui/Makefile, git-gui/po/glossary/Makefile, t/interop/Makefile, t/perf/Makefile, templates/Makefile

diff --git a/Documentation/Makefile b/Documentation/Makefile
index aedfe99d1d..ddf3aa8fac 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -1,3 +1,6 @@
+# The default target of this Makefile is...
+all::
+
 # Import tree-wide shared Makefile behavior and libraries
 include ../shared.mak
 
@@ -238,7 +241,7 @@ DEFAULT_EDITOR_SQ = $(subst ','\'',$(DEFAULT_EDITOR))
 ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'
 endif
 
-all: html man
+all:: html man
 
 html: $(DOC_HTML)
 
diff --git a/contrib/credential/libsecret/Makefile b/contrib/credential/libsecret/Makefile
index 3e67552cc5..97ce9c92fb 100644
--- a/contrib/credential/libsecret/Makefile
+++ b/contrib/credential/libsecret/Makefile
@@ -1,3 +1,6 @@
+# The default target of this Makefile is...
+all::
+
 MAIN:=git-credential-libsecret
 all:: $(MAIN)
 
diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile
index 238f5f8c36..0948297e20 100644
--- a/contrib/credential/osxkeychain/Makefile
+++ b/contrib/credential/osxkeychain/Makefile
@@ -1,3 +1,4 @@
+# The default target of this Makefile is...
 all:: git-credential-osxkeychain
 
 CC = gcc
diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile
index 6e992c0866..5b795fc9fe 100644
--- a/contrib/credential/wincred/Makefile
+++ b/contrib/credential/wincred/Makefile
@@ -1,4 +1,5 @@
-all: git-credential-wincred.exe
+# The default target of this Makefile is...
+all:: git-credential-wincred.exe
 
 -include ../../../config.mak.autogen
 -include ../../../config.mak
diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
index f2be7cc924..33c2ccc9f7 100644
--- a/contrib/diff-highlight/Makefile
+++ b/contrib/diff-highlight/Makefile
@@ -1,4 +1,5 @@
-all: diff-highlight
+# The default target of this Makefile is...
+all:: diff-highlight
 
 PERL_PATH = /usr/bin/perl
 -include ../../config.mak
diff --git a/contrib/diff-highlight/t/Makefile b/contrib/diff-highlight/t/Makefile
index 5ff5275496..2a98541477 100644
--- a/contrib/diff-highlight/t/Makefile
+++ b/contrib/diff-highlight/t/Makefile
@@ -1,12 +1,15 @@
+# The default target of this Makefile is...
+all::
+
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
 # copied from ../../t/Makefile
 SHELL_PATH ?= $(SHELL)
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 
-all: test
+all:: test
 test: $(T)
 
 .PHONY: help clean all test $(T)
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index 4e603512a3..497ac434d6 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -12,6 +12,9 @@
 #
 #   make install
 
+# The default target of this Makefile is...
+all::
+
 GIT_MEDIAWIKI_PM=Git/Mediawiki.pm
 SCRIPT_PERL=git-remote-mediawiki.perl
 SCRIPT_PERL+=git-mw.perl
@@ -27,7 +30,7 @@ INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
 INSTLIBDIR_SQ = $(subst ','\'',$(INSTLIBDIR))
 
-all: build
+all:: build
 
 test: all
 	$(MAKE) -C t
diff --git a/contrib/mw-to-git/t/Makefile b/contrib/mw-to-git/t/Makefile
index f422203fa0..6c9f377caa 100644
--- a/contrib/mw-to-git/t/Makefile
+++ b/contrib/mw-to-git/t/Makefile
@@ -8,7 +8,8 @@
 #
 ## Test git-remote-mediawiki
 
-all: test
+# The default target of this Makefile is...
+all:: test
 
 -include ../../../config.mak.autogen
 -include ../../../config.mak
diff --git a/contrib/persistent-https/Makefile b/contrib/persistent-https/Makefile
index 52b84ba3d4..691737e76b 100644
--- a/contrib/persistent-https/Makefile
+++ b/contrib/persistent-https/Makefile
@@ -12,10 +12,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# The default target of this Makefile is...
+all::
+
 BUILD_LABEL=$(shell cut -d" " -f3 ../../GIT-VERSION-FILE)
 TAR_OUT=$(shell go env GOOS)_$(shell go env GOARCH).tar.gz
 
-all: git-remote-persistent-https git-remote-persistent-https--proxy \
+all:: git-remote-persistent-https git-remote-persistent-https--proxy \
 	git-remote-persistent-http
 
 git-remote-persistent-https--proxy: git-remote-persistent-https
diff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile
index 093399c788..2a85f5ee84 100644
--- a/contrib/subtree/t/Makefile
+++ b/contrib/subtree/t/Makefile
@@ -3,6 +3,9 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
+# The default target of this Makefile is...
+all::
+
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
@@ -31,7 +34,7 @@ TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))
 TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
 THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
 
-all: $(DEFAULT_TEST_TARGET)
+all:: $(DEFAULT_TEST_TARGET)
 
 test: pre-clean $(TEST_LINT)
 	$(MAKE) aggregate-results-and-cleanup
diff --git a/git-gui/Makefile b/git-gui/Makefile
index 667c39ed56..6c5a12bc32 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -1,3 +1,4 @@
+# The default target of this Makefile is...
 all::
 
 # Define V=1 to have a more verbose compile.
diff --git a/git-gui/po/glossary/Makefile b/git-gui/po/glossary/Makefile
index 749aa2e7ec..e656b0d2b0 100644
--- a/git-gui/po/glossary/Makefile
+++ b/git-gui/po/glossary/Makefile
@@ -1,3 +1,6 @@
+# The default target of this Makefile is...
+update-po::
+
 PO_TEMPLATE = git-gui-glossary.pot
 
 ALL_POFILES = $(wildcard *.po)
diff --git a/t/interop/Makefile b/t/interop/Makefile
index 6911c2915a..4ff4ed0616 100644
--- a/t/interop/Makefile
+++ b/t/interop/Makefile
@@ -1,14 +1,17 @@
+# The default target of this Makefile is...
+all::
+
 # Import tree-wide shared Makefile behavior and libraries
 include ../../shared.mak
 
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
 SHELL_PATH ?= $(SHELL)
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 T = $(sort $(wildcard i[0-9][0-9][0-9][0-9]-*.sh))
 
-all: $(T)
+all:: $(T)
 
 $(T):
 	@echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
diff --git a/t/perf/Makefile b/t/perf/Makefile
index e4808aebed..9b3090c4ed 100644
--- a/t/perf/Makefile
+++ b/t/perf/Makefile
@@ -1,10 +1,13 @@
+# The default target of this Makefile is...
+all::
+
 # Import tree-wide shared Makefile behavior and libraries
 include ../../shared.mak
 
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
-all: test-lint perf
+all:: test-lint perf
 
 perf: pre-clean
 	./run
diff --git a/templates/Makefile b/templates/Makefile
index bd1e9e30c1..722755338d 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -1,3 +1,6 @@
+# The default target of this Makefile is...
+all::
+
 # Import tree-wide shared Makefile behavior and libraries
 include ../shared.mak
 
@@ -23,7 +26,7 @@ PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
 template_instdir_SQ = $(subst ','\'',$(template_instdir))
 
-all: boilerplates.made custom
+all:: boilerplates.made custom
 
 # Put templates that can be copied straight from the source
 # in a file direc--tory--file in the source.  They will be

-- 
2.47.0
Patrick Steinhardt· Feb 17, 2025, 06:37 UTC · re: Adam Dinwoodie · lore

Re: [PATCH v2] Makefile: set default goals in makefiles

On Sat, Feb 15, 2025 at 09:19:03PM +0000, Adam Dinwoodie wrote:
Show 11 quoted lines
> Explicitly set the default goal at the very top of various makefiles.
> This is already present in some makefiles, but not all of them.
> 
> In particular, this corrects a regression introduced in a38edab7c8
> (Makefile: generate doc versions via GIT-VERSION-GEN, 2024-12-06).  That
> commit added some config files as build targets for the Documentation
> directory, and put the target configuration in a sensible place.
> Unfortunately, that sensible place was above any other build target
> definitions, meaning the default goal changed to being those
> configuration files only, rather than the HTML and man page
> documentation.

Thanks for the fix! The patch looks good to me, and I've double-checked that preexisting "all:" targets were all converted to "all::".

Patrick
Junio C Hamano· Feb 18, 2025, 17:00 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v2] Makefile: set default goals in makefiles

Patrick Steinhardt <ps@pks.im> writes:
Show 15 quoted lines
> On Sat, Feb 15, 2025 at 09:19:03PM +0000, Adam Dinwoodie wrote:
>> Explicitly set the default goal at the very top of various makefiles.
>> This is already present in some makefiles, but not all of them.
>> 
>> In particular, this corrects a regression introduced in a38edab7c8
>> (Makefile: generate doc versions via GIT-VERSION-GEN, 2024-12-06).  That
>> commit added some config files as build targets for the Documentation
>> directory, and put the target configuration in a sensible place.
>> Unfortunately, that sensible place was above any other build target
>> definitions, meaning the default goal changed to being those
>> configuration files only, rather than the HTML and man page
>> documentation.
>
> Thanks for the fix! The patch looks good to me, and I've double-checked
> that preexisting "all:" targets were all converted to "all::".
Thanks, both of you.  Will apply.

← back to recent threads