Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Ævar

On 25/02/2022 09:04, Ævar Arnfjörð Bjarmason wrote:
> We have various behavior that's shared across our Makefiles, or that
> really should be (e.g. via defined templates). Let's create a
> top-level "shared.mak" to house those sorts of things, and start by
> adding the ".DELETE_ON_ERROR" flag to it.
> 
> See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
> flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
> .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
> ".DELETE_ON_ERROR" flag.
> 
> This does have the potential downside that if e.g. templates/Makefile
> would like to include this "shared.mak" in the future the semantics of
> such a Makefile will change, but as noted in the above commits (and
> GNU make's own documentation) any such change would be for the better,
> so it's safe to do this.

I was confused about the mention of templates/Makefile in this 
paragraph, it seems to be saying that the behavior would change in the 
future if we included shared.mak in templates/Makefile but this patch 
does exactly that.

Also does this patch mean we're now using .DELETE_ON_ERROR in places 
where we were not previously using it?

Best Wishes

Phillip

> This also doesn't introduce a bug by e.g. having this
> ".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
> have no such scoping semantics.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   Documentation/Makefile    |  6 +++---
>   Makefile                  | 13 +++----------
>   contrib/scalar/Makefile   |  3 +++
>   contrib/scalar/t/Makefile |  3 +++
>   shared.mak                |  9 +++++++++
>   t/Makefile                |  3 +++
>   t/interop/Makefile        |  3 +++
>   t/perf/Makefile           |  3 +++
>   templates/Makefile        |  3 +++
>   9 files changed, 33 insertions(+), 13 deletions(-)
>   create mode 100644 shared.mak
> 
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index ed656db2ae9..ba27456c86a 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../shared.mak
> +
>   # Guard against environment variables
>   MAN1_TXT =
>   MAN5_TXT =
> @@ -524,7 +527,4 @@ doc-l10n install-l10n::
>   	$(MAKE) -C po $@
>   endif
>   
> -# Delete the target file on error
> -.DELETE_ON_ERROR:
> -
>   .PHONY: FORCE
> diff --git a/Makefile b/Makefile
> index 6f0b4b775fe..d378ec22545 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include shared.mak
> +
>   # The default target of this Makefile is...
>   all::
>   
> @@ -2194,16 +2197,6 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>   strip: $(PROGRAMS) git$X
>   	$(STRIP) $(STRIP_OPTS) $^
>   
> -### Flags affecting all rules
> -
> -# A GNU make extension since gmake 3.72 (released in late 1994) to
> -# remove the target of rules if commands in those rules fail. The
> -# default is to only do that if make itself receives a signal. Affects
> -# all targets, see:
> -#
> -#    info make --index-search=.DELETE_ON_ERROR
> -.DELETE_ON_ERROR:
> -
>   ### Target-specific flags and dependencies
>   
>   # The generic compilation pattern rule and automatically
> diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
> index 5b12a437426..6fb5cc8b701 100644
> --- a/contrib/scalar/Makefile
> +++ b/contrib/scalar/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../../shared.mak
> +
>   QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
>   QUIET_SUBDIR1  =
>   
> diff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile
> index 6170672bb37..01e82e56d15 100644
> --- a/contrib/scalar/t/Makefile
> +++ b/contrib/scalar/t/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../../../shared.mak
> +
>   # Run scalar tests
>   #
>   # Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin
> diff --git a/shared.mak b/shared.mak
> new file mode 100644
> index 00000000000..0170bb397ae
> --- /dev/null
> +++ b/shared.mak
> @@ -0,0 +1,9 @@
> +### Flags affecting all rules
> +
> +# A GNU make extension since gmake 3.72 (released in late 1994) to
> +# remove the target of rules if commands in those rules fail. The
> +# default is to only do that if make itself receives a signal. Affects
> +# all targets, see:
> +#
> +#    info make --index-search=.DELETE_ON_ERROR
> +.DELETE_ON_ERROR:
> diff --git a/t/Makefile b/t/Makefile
> index 46cd5fc5273..056ce55dcc9 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../shared.mak
> +
>   # Run tests
>   #
>   # Copyright (c) 2005 Junio C Hamano
> diff --git a/t/interop/Makefile b/t/interop/Makefile
> index 31a4bbc716a..6911c2915a7 100644
> --- a/t/interop/Makefile
> +++ b/t/interop/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../../shared.mak
> +
>   -include ../../config.mak
>   export GIT_TEST_OPTIONS
>   
> diff --git a/t/perf/Makefile b/t/perf/Makefile
> index 2465770a782..e4808aebed0 100644
> --- a/t/perf/Makefile
> +++ b/t/perf/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../../shared.mak
> +
>   -include ../../config.mak
>   export GIT_TEST_OPTIONS
>   
> diff --git a/templates/Makefile b/templates/Makefile
> index d22a71a3999..636cee52f51 100644
> --- a/templates/Makefile
> +++ b/templates/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../shared.mak
> +
>   # make and install sample templates
>   
>   ifndef V