git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [RFC PATCH] Makefile: add deprecation message for strip target
@ 2021-11-23 12:29 Bagas Sanjaya
  2021-11-23 13:00 ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 2+ messages in thread
From: Bagas Sanjaya @ 2021-11-23 12:29 UTC (permalink / raw)
  To: git; +Cc: Bagas Sanjaya

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
-- 
An old man doll... just what I always wanted! - Clara


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

* Re: [RFC PATCH] Makefile: add deprecation message for strip target
  2021-11-23 12:29 [RFC PATCH] Makefile: add deprecation message for strip target Bagas Sanjaya
@ 2021-11-23 13:00 ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 2+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-23 13:00 UTC (permalink / raw)
  To: Bagas Sanjaya; +Cc: git


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

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

end of thread, other threads:[~2021-11-23 13:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-23 12:29 [RFC PATCH] Makefile: add deprecation message for strip target Bagas Sanjaya
2021-11-23 13:00 ` Ævar Arnfjörð Bjarmason

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).