{"thread":{"id":"62955","subject":"[PATCH] Makefile: correct default docs build target","startedAt":"2025-02-14T21:58:15Z","lastAt":"2025-02-18T17:00:15Z","messageCount":6,"participants":["Adam Dinwoodie","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"512440","messageId":"20250214215717.2854453-1-adam@dinwoodie.org","threadId":"62955","inReplyTo":null,"subject":"[PATCH] Makefile: correct default docs build target","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2025-02-14T21:52:55Z","receivedAt":"2025-02-14T21:58:15Z","isPatch":true,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"Put the \"all\" target definition near the top of Documentation/Makefile,\nso that attempts to run make in the documentation directory actually\nbuild the documentation.\n\nThis seems like the expected behaviour, and was the behaviour up until\na38edab7c8 (Makefile: generate doc versions via GIT-VERSION-GEN,\n2024-12-06).  That commit added some config files as build targets, and\nput the configuration in a sensible place, but unfortunately that\nsensible place was above any other build target definitions, meaning the\ndefault goal changed to being those configuration files only.\n\nSigned-off-by: Adam Dinwoodie <adam@dinwoodie.org>\n---\n\nSending with my apologies to anyone who receives this twice; I made an\nerror with my sendmail configuration, meaning servers checking the DMARC\nrecords would have rejected the previous patch.\n\n Documentation/Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex aedfe99d1d..31f40b6f37 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -3,6 +3,8 @@ include ../shared.mak\n \n .PHONY: FORCE\n \n+all: html man\n+\n # Guard against environment variables\n MAN1_TXT =\n MAN5_TXT =\n@@ -238,8 +240,6 @@ DEFAULT_EDITOR_SQ = $(subst ','\\'',$(DEFAULT_EDITOR))\n ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'\n endif\n \n-all: html man\n-\n html: $(DOC_HTML)\n \n man: man1 man5 man7\n-- \n2.47.0\n\n"},{"id":"512445","messageId":"xmqq34gg172x.fsf@gitster.g","threadId":"62955","inReplyTo":"20250214215717.2854453-1-adam@dinwoodie.org","subject":"Re: [PATCH] Makefile: correct default docs build target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-14T22:42:46Z","receivedAt":"2025-02-14T22:42:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Dinwoodie <adam@dinwoodie.org> writes:\n\n> Put the \"all\" target definition near the top of Documentation/Makefile,\n> so that attempts to run make in the documentation directory actually\n> build the documentation.\n\nGood eyes.  To make the intent even more clear, please adopt the\ntrick (or \"convention\") used by t/Makefile and our main Makefile to\nhave an empty \"all::\" at the very beginning of the file, instead of\nmoving things around, to avoid this kind of mistake to ever enter\nthe repository again.\n\nThanks.\n\n\n[Footnote]\n\n* If existing \"all\" targets are single-colon rules by mistake, they\n  need to be corrected.  There is no reason why these phony targets\n  should be anything but double-colon rules).\n\n\n>\n> This seems like the expected behaviour, and was the behaviour up until\n> a38edab7c8 (Makefile: generate doc versions via GIT-VERSION-GEN,\n> 2024-12-06).  That commit added some config files as build targets, and\n> put the configuration in a sensible place, but unfortunately that\n> sensible place was above any other build target definitions, meaning the\n> default goal changed to being those configuration files only.\n>\n> Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>\n> ---\n>\n> Sending with my apologies to anyone who receives this twice; I made an\n> error with my sendmail configuration, meaning servers checking the DMARC\n> records would have rejected the previous patch.\n>\n>  Documentation/Makefile | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index aedfe99d1d..31f40b6f37 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -3,6 +3,8 @@ include ../shared.mak\n>  \n>  .PHONY: FORCE\n>  \n> +all: html man\n> +\n>  # Guard against environment variables\n>  MAN1_TXT =\n>  MAN5_TXT =\n> @@ -238,8 +240,6 @@ DEFAULT_EDITOR_SQ = $(subst ','\\'',$(DEFAULT_EDITOR))\n>  ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'\n>  endif\n>  \n> -all: html man\n> -\n>  html: $(DOC_HTML)\n>  \n>  man: man1 man5 man7\n"},{"id":"512446","messageId":"xmqqy0y8ywc7.fsf@gitster.g","threadId":"62955","inReplyTo":"xmqq34gg172x.fsf@gitster.g","subject":"Re: [PATCH] Makefile: correct default docs build target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-14T22:50:48Z","receivedAt":"2025-02-14T22:50:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Adam Dinwoodie <adam@dinwoodie.org> writes:\n>\n>> Put the \"all\" target definition near the top of Documentation/Makefile,\n>> so that attempts to run make in the documentation directory actually\n>> build the documentation.\n>\n> Good eyes.  To make the intent even more clear, please adopt the\n> trick (or \"convention\") used by t/Makefile and our main Makefile to\n> have an empty \"all::\" at the very beginning of the file, instead of\n> moving things around, to avoid this kind of mistake to ever enter\n> the repository again.\n>\n> Thanks.\n>\n>\n> [Footnote]\n>\n> * If existing \"all\" targets are single-colon rules by mistake, they\n>   need to be corrected.  There is no reason why these phony targets\n>   should be anything but double-colon rules).\n\nYikes, it turns out this is needed, but because there is only one\nplace right now, fixing it is easy.  Something like this, perhaps.\n\n\n\ndiff --git c/Documentation/Makefile w/Documentation/Makefile\nindex aedfe99d1d..ddf3aa8fac 100644\n--- c/Documentation/Makefile\n+++ w/Documentation/Makefile\n@@ -1,3 +1,6 @@\n+# The default target of this Makefile is...\n+all::\n+\n # Import tree-wide shared Makefile behavior and libraries\n include ../shared.mak\n \n@@ -238,7 +241,7 @@ DEFAULT_EDITOR_SQ = $(subst ','\\'',$(DEFAULT_EDITOR))\n ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'\n endif\n \n-all: html man\n+all:: html man\n \n html: $(DOC_HTML)\n \n"},{"id":"512471","messageId":"20250215211904.41883-1-adam@dinwoodie.org","threadId":"62955","inReplyTo":"xmqqy0y8ywc7.fsf@gitster.g","subject":"[PATCH v2] Makefile: set default goals in makefiles","fromName":"Adam Dinwoodie","fromEmail":"adam@dinwoodie.org","sentAt":"2025-02-15T21:19:03Z","receivedAt":"2025-02-15T21:19:26Z","isPatch":true,"sender":{"key":"adam@dinwoodie.org","avatar":"https://avatars.githubusercontent.com/u/1397507?v=4"},"body":"Explicitly set the default goal at the very top of various makefiles.\nThis is already present in some makefiles, but not all of them.\n\nIn particular, this corrects a regression introduced in a38edab7c8\n(Makefile: generate doc versions via GIT-VERSION-GEN, 2024-12-06).  That\ncommit added some config files as build targets for the Documentation\ndirectory, and put the target configuration in a sensible place.\nUnfortunately, that sensible place was above any other build target\ndefinitions, meaning the default goal changed to being those\nconfiguration files only, rather than the HTML and man page\ndocumentation.\n\nSigned-off-by: Adam Dinwoodie <adam@dinwoodie.org>\nHelped-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/Makefile                  | 5 ++++-\n contrib/credential/libsecret/Makefile   | 3 +++\n contrib/credential/osxkeychain/Makefile | 1 +\n contrib/credential/wincred/Makefile     | 3 ++-\n contrib/diff-highlight/Makefile         | 3 ++-\n contrib/diff-highlight/t/Makefile       | 5 ++++-\n contrib/mw-to-git/Makefile              | 5 ++++-\n contrib/mw-to-git/t/Makefile            | 3 ++-\n contrib/persistent-https/Makefile       | 5 ++++-\n contrib/subtree/t/Makefile              | 5 ++++-\n git-gui/Makefile                        | 1 +\n git-gui/po/glossary/Makefile            | 3 +++\n t/interop/Makefile                      | 5 ++++-\n t/perf/Makefile                         | 5 ++++-\n templates/Makefile                      | 5 ++++-\n 15 files changed, 46 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex aedfe99d1d..ddf3aa8fac 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -1,3 +1,6 @@\n+# The default target of this Makefile is...\n+all::\n+\n # Import tree-wide shared Makefile behavior and libraries\n include ../shared.mak\n \n@@ -238,7 +241,7 @@ DEFAULT_EDITOR_SQ = $(subst ','\\'',$(DEFAULT_EDITOR))\n ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'\n endif\n \n-all: html man\n+all:: html man\n \n html: $(DOC_HTML)\n \ndiff --git a/contrib/credential/libsecret/Makefile b/contrib/credential/libsecret/Makefile\nindex 3e67552cc5..97ce9c92fb 100644\n--- a/contrib/credential/libsecret/Makefile\n+++ b/contrib/credential/libsecret/Makefile\n@@ -1,3 +1,6 @@\n+# The default target of this Makefile is...\n+all::\n+\n MAIN:=git-credential-libsecret\n all:: $(MAIN)\n \ndiff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile\nindex 238f5f8c36..0948297e20 100644\n--- a/contrib/credential/osxkeychain/Makefile\n+++ b/contrib/credential/osxkeychain/Makefile\n@@ -1,3 +1,4 @@\n+# The default target of this Makefile is...\n all:: git-credential-osxkeychain\n \n CC = gcc\ndiff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile\nindex 6e992c0866..5b795fc9fe 100644\n--- a/contrib/credential/wincred/Makefile\n+++ b/contrib/credential/wincred/Makefile\n@@ -1,4 +1,5 @@\n-all: git-credential-wincred.exe\n+# The default target of this Makefile is...\n+all:: git-credential-wincred.exe\n \n -include ../../../config.mak.autogen\n -include ../../../config.mak\ndiff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile\nindex f2be7cc924..33c2ccc9f7 100644\n--- a/contrib/diff-highlight/Makefile\n+++ b/contrib/diff-highlight/Makefile\n@@ -1,4 +1,5 @@\n-all: diff-highlight\n+# The default target of this Makefile is...\n+all:: diff-highlight\n \n PERL_PATH = /usr/bin/perl\n -include ../../config.mak\ndiff --git a/contrib/diff-highlight/t/Makefile b/contrib/diff-highlight/t/Makefile\nindex 5ff5275496..2a98541477 100644\n--- a/contrib/diff-highlight/t/Makefile\n+++ b/contrib/diff-highlight/t/Makefile\n@@ -1,12 +1,15 @@\n+# The default target of this Makefile is...\n+all::\n+\n -include ../../../config.mak.autogen\n -include ../../../config.mak\n \n # copied from ../../t/Makefile\n SHELL_PATH ?= $(SHELL)\n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)\n \n-all: test\n+all:: test\n test: $(T)\n \n .PHONY: help clean all test $(T)\ndiff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile\nindex 4e603512a3..497ac434d6 100644\n--- a/contrib/mw-to-git/Makefile\n+++ b/contrib/mw-to-git/Makefile\n@@ -12,6 +12,9 @@\n #\n #   make install\n \n+# The default target of this Makefile is...\n+all::\n+\n GIT_MEDIAWIKI_PM=Git/Mediawiki.pm\n SCRIPT_PERL=git-remote-mediawiki.perl\n SCRIPT_PERL+=git-mw.perl\n@@ -27,7 +30,7 @@ INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \\\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n INSTLIBDIR_SQ = $(subst ','\\'',$(INSTLIBDIR))\n \n-all: build\n+all:: build\n \n test: all\n \t$(MAKE) -C t\ndiff --git a/contrib/mw-to-git/t/Makefile b/contrib/mw-to-git/t/Makefile\nindex f422203fa0..6c9f377caa 100644\n--- a/contrib/mw-to-git/t/Makefile\n+++ b/contrib/mw-to-git/t/Makefile\n@@ -8,7 +8,8 @@\n #\n ## Test git-remote-mediawiki\n \n-all: test\n+# The default target of this Makefile is...\n+all:: test\n \n -include ../../../config.mak.autogen\n -include ../../../config.mak\ndiff --git a/contrib/persistent-https/Makefile b/contrib/persistent-https/Makefile\nindex 52b84ba3d4..691737e76b 100644\n--- a/contrib/persistent-https/Makefile\n+++ b/contrib/persistent-https/Makefile\n@@ -12,10 +12,13 @@\n # See the License for the specific language governing permissions and\n # limitations under the License.\n \n+# The default target of this Makefile is...\n+all::\n+\n BUILD_LABEL=$(shell cut -d\" \" -f3 ../../GIT-VERSION-FILE)\n TAR_OUT=$(shell go env GOOS)_$(shell go env GOARCH).tar.gz\n \n-all: git-remote-persistent-https git-remote-persistent-https--proxy \\\n+all:: git-remote-persistent-https git-remote-persistent-https--proxy \\\n \tgit-remote-persistent-http\n \n git-remote-persistent-https--proxy: git-remote-persistent-https\ndiff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile\nindex 093399c788..2a85f5ee84 100644\n--- a/contrib/subtree/t/Makefile\n+++ b/contrib/subtree/t/Makefile\n@@ -3,6 +3,9 @@\n # Copyright (c) 2005 Junio C Hamano\n #\n \n+# The default target of this Makefile is...\n+all::\n+\n -include ../../../config.mak.autogen\n -include ../../../config.mak\n \n@@ -31,7 +34,7 @@ TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))\n TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))\n THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))\n \n-all: $(DEFAULT_TEST_TARGET)\n+all:: $(DEFAULT_TEST_TARGET)\n \n test: pre-clean $(TEST_LINT)\n \t$(MAKE) aggregate-results-and-cleanup\ndiff --git a/git-gui/Makefile b/git-gui/Makefile\nindex 667c39ed56..6c5a12bc32 100644\n--- a/git-gui/Makefile\n+++ b/git-gui/Makefile\n@@ -1,3 +1,4 @@\n+# The default target of this Makefile is...\n all::\n \n # Define V=1 to have a more verbose compile.\ndiff --git a/git-gui/po/glossary/Makefile b/git-gui/po/glossary/Makefile\nindex 749aa2e7ec..e656b0d2b0 100644\n--- a/git-gui/po/glossary/Makefile\n+++ b/git-gui/po/glossary/Makefile\n@@ -1,3 +1,6 @@\n+# The default target of this Makefile is...\n+update-po::\n+\n PO_TEMPLATE = git-gui-glossary.pot\n \n ALL_POFILES = $(wildcard *.po)\ndiff --git a/t/interop/Makefile b/t/interop/Makefile\nindex 6911c2915a..4ff4ed0616 100644\n--- a/t/interop/Makefile\n+++ b/t/interop/Makefile\n@@ -1,14 +1,17 @@\n+# The default target of this Makefile is...\n+all::\n+\n # Import tree-wide shared Makefile behavior and libraries\n include ../../shared.mak\n \n -include ../../config.mak\n export GIT_TEST_OPTIONS\n \n SHELL_PATH ?= $(SHELL)\n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n T = $(sort $(wildcard i[0-9][0-9][0-9][0-9]-*.sh))\n \n-all: $(T)\n+all:: $(T)\n \n $(T):\n \t@echo \"*** $@ ***\"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\ndiff --git a/t/perf/Makefile b/t/perf/Makefile\nindex e4808aebed..9b3090c4ed 100644\n--- a/t/perf/Makefile\n+++ b/t/perf/Makefile\n@@ -1,10 +1,13 @@\n+# The default target of this Makefile is...\n+all::\n+\n # Import tree-wide shared Makefile behavior and libraries\n include ../../shared.mak\n \n -include ../../config.mak\n export GIT_TEST_OPTIONS\n \n-all: test-lint perf\n+all:: test-lint perf\n \n perf: pre-clean\n \t./run\ndiff --git a/templates/Makefile b/templates/Makefile\nindex bd1e9e30c1..722755338d 100644\n--- a/templates/Makefile\n+++ b/templates/Makefile\n@@ -1,3 +1,6 @@\n+# The default target of this Makefile is...\n+all::\n+\n # Import tree-wide shared Makefile behavior and libraries\n include ../shared.mak\n \n@@ -23,7 +26,7 @@ PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n template_instdir_SQ = $(subst ','\\'',$(template_instdir))\n \n-all: boilerplates.made custom\n+all:: boilerplates.made custom\n \n # Put templates that can be copied straight from the source\n # in a file direc--tory--file in the source.  They will be\n-- \n2.47.0\n\n"},{"id":"512499","messageId":"Z7LZJ0tRz3iLPgmx@pks.im","threadId":"62955","inReplyTo":"20250215211904.41883-1-adam@dinwoodie.org","subject":"Re: [PATCH v2] Makefile: set default goals in makefiles","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-17T06:37:27Z","receivedAt":"2025-02-17T06:37:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Feb 15, 2025 at 09:19:03PM +0000, Adam Dinwoodie wrote:\n> Explicitly set the default goal at the very top of various makefiles.\n> This is already present in some makefiles, but not all of them.\n> \n> In particular, this corrects a regression introduced in a38edab7c8\n> (Makefile: generate doc versions via GIT-VERSION-GEN, 2024-12-06).  That\n> commit added some config files as build targets for the Documentation\n> directory, and put the target configuration in a sensible place.\n> Unfortunately, that sensible place was above any other build target\n> definitions, meaning the default goal changed to being those\n> configuration files only, rather than the HTML and man page\n> documentation.\n\nThanks for the fix! The patch looks good to me, and I've double-checked\nthat preexisting \"all:\" targets were all converted to \"all::\".\n\nPatrick\n"},{"id":"512620","messageId":"xmqqeczvyyqr.fsf@gitster.g","threadId":"62955","inReplyTo":"Z7LZJ0tRz3iLPgmx@pks.im","subject":"Re: [PATCH v2] Makefile: set default goals in makefiles","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-18T17:00:12Z","receivedAt":"2025-02-18T17:00:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Sat, Feb 15, 2025 at 09:19:03PM +0000, Adam Dinwoodie wrote:\n>> Explicitly set the default goal at the very top of various makefiles.\n>> This is already present in some makefiles, but not all of them.\n>> \n>> In particular, this corrects a regression introduced in a38edab7c8\n>> (Makefile: generate doc versions via GIT-VERSION-GEN, 2024-12-06).  That\n>> commit added some config files as build targets for the Documentation\n>> directory, and put the target configuration in a sensible place.\n>> Unfortunately, that sensible place was above any other build target\n>> definitions, meaning the default goal changed to being those\n>> configuration files only, rather than the HTML and man page\n>> documentation.\n>\n> Thanks for the fix! The patch looks good to me, and I've double-checked\n> that preexisting \"all:\" targets were all converted to \"all::\".\n\nThanks, both of you.  Will apply.\n"}]}