All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bagas Sanjaya <bagasdotme@gmail.com>
To: git@vger.kernel.org
Cc: "Carlo Arenas" <carenas@gmail.com>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
	felipe.contreras@gmail.com,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Elijah Newren" <newren@gmail.com>,
	"Derrick Stolee" <stolee@gmail.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: [PATCH RESEND v3] make: add INSTALL_STRIP option variable
Date: Sun,  5 Sep 2021 18:04:18 +0700	[thread overview]
Message-ID: <20210905110417.35702-1-bagasdotme@gmail.com> (raw)

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


             reply	other threads:[~2021-09-05 11:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-05 11:04 Bagas Sanjaya [this message]
2021-09-05 19:17 ` [PATCH RESEND v3] make: add INSTALL_STRIP option variable Junio C Hamano
2021-09-06  3:11   ` Đoàn Trần Công Danh
2021-09-06  6:49     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210905110417.35702-1-bagasdotme@gmail.com \
    --to=bagasdotme@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=carenas@gmail.com \
    --cc=congdanhqx@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=stolee@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.