git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2] Makefile: error out invoking strip target
@ 2021-11-25 12:26 Bagas Sanjaya
  2021-11-26  7:29 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Bagas Sanjaya @ 2021-11-25 12:26 UTC (permalink / raw)
  To: git; +Cc: Ævar Arnfjörð Bjarmason, Bagas Sanjaya

Now that $INSTALL_STRIP variable can be defined since 3231f41009 (make:
add INSTALL_STRIP option variable, 2021-09-05), it is redundant to have
'strip' target when $INSTALL_STRIP does the job. Error out when invoking
the target so that users are forced to define the variable instead.

Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 Changes since v1 [1]:
   - use $(error) function (suggested by Ævar)
   - message rewording (suggested by Ævar)

 [1]:
https://lore.kernel.org/git/211123.86a6hvuikj.gmgdl@evledraar.gmail.com/T/#u

 Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile b/Makefile
index 12be39ac49..d569b5cba8 100644
--- a/Makefile
+++ b/Makefile
@@ -2159,6 +2159,9 @@ 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
+	$(error You are about to install stripped Git binaries using 'strip' \
+target, but it is deprecated and will be removed in future version of Git, \
+define INSTALL_STRIP instead)
 	$(STRIP) $(STRIP_OPTS) $^
 
 ### Flags affecting all rules

base-commit: 35151cf0720460a897cde9b8039af364743240e7
-- 
An old man doll... just what I always wanted! - Clara


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

* Re: [PATCH v2] Makefile: error out invoking strip target
  2021-11-25 12:26 [PATCH v2] Makefile: error out invoking strip target Bagas Sanjaya
@ 2021-11-26  7:29 ` Junio C Hamano
  2021-11-26  7:38   ` Bagas Sanjaya
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2021-11-26  7:29 UTC (permalink / raw)
  To: Bagas Sanjaya; +Cc: git, Ævar Arnfjörð Bjarmason

Bagas Sanjaya <bagasdotme@gmail.com> writes:

> Now that $INSTALL_STRIP variable can be defined since 3231f41009 (make:
> add INSTALL_STRIP option variable, 2021-09-05), it is redundant to have
> 'strip' target when $INSTALL_STRIP does the job. Error out when invoking
> the target so that users are forced to define the variable instead.

It is not exactly redundant for folks who like to build and use in
place without installing.

What is the reason why we might want to eventually remove the
"strip" target, making "make strip" an error?  I do not quite see
much downsides for having just a target with a simple one-liner
recipe.


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

* Re: [PATCH v2] Makefile: error out invoking strip target
  2021-11-26  7:29 ` Junio C Hamano
@ 2021-11-26  7:38   ` Bagas Sanjaya
  0 siblings, 0 replies; 3+ messages in thread
From: Bagas Sanjaya @ 2021-11-26  7:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Ævar Arnfjörð Bjarmason

On 26/11/21 14.29, Junio C Hamano wrote:
> Bagas Sanjaya <bagasdotme@gmail.com> writes:
> 
>> Now that $INSTALL_STRIP variable can be defined since 3231f41009 (make:
>> add INSTALL_STRIP option variable, 2021-09-05), it is redundant to have
>> 'strip' target when $INSTALL_STRIP does the job. Error out when invoking
>> the target so that users are forced to define the variable instead.
> 
> It is not exactly redundant for folks who like to build and use in
> place without installing.
> 
> What is the reason why we might want to eventually remove the
> "strip" target, making "make strip" an error?  I do not quite see
> much downsides for having just a target with a simple one-liner
> recipe.
> 

I think we have two ways to do the same thing (installing stripped) and I want 
to push users to go with $INSTALL_STRIP instead of strip target.

Regarding deprecation, making $(warning) message instead of $(error) is better 
option, because users can still use the target (albeit it is deprecated) and 
they can update their build recipe to use $INSTALL_STRIP before we flip to 
$(error) or remove the target.

-- 
An old man doll... just what I always wanted! - Clara

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

end of thread, other threads:[~2021-11-26  7:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25 12:26 [PATCH v2] Makefile: error out invoking strip target Bagas Sanjaya
2021-11-26  7:29 ` Junio C Hamano
2021-11-26  7:38   ` Bagas Sanjaya

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