| Message ID | 20211125122607.26602-1-bagasdotme@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] Makefile: error out invoking strip target | expand |
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.
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.
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
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(+) base-commit: 35151cf0720460a897cde9b8039af364743240e7