* [PATCH RESEND v3] make: add INSTALL_STRIP option variable
@ 2021-09-05 11:04 Bagas Sanjaya
2021-09-05 19:17 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Bagas Sanjaya @ 2021-09-05 11:04 UTC (permalink / raw)
To: git
Cc: Carlo Arenas, Đoàn Trần Công Danh,
felipe.contreras, Ævar Arnfjörð Bjarmason,
Eric Sunshine, Johannes Schindelin, Elijah Newren,
Derrick Stolee, Bagas Sanjaya, Junio C Hamano
From: Junio C Hamano <gitster@pobox.com>
Add $(INSTALL_STRIP), which allows passing stripping options to
$(INSTALL).
For this to work, installing executables must be split to installing
compiled binaries and scripts portions, since $(INSTALL_STRIP) is only
meaningful to the former.
Users can set this variable depending on their system. For example,
Linux users can use `-s --strip-program=strip`, while FreeBSD users can
simply set to `-s` and choose strip program with $STRIPBIN.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
Changes from v2 [1]:
- Squash suggestion patch from Junio, which:
- drops install-stripped target and rename $(INSTALL_OPTS) to
$(INSTALL_STRIP).
- Describe $(INSTALL_STRIP) usage on the top of Makefile
- Rebase on e0a2f5cbc5 (The third batch, 2021-09-03)
Note: In the future, we may add global $(INSTALL_OPTS), which applies
to both compiled binaries and scripts. When such happens, we should
rename $(INSTALL_STRIP) to $(INSTALL_STRIP_OPTS).
[QUESTION]: I squashed Junio's suggestion patch to produce this patch,
and I want to credit his work. In such situation, what should I do the
right way? For now I add From: line and S-o-b trailer with his
identity, in addition to my S-o-b.
[1]: https://lore.kernel.org/git/xmqqo89cqkt9.fsf@gitster.g/T/#t
Makefile | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 429c276058..c7a9995062 100644
--- a/Makefile
+++ b/Makefile
@@ -465,6 +465,9 @@ all::
# the global variable _wpgmptr containing the absolute path of the current
# executable (this is the case on Windows).
#
+# INSTALL_STRIP can be set to "-s" to strip binaries during installation,
+# if your $(INSTALL) command supports the option.
+#
# Define GENERATE_COMPILATION_DATABASE to "yes" to generate JSON compilation
# database entries during compilation if your compiler supports it, using the
# `-MJ` flag. The JSON entries will be placed in the `compile_commands/`
@@ -3004,7 +3007,8 @@ mergetools_instdir = $(prefix)/$(mergetoolsdir)
endif
mergetools_instdir_SQ = $(subst ','\'',$(mergetools_instdir))
-install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)
+install_bindir_xprograms := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X))
+install_bindir_programs := $(install_bindir_xprograms) $(BINDIR_PROGRAMS_NO_X)
.PHONY: profile-install profile-fast-install
profile-install: profile
@@ -3013,12 +3017,17 @@ profile-install: profile
profile-fast-install: profile-fast
$(MAKE) install
+INSTALL_STRIP =
+
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
- $(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+ $(INSTALL) $(INSTALL_STRIP) $(PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+ $(INSTALL) $(SCRIPTS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
- $(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
+ $(INSTALL) $(INSTALL_STRIP) $(install_bindir_xprograms) '$(DESTDIR_SQ)$(bindir_SQ)'
+ $(INSTALL) $(BINDIR_PROGRAMS_NO_X) '$(DESTDIR_SQ)$(bindir_SQ)'
+
ifdef MSVC
# We DO NOT install the individual foo.o.pdb files because they
# have already been rolled up into the exe's pdb file.
base-commit: e0a2f5cbc585657e757385ad918f167f519cfb96
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v3] make: add INSTALL_STRIP option variable
2021-09-05 11:04 [PATCH RESEND v3] make: add INSTALL_STRIP option variable Bagas Sanjaya
@ 2021-09-05 19:17 ` Junio C Hamano
2021-09-06 3:11 ` Đoàn Trần Công Danh
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2021-09-05 19:17 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: git, Carlo Arenas, Đoàn Trần Công Danh,
felipe.contreras, Ævar Arnfjörð Bjarmason,
Eric Sunshine, Johannes Schindelin, Elijah Newren,
Derrick Stolee
Bagas Sanjaya <bagasdotme@gmail.com> writes:
> [QUESTION]: I squashed Junio's suggestion patch to produce this patch,
> and I want to credit his work. In such situation, what should I do the
> right way? For now I add From: line and S-o-b trailer with his
> identity, in addition to my S-o-b.
As far as I'm concerned, everything we see in this resulting patch
(like studying how we should sift $(ALL_PROGRAMS) into two classes,
one that can be stripped and the other that shouldn't be) came from
your brain. I may have helped you in writing it down in a better
form but I see it within the usual "Helped-by:".
Applying this to the same base as the previous iteration of the
topic that I queued in 'seen', and running "git range-diff" between
them, I see that there is no difference at all, so I'll keep the one
I already have, but I probably should correct the authorship
information for it (I failed to notice you had in-body From: header
that shifts the blame for this change on me---you should be the
author of this change).
IOW, here is what I would expect in a situation like this.
-- >8 --
From: Bagas Sanjaya <bagasdotme@gmail.com>
Subject: [PATCH] make: add INSTALL_STRIP option variable
Add $(INSTALL_STRIP), which allows passing stripping options to
$(INSTALL).
For this to work, installing executables must be split to installing
compiled binaries and scripts portions, since $(INSTALL_STRIP) is only
meaningful to the former.
Users can set this variable depending on their system. For example,
Linux users can use `-s --strip-program=strip`, while FreeBSD users can
simply set to `-s` and choose strip program with $STRIPBIN.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Makefile | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index d1feab008f..ebef4da50c 100644
--- a/Makefile
+++ b/Makefile
@@ -465,6 +465,9 @@ all::
# the global variable _wpgmptr containing the absolute path of the current
# executable (this is the case on Windows).
#
+# INSTALL_STRIP can be set to "-s" to strip binaries during installation,
+# if your $(INSTALL) command supports the option.
+#
# Define GENERATE_COMPILATION_DATABASE to "yes" to generate JSON compilation
# database entries during compilation if your compiler supports it, using the
# `-MJ` flag. The JSON entries will be placed in the `compile_commands/`
@@ -3004,7 +3007,8 @@ mergetools_instdir = $(prefix)/$(mergetoolsdir)
endif
mergetools_instdir_SQ = $(subst ','\'',$(mergetools_instdir))
-install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)
+install_bindir_xprograms := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X))
+install_bindir_programs := $(install_bindir_xprograms) $(BINDIR_PROGRAMS_NO_X)
.PHONY: profile-install profile-fast-install
profile-install: profile
@@ -3013,12 +3017,17 @@ profile-install: profile
profile-fast-install: profile-fast
$(MAKE) install
+INSTALL_STRIP =
+
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
- $(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+ $(INSTALL) $(INSTALL_STRIP) $(PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+ $(INSTALL) $(SCRIPTS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
- $(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
+ $(INSTALL) $(INSTALL_STRIP) $(install_bindir_xprograms) '$(DESTDIR_SQ)$(bindir_SQ)'
+ $(INSTALL) $(BINDIR_PROGRAMS_NO_X) '$(DESTDIR_SQ)$(bindir_SQ)'
+
ifdef MSVC
# We DO NOT install the individual foo.o.pdb files because they
# have already been rolled up into the exe's pdb file.
--
2.33.0-408-g8e1aa136b3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v3] make: add INSTALL_STRIP option variable
2021-09-05 19:17 ` Junio C Hamano
@ 2021-09-06 3:11 ` Đoàn Trần Công Danh
2021-09-06 6:49 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Đoàn Trần Công Danh @ 2021-09-06 3:11 UTC (permalink / raw)
To: Junio C Hamano
Cc: Bagas Sanjaya, git, Carlo Arenas, felipe.contreras,
Ævar Arnfjörð Bjarmason, Eric Sunshine,
Johannes Schindelin, Elijah Newren, Derrick Stolee
On 2021-09-05 12:17:56-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Bagas Sanjaya <bagasdotme@gmail.com> writes:
>
> > [QUESTION]: I squashed Junio's suggestion patch to produce this patch,
> > and I want to credit his work. In such situation, what should I do the
> > right way? For now I add From: line and S-o-b trailer with his
> > identity, in addition to my S-o-b.
>
> As far as I'm concerned, everything we see in this resulting patch
> (like studying how we should sift $(ALL_PROGRAMS) into two classes,
> one that can be stripped and the other that shouldn't be) came from
> your brain. I may have helped you in writing it down in a better
> form but I see it within the usual "Helped-by:".
FWIW, it was rewrite from my original suggestion in <YSjdc/tNlhbCosC2@danh.dev>
> Applying this to the same base as the previous iteration of the
> topic that I queued in 'seen', and running "git range-diff" between
> them, I see that there is no difference at all, so I'll keep the one
> I already have, but I probably should correct the authorship
> information for it (I failed to notice you had in-body From: header
> that shifts the blame for this change on me---you should be the
> author of this change).
>
> IOW, here is what I would expect in a situation like this.
>
> -- >8 --
> From: Bagas Sanjaya <bagasdotme@gmail.com>
> Subject: [PATCH] make: add INSTALL_STRIP option variable
>
> Add $(INSTALL_STRIP), which allows passing stripping options to
> $(INSTALL).
>
> For this to work, installing executables must be split to installing
> compiled binaries and scripts portions, since $(INSTALL_STRIP) is only
> meaningful to the former.
>
> Users can set this variable depending on their system. For example,
> Linux users can use `-s --strip-program=strip`, while FreeBSD users can
> simply set to `-s` and choose strip program with $STRIPBIN.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
So, here is my SoB, too:
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
-- Danh
> ---
> Makefile | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d1feab008f..ebef4da50c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -465,6 +465,9 @@ all::
> # the global variable _wpgmptr containing the absolute path of the current
> # executable (this is the case on Windows).
> #
> +# INSTALL_STRIP can be set to "-s" to strip binaries during installation,
> +# if your $(INSTALL) command supports the option.
> +#
> # Define GENERATE_COMPILATION_DATABASE to "yes" to generate JSON compilation
> # database entries during compilation if your compiler supports it, using the
> # `-MJ` flag. The JSON entries will be placed in the `compile_commands/`
> @@ -3004,7 +3007,8 @@ mergetools_instdir = $(prefix)/$(mergetoolsdir)
> endif
> mergetools_instdir_SQ = $(subst ','\'',$(mergetools_instdir))
>
> -install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)
> +install_bindir_xprograms := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X))
> +install_bindir_programs := $(install_bindir_xprograms) $(BINDIR_PROGRAMS_NO_X)
>
> .PHONY: profile-install profile-fast-install
> profile-install: profile
> @@ -3013,12 +3017,17 @@ profile-install: profile
> profile-fast-install: profile-fast
> $(MAKE) install
>
> +INSTALL_STRIP =
> +
> install: all
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> - $(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> + $(INSTALL) $(INSTALL_STRIP) $(PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> + $(INSTALL) $(SCRIPTS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> $(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> - $(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
> + $(INSTALL) $(INSTALL_STRIP) $(install_bindir_xprograms) '$(DESTDIR_SQ)$(bindir_SQ)'
> + $(INSTALL) $(BINDIR_PROGRAMS_NO_X) '$(DESTDIR_SQ)$(bindir_SQ)'
> +
> ifdef MSVC
> # We DO NOT install the individual foo.o.pdb files because they
> # have already been rolled up into the exe's pdb file.
> --
> 2.33.0-408-g8e1aa136b3
>
--
Danh
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND v3] make: add INSTALL_STRIP option variable
2021-09-06 3:11 ` Đoàn Trần Công Danh
@ 2021-09-06 6:49 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2021-09-06 6:49 UTC (permalink / raw)
To: Đoàn Trần Công Danh
Cc: Bagas Sanjaya, git, Carlo Arenas, felipe.contreras,
Ævar Arnfjörð Bjarmason, Eric Sunshine,
Johannes Schindelin, Elijah Newren, Derrick Stolee
Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
> So, here is my SoB, too:
>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-06 6:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-05 11:04 [PATCH RESEND v3] make: add INSTALL_STRIP option variable Bagas Sanjaya
2021-09-05 19:17 ` Junio C Hamano
2021-09-06 3:11 ` Đoàn Trần Công Danh
2021-09-06 6:49 ` Junio C Hamano
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 NNTP newsgroup(s).