{"thread":{"id":"56443","subject":"[PATCH RESEND v3] make: add INSTALL_STRIP option variable","startedAt":"2021-09-05T11:04:58Z","lastAt":"2021-09-06T06:49:17Z","messageCount":4,"participants":["Bagas Sanjaya","Junio C Hamano","Đoàn Trần Công Danh"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"434724","messageId":"20210905110417.35702-1-bagasdotme@gmail.com","threadId":"56443","inReplyTo":null,"subject":"[PATCH RESEND v3] make: add INSTALL_STRIP option variable","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-09-05T11:04:18Z","receivedAt":"2021-09-05T11:04:58Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"From: Junio C Hamano \n\nAdd $(INSTALL_STRIP), which allows passing stripping options to\n$(INSTALL).\n\nFor this to work, installing executables must be split to installing\ncompiled binaries and scripts portions, since $(INSTALL_STRIP) is only\nmeaningful to the former.\n\nUsers can set this variable depending on their system. For example,\nLinux users can use `-s --strip-program=strip`, while FreeBSD users can\nsimply set to `-s` and choose strip program with $STRIPBIN.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Bagas Sanjaya \n---\n\n Changes from v2 [1]:\n - Squash suggestion patch from Junio, which:\n - drops install-stripped target and rename $(INSTALL_OPTS) to\n $(INSTALL_STRIP).\n - Describe $(INSTALL_STRIP) usage on the top of Makefile\n - Rebase on e0a2f5cbc5 (The third batch, 2021-09-03)\n\n Note: In the future, we may add global $(INSTALL_OPTS), which applies\n to both compiled binaries and scripts. When such happens, we should\n rename $(INSTALL_STRIP) to $(INSTALL_STRIP_OPTS).\n\n [QUESTION]: I squashed Junio's suggestion patch to produce this patch,\n and I want to credit his work. In such situation, what should I do the\n right way? For now I add From: line and S-o-b trailer with his\n identity, in addition to my S-o-b.\n\n [1]: https://lore.kernel.org/git/xmqqo89cqkt9.fsf@gitster.g/T/#t\n Makefile | 15 ++++++++++++---\n 1 file changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 429c276058..c7a9995062 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -465,6 +465,9 @@ all::\n # the global variable _wpgmptr containing the absolute path of the current\n # executable (this is the case on Windows).\n #\n+# INSTALL_STRIP can be set to \"-s\" to strip binaries during installation,\n+# if your $(INSTALL) command supports the option.\n+#\n # Define GENERATE_COMPILATION_DATABASE to \"yes\" to generate JSON compilation\n # database entries during compilation if your compiler supports it, using the\n # `-MJ` flag. The JSON entries will be placed in the `compile_commands/`\n@@ -3004,7 +3007,8 @@ mergetools_instdir = $(prefix)/$(mergetoolsdir)\n endif\n mergetools_instdir_SQ = $(subst ','\\'',$(mergetools_instdir))\n \n-install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n+install_bindir_xprograms := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X))\n+install_bindir_programs := $(install_bindir_xprograms) $(BINDIR_PROGRAMS_NO_X)\n \n .PHONY: profile-install profile-fast-install\n profile-install: profile\n@@ -3013,12 +3017,17 @@ profile-install: profile\n profile-fast-install: profile-fast\n \t$(MAKE) install\n \n+INSTALL_STRIP =\n+\n install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) $(INSTALL_STRIP) $(PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) $(SCRIPTS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n+\t$(INSTALL) $(INSTALL_STRIP) $(install_bindir_xprograms) '$(DESTDIR_SQ)$(bindir_SQ)'\n+\t$(INSTALL) $(BINDIR_PROGRAMS_NO_X) '$(DESTDIR_SQ)$(bindir_SQ)'\n+\n ifdef MSVC\n \t# We DO NOT install the individual foo.o.pdb files because they\n \t# have already been rolled up into the exe's pdb file.\n\nbase-commit: e0a2f5cbc585657e757385ad918f167f519cfb96\n-- \n2.25.1\n\n"},{"id":"434735","messageId":"xmqqwnnuesrf.fsf@gitster.g","threadId":"56443","inReplyTo":"20210905110417.35702-1-bagasdotme@gmail.com","subject":"Re: [PATCH RESEND v3] make: add INSTALL_STRIP option variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-05T19:17:56Z","receivedAt":"2021-09-05T19:18:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bagas Sanjaya writes:\n\n> [QUESTION]: I squashed Junio's suggestion patch to produce this patch,\n> and I want to credit his work. In such situation, what should I do the\n> right way? For now I add From: line and S-o-b trailer with his\n> identity, in addition to my S-o-b.\n\nAs far as I'm concerned, everything we see in this resulting patch\n(like studying how we should sift $(ALL_PROGRAMS) into two classes,\none that can be stripped and the other that shouldn't be) came from\nyour brain. I may have helped you in writing it down in a better\nform but I see it within the usual \"Helped-by:\".\n\nApplying this to the same base as the previous iteration of the\ntopic that I queued in 'seen', and running \"git range-diff\" between\nthem, I see that there is no difference at all, so I'll keep the one\nI already have, but I probably should correct the authorship\ninformation for it (I failed to notice you had in-body From: header\nthat shifts the blame for this change on me---you should be the\nauthor of this change).\n\nIOW, here is what I would expect in a situation like this.\n\n-- >8 --\nFrom: Bagas Sanjaya \nSubject: [PATCH] make: add INSTALL_STRIP option variable\n\nAdd $(INSTALL_STRIP), which allows passing stripping options to\n$(INSTALL).\n\nFor this to work, installing executables must be split to installing\ncompiled binaries and scripts portions, since $(INSTALL_STRIP) is only\nmeaningful to the former.\n\nUsers can set this variable depending on their system. For example,\nLinux users can use `-s --strip-program=strip`, while FreeBSD users can\nsimply set to `-s` and choose strip program with $STRIPBIN.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Bagas Sanjaya \nSigned-off-by: Junio C Hamano \n---\n Makefile | 15 ++++++++++++---\n 1 file changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex d1feab008f..ebef4da50c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -465,6 +465,9 @@ all::\n # the global variable _wpgmptr containing the absolute path of the current\n # executable (this is the case on Windows).\n #\n+# INSTALL_STRIP can be set to \"-s\" to strip binaries during installation,\n+# if your $(INSTALL) command supports the option.\n+#\n # Define GENERATE_COMPILATION_DATABASE to \"yes\" to generate JSON compilation\n # database entries during compilation if your compiler supports it, using the\n # `-MJ` flag. The JSON entries will be placed in the `compile_commands/`\n@@ -3004,7 +3007,8 @@ mergetools_instdir = $(prefix)/$(mergetoolsdir)\n endif\n mergetools_instdir_SQ = $(subst ','\\'',$(mergetools_instdir))\n \n-install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n+install_bindir_xprograms := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X))\n+install_bindir_programs := $(install_bindir_xprograms) $(BINDIR_PROGRAMS_NO_X)\n \n .PHONY: profile-install profile-fast-install\n profile-install: profile\n@@ -3013,12 +3017,17 @@ profile-install: profile\n profile-fast-install: profile-fast\n \t$(MAKE) install\n \n+INSTALL_STRIP =\n+\n install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) $(INSTALL_STRIP) $(PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) $(SCRIPTS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n+\t$(INSTALL) $(INSTALL_STRIP) $(install_bindir_xprograms) '$(DESTDIR_SQ)$(bindir_SQ)'\n+\t$(INSTALL) $(BINDIR_PROGRAMS_NO_X) '$(DESTDIR_SQ)$(bindir_SQ)'\n+\n ifdef MSVC\n \t# We DO NOT install the individual foo.o.pdb files because they\n \t# have already been rolled up into the exe's pdb file.\n-- \n2.33.0-408-g8e1aa136b3\n\n"},{"id":"434753","messageId":"YTWG8xpoxXvRkoNV@danh.dev","threadId":"56443","inReplyTo":"xmqqwnnuesrf.fsf@gitster.g","subject":"Re: [PATCH RESEND v3] make: add INSTALL_STRIP option variable","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-09-06T03:11:47Z","receivedAt":"2021-09-06T03:11:53Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-09-05 12:17:56-0700, Junio C Hamano wrote:\n> Bagas Sanjaya writes:\n> \n> > [QUESTION]: I squashed Junio's suggestion patch to produce this patch,\n> > and I want to credit his work. In such situation, what should I do the\n> > right way? For now I add From: line and S-o-b trailer with his\n> > identity, in addition to my S-o-b.\n> \n> As far as I'm concerned, everything we see in this resulting patch\n> (like studying how we should sift $(ALL_PROGRAMS) into two classes,\n> one that can be stripped and the other that shouldn't be) came from\n> your brain. I may have helped you in writing it down in a better\n> form but I see it within the usual \"Helped-by:\".\n\nFWIW, it was rewrite from my original suggestion in \n\n> Applying this to the same base as the previous iteration of the\n> topic that I queued in 'seen', and running \"git range-diff\" between\n> them, I see that there is no difference at all, so I'll keep the one\n> I already have, but I probably should correct the authorship\n> information for it (I failed to notice you had in-body From: header\n> that shifts the blame for this change on me---you should be the\n> author of this change).\n> \n> IOW, here is what I would expect in a situation like this.\n> \n> -- >8 --\n> From: Bagas Sanjaya \n> Subject: [PATCH] make: add INSTALL_STRIP option variable\n> \n> Add $(INSTALL_STRIP), which allows passing stripping options to\n> $(INSTALL).\n> \n> For this to work, installing executables must be split to installing\n> compiled binaries and scripts portions, since $(INSTALL_STRIP) is only\n> meaningful to the former.\n> \n> Users can set this variable depending on their system. For example,\n> Linux users can use `-s --strip-program=strip`, while FreeBSD users can\n> simply set to `-s` and choose strip program with $STRIPBIN.\n> \n> Helped-by: Junio C Hamano \n> Signed-off-by: Bagas Sanjaya \n> Signed-off-by: Junio C Hamano \n\nSo, here is my SoB, too:\n\nSigned-off-by: Đoàn Trần Công Danh \n\n-- Danh\n\n> ---\n> Makefile | 15 ++++++++++++---\n> 1 file changed, 12 insertions(+), 3 deletions(-)\n> \n> diff --git a/Makefile b/Makefile\n> index d1feab008f..ebef4da50c 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -465,6 +465,9 @@ all::\n> # the global variable _wpgmptr containing the absolute path of the current\n> # executable (this is the case on Windows).\n> #\n> +# INSTALL_STRIP can be set to \"-s\" to strip binaries during installation,\n> +# if your $(INSTALL) command supports the option.\n> +#\n> # Define GENERATE_COMPILATION_DATABASE to \"yes\" to generate JSON compilation\n> # database entries during compilation if your compiler supports it, using the\n> # `-MJ` flag. The JSON entries will be placed in the `compile_commands/`\n> @@ -3004,7 +3007,8 @@ mergetools_instdir = $(prefix)/$(mergetoolsdir)\n> endif\n> mergetools_instdir_SQ = $(subst ','\\'',$(mergetools_instdir))\n> \n> -install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n> +install_bindir_xprograms := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X))\n> +install_bindir_programs := $(install_bindir_xprograms) $(BINDIR_PROGRAMS_NO_X)\n> \n> .PHONY: profile-install profile-fast-install\n> profile-install: profile\n> @@ -3013,12 +3017,17 @@ profile-install: profile\n> profile-fast-install: profile-fast\n> \t$(MAKE) install\n> \n> +INSTALL_STRIP =\n> +\n> install: all\n> \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n> \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> -\t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> +\t$(INSTALL) $(INSTALL_STRIP) $(PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> +\t$(INSTALL) $(SCRIPTS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> \t$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> -\t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n> +\t$(INSTALL) $(INSTALL_STRIP) $(install_bindir_xprograms) '$(DESTDIR_SQ)$(bindir_SQ)'\n> +\t$(INSTALL) $(BINDIR_PROGRAMS_NO_X) '$(DESTDIR_SQ)$(bindir_SQ)'\n> +\n> ifdef MSVC\n> \t# We DO NOT install the individual foo.o.pdb files because they\n> \t# have already been rolled up into the exe's pdb file.\n> -- \n> 2.33.0-408-g8e1aa136b3\n> \n\n-- \nDanh\n"},{"id":"434764","messageId":"xmqqh7eydwrd.fsf@gitster.g","threadId":"56443","inReplyTo":"YTWG8xpoxXvRkoNV@danh.dev","subject":"Re: [PATCH RESEND v3] make: add INSTALL_STRIP option variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-06T06:49:10Z","receivedAt":"2021-09-06T06:49:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> So, here is my SoB, too:\n>\n> Signed-off-by: Đoàn Trần Công Danh \n\nThanks.\n"}]}