git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Makefile: correct default docs build target
@ 2025-02-14 21:52 Adam Dinwoodie
  2025-02-14 22:42 ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Adam Dinwoodie @ 2025-02-14 21:52 UTC (permalink / raw)
  To: git
  Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano,
	Patrick Steinhardt

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(-)

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Makefile: correct default docs build target
  2025-02-14 21:52 [PATCH] Makefile: correct default docs build target Adam Dinwoodie
@ 2025-02-14 22:42 ` Junio C Hamano
  2025-02-14 22:50   ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2025-02-14 22:42 UTC (permalink / raw)
  To: Adam Dinwoodie
  Cc: git, Ævar Arnfjörð Bjarmason, Patrick Steinhardt

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).


>
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Makefile: correct default docs build target
  2025-02-14 22:42 ` Junio C Hamano
@ 2025-02-14 22:50   ` Junio C Hamano
  2025-02-15 21:19     ` [PATCH v2] Makefile: set default goals in makefiles Adam Dinwoodie
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2025-02-14 22:50 UTC (permalink / raw)
  To: Adam Dinwoodie
  Cc: git, Ævar Arnfjörð Bjarmason, Patrick Steinhardt

Junio C Hamano <gitster@pobox.com> writes:

> 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.



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)
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2] Makefile: set default goals in makefiles
  2025-02-14 22:50   ` Junio C Hamano
@ 2025-02-15 21:19     ` Adam Dinwoodie
  2025-02-17  6:37       ` Patrick Steinhardt
  0 siblings, 1 reply; 6+ messages in thread
From: Adam Dinwoodie @ 2025-02-15 21:19 UTC (permalink / raw)
  To: git
  Cc: Patrick Steinhardt, Ævar Arnfjörð Bjarmason,
	Junio C Hamano

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(-)

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Makefile: set default goals in makefiles
  2025-02-15 21:19     ` [PATCH v2] Makefile: set default goals in makefiles Adam Dinwoodie
@ 2025-02-17  6:37       ` Patrick Steinhardt
  2025-02-18 17:00         ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Steinhardt @ 2025-02-17  6:37 UTC (permalink / raw)
  To: Adam Dinwoodie
  Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano

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::".

Patrick

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] Makefile: set default goals in makefiles
  2025-02-17  6:37       ` Patrick Steinhardt
@ 2025-02-18 17:00         ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2025-02-18 17:00 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Adam Dinwoodie, git, Ævar Arnfjörð Bjarmason

Patrick Steinhardt <ps@pks.im> writes:

> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-02-18 17:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-14 21:52 [PATCH] Makefile: correct default docs build target Adam Dinwoodie
2025-02-14 22:42 ` Junio C Hamano
2025-02-14 22:50   ` Junio C Hamano
2025-02-15 21:19     ` [PATCH v2] Makefile: set default goals in makefiles Adam Dinwoodie
2025-02-17  6:37       ` Patrick Steinhardt
2025-02-18 17:00         ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).