Re: [RFC PATCH] Makefile: add deprecation message for strip target

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Nov 23 2021, Bagas Sanjaya wrote:

> Now that $INSTALL_STRIP variable can be defined since 3231f41009 (make:
> add INSTALL_STRIP option variable, 2021-09-05), deprecate 'strip' target
> to encourage users to move to $INSTALL_STRIP. The target will eventually
> be removed in Git 2.35+1.
>
> Only deprecation message is printed.
>
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 12be39ac49..ee83860f7d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2159,6 +2159,8 @@ please_set_SHELL_PATH_to_a_more_modern_shell:
>  shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>  
>  strip: $(PROGRAMS) git$X
> +	@echo "The 'strip' target is deprecated, define INSTALL_STRIP if you want to"
> +	@echo "install Git with stripped binaries."
>  	$(STRIP) $(STRIP_OPTS) $^
>  
>  ### Flags affecting all rules
>
> base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc

This is a better way to do this:

diff --git a/Makefile b/Makefile
index 12be39ac497..fd4736dff2f 100644
--- a/Makefile
+++ b/Makefile
@@ -2159,6 +2159,8 @@ please_set_SHELL_PATH_to_a_more_modern_shell:
 shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 
 strip: $(PROGRAMS) git$X
+       $(warning The 'strip' target is deprecated, define INSTALL_STRIP if you want to \
+install stripped binaries)
        $(STRIP) $(STRIP_OPTS) $^
 
 ### Flags affecting all rules

I.e. GNU make has a built-in way to do this which emits the line number.

The message also needs to be reworded, now it's telling me "do xyz to
..." do what I just did successfully? It should say something like

    you just did X, but doing that via Y will soon be deprecated, do Z instead
    to accomplish X"

See also:

    git log -p -G'\$\((warning|error)' -- Makefile

For some recent-ish ways of doing phase-in deprecation.

Personally I think just starting with $(error) would be fine here. If
someone needs to adjust their build system anyway they can just adjust
it the first time around, it's not like a missing feature in git itself
where the carpet is rudely swept from under you. You'll still be able to
build, you just need to tweak your recipe.

The real value of $(warning) (or the same with @echo) is IMO something
like 6cdccfce1e0 (i18n: make GETTEXT_POISON a runtime option,
2018-11-08), i.e. to give someone a hint that something works
differently now (although I'd probably just make that $(error) if I was
doing it now, with the same rationale).