{"thread":{"id":"56978","subject":"[PATCH v2] Makefile: error out invoking strip target","startedAt":"2021-11-25T12:28:38Z","lastAt":"2021-11-26T07:41:05Z","messageCount":3,"participants":["Bagas Sanjaya","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"442304","messageId":"20211125122607.26602-1-bagasdotme@gmail.com","threadId":"56978","inReplyTo":null,"subject":"[PATCH v2] Makefile: error out invoking strip target","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-11-25T12:26:08Z","receivedAt":"2021-11-25T12:28:38Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"Now that $INSTALL_STRIP variable can be defined since 3231f41009 (make:\nadd INSTALL_STRIP option variable, 2021-09-05), it is redundant to have\n'strip' target when $INSTALL_STRIP does the job. Error out when invoking\nthe target so that users are forced to define the variable instead.\n\nSigned-off-by: Bagas Sanjaya <bagasdotme@gmail.com>\n---\n Changes since v1 [1]:\n   - use $(error) function (suggested by Ævar)\n   - message rewording (suggested by Ævar)\n\n [1]:\nhttps://lore.kernel.org/git/211123.86a6hvuikj.gmgdl@evledraar.gmail.com/T/#u\n\n Makefile | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 12be39ac49..d569b5cba8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2159,6 +2159,9 @@ please_set_SHELL_PATH_to_a_more_modern_shell:\n shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell\n \n strip: $(PROGRAMS) git$X\n+\t$(error You are about to install stripped Git binaries using 'strip' \\\n+target, but it is deprecated and will be removed in future version of Git, \\\n+define INSTALL_STRIP instead)\n \t$(STRIP) $(STRIP_OPTS) $^\n \n ### Flags affecting all rules\n\nbase-commit: 35151cf0720460a897cde9b8039af364743240e7\n-- \nAn old man doll... just what I always wanted! - Clara\n\n"},{"id":"442345","messageId":"xmqqilwf8je9.fsf@gitster.g","threadId":"56978","inReplyTo":"20211125122607.26602-1-bagasdotme@gmail.com","subject":"Re: [PATCH v2] Makefile: error out invoking strip target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-26T07:29:18Z","receivedAt":"2021-11-26T07:31:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bagas Sanjaya <bagasdotme@gmail.com> writes:\n\n> Now that $INSTALL_STRIP variable can be defined since 3231f41009 (make:\n> add INSTALL_STRIP option variable, 2021-09-05), it is redundant to have\n> 'strip' target when $INSTALL_STRIP does the job. Error out when invoking\n> the target so that users are forced to define the variable instead.\n\nIt is not exactly redundant for folks who like to build and use in\nplace without installing.\n\nWhat is the reason why we might want to eventually remove the\n\"strip\" target, making \"make strip\" an error?  I do not quite see\nmuch downsides for having just a target with a simple one-liner\nrecipe.\n\n"},{"id":"442347","messageId":"c057b7fc-54b3-4946-16ad-8fbd548b505d@gmail.com","threadId":"56978","inReplyTo":"xmqqilwf8je9.fsf@gitster.g","subject":"Re: [PATCH v2] Makefile: error out invoking strip target","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-11-26T07:38:59Z","receivedAt":"2021-11-26T07:41:05Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 26/11/21 14.29, Junio C Hamano wrote:\n> Bagas Sanjaya <bagasdotme@gmail.com> writes:\n> \n>> Now that $INSTALL_STRIP variable can be defined since 3231f41009 (make:\n>> add INSTALL_STRIP option variable, 2021-09-05), it is redundant to have\n>> 'strip' target when $INSTALL_STRIP does the job. Error out when invoking\n>> the target so that users are forced to define the variable instead.\n> \n> It is not exactly redundant for folks who like to build and use in\n> place without installing.\n> \n> What is the reason why we might want to eventually remove the\n> \"strip\" target, making \"make strip\" an error?  I do not quite see\n> much downsides for having just a target with a simple one-liner\n> recipe.\n> \n\nI think we have two ways to do the same thing (installing stripped) and I want \nto push users to go with $INSTALL_STRIP instead of strip target.\n\nRegarding deprecation, making $(warning) message instead of $(error) is better \noption, because users can still use the target (albeit it is deprecated) and \nthey can update their build recipe to use $INSTALL_STRIP before we flip to \n$(error) or remove the target.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"}]}