{"thread":{"id":"56332","subject":"[PATCH] make: add INSTALL_STRIP variable","startedAt":"2021-08-20T10:54:11Z","lastAt":"2021-08-24T18:58:22Z","messageCount":10,"participants":["Bagas Sanjaya","Đoàn Trần Công Danh","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"433214","messageId":"20210820105052.30631-1-bagasdotme@gmail.com","threadId":"56332","inReplyTo":null,"subject":"[PATCH] make: add INSTALL_STRIP variable","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-08-20T10:50:53Z","receivedAt":"2021-08-20T10:54:11Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"In some environments (most notably embedded systems and small production\nservers), it is often desirable to have stripped Git binaries due to\ntight disk space constraint.\n\nUntil now stripped Git can be built wih `make strip install`. Add\nINSTALL_STRIP make variable so that they can install stripped Git\nbinaries with `make INSTALL_STRIP=yes install`.\n\nAlso document stripping and using INSTALL_STRIP in INSTALL.\n\nSigned-off-by: Bagas Sanjaya <bagasdotme@gmail.com>\n---\n Junio suggested me to have INSTALL_STRIP make variable [1] when\n reviewing the install-strip target patch.\n\n [1]:\nhttps://lore.kernel.org/git/xmqq1r6p1ark.fsf@gitster.g/T/#mc3b8017448bdafedf9250ba407f5de767c20ad67\n\n INSTALL  | 8 ++++++++\n Makefile | 5 +++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/INSTALL b/INSTALL\nindex 66389ce059..98e541ee4d 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -58,6 +58,14 @@ suite has to be run using only a single CPU.  In addition, the profile\n feedback build stage currently generates a lot of additional compiler\n warnings.\n \n+You can also strip debug info from built binaries by:\n+\n+\t$ make strip\n+\n+or for stripping and installing together:\n+\n+\t$ make INSTALL_STRIP=yes install\n+\n Issues of note:\n \n  - Ancient versions of GNU Interactive Tools (pre-4.9.2) installed a\ndiff --git a/Makefile b/Makefile\nindex 9573190f1d..e486f3ab75 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -8,6 +8,8 @@ all::\n # Define SANE_TOOL_PATH to a colon-separated list of paths to prepend\n # to PATH if your tools in /usr/bin are broken.\n #\n+# Define INSTALL_STRIP if you want to install with stripped binaries.\n+#\n # Define SOCKLEN_T to a suitable type (such as 'size_t') if your\n # system headers do not define a socklen_t type.\n #\n@@ -3005,6 +3007,9 @@ profile-fast-install: profile-fast\n \t$(MAKE) install\n \n install: all\n+ifdef INSTALL_STRIP\n+\t$(MAKE) strip\n+endif\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\nbase-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf\n-- \n2.25.1\n\n"},{"id":"433215","messageId":"YR+Tp2AGeeKyRKoC@danh.dev","threadId":"56332","inReplyTo":"20210820105052.30631-1-bagasdotme@gmail.com","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-20T11:36:07Z","receivedAt":"2021-08-20T11:36:15Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-08-20 17:50:53+0700, Bagas Sanjaya <bagasdotme@gmail.com> wrote:\n> In some environments (most notably embedded systems and small production\n> servers), it is often desirable to have stripped Git binaries due to\n> tight disk space constraint.\n> \n> Until now stripped Git can be built wih `make strip install`. Add\n> INSTALL_STRIP make variable so that they can install stripped Git\n> binaries with `make INSTALL_STRIP=yes install`.\n> \n> Also document stripping and using INSTALL_STRIP in INSTALL.\n> \n> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>\n> ---\n>  Junio suggested me to have INSTALL_STRIP make variable [1] when\n>  reviewing the install-strip target patch.\n> \n>  [1]:\n> https://lore.kernel.org/git/xmqq1r6p1ark.fsf@gitster.g/T/#mc3b8017448bdafedf9250ba407f5de767c20ad67\n> \n>  INSTALL  | 8 ++++++++\n>  Makefile | 5 +++++\n>  2 files changed, 13 insertions(+)\n> \n> diff --git a/INSTALL b/INSTALL\n> index 66389ce059..98e541ee4d 100644\n> --- a/INSTALL\n> +++ b/INSTALL\n> @@ -58,6 +58,14 @@ suite has to be run using only a single CPU.  In addition, the profile\n>  feedback build stage currently generates a lot of additional compiler\n>  warnings.\n>  \n> +You can also strip debug info from built binaries by:\n> +\n> +\t$ make strip\n> +\n> +or for stripping and installing together:\n> +\n> +\t$ make INSTALL_STRIP=yes install\n> +\n>  Issues of note:\n>  \n>   - Ancient versions of GNU Interactive Tools (pre-4.9.2) installed a\n> diff --git a/Makefile b/Makefile\n> index 9573190f1d..e486f3ab75 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -8,6 +8,8 @@ all::\n>  # Define SANE_TOOL_PATH to a colon-separated list of paths to prepend\n>  # to PATH if your tools in /usr/bin are broken.\n>  #\n> +# Define INSTALL_STRIP if you want to install with stripped binaries.\n> +#\n>  # Define SOCKLEN_T to a suitable type (such as 'size_t') if your\n>  # system headers do not define a socklen_t type.\n>  #\n> @@ -3005,6 +3007,9 @@ profile-fast-install: profile-fast\n>  \t$(MAKE) install\n>  \n>  install: all\n> +ifdef INSTALL_STRIP\n> +\t$(MAKE) strip\n> +endif\n\nI believe it's better to write like this:\n\n----- 8< ------\nifdef INSTALL_STRIP\ninstall: strip\nendif\n\ninstall: all\n\t....\n---- >8-------\n\nIOW, install depends on strip, not install invoke strip.\nI think it would work better for:\n\n\tmake install strip\n\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> \n> base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf\n> -- \n> 2.25.1\n> \n\n-- \nDanh\n"},{"id":"433266","messageId":"xmqqwnogt20q.fsf@gitster.g","threadId":"56332","inReplyTo":"YR+Tp2AGeeKyRKoC@danh.dev","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-20T18:16:37Z","receivedAt":"2021-08-20T18:16:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n>>  install: all\n>> +ifdef INSTALL_STRIP\n>> +\t$(MAKE) strip\n>> +endif\n>\n> I believe it's better to write like this:\n>\n> ----- 8< ------\n> ifdef INSTALL_STRIP\n> install: strip\n> endif\n>\n> install: all\n> \t....\n> ---- >8-------\n>\n> IOW, install depends on strip, not install invoke strip.\n> I think it would work better for:\n>\n> \tmake install strip\n\nI think you meant \"it would work better than 'make install strip'\",\nand if so, I tend to agree.  With\n\n\techo INSTALL_STRIP=YesPlease >>config.mak\n\neither Bagas's or your \"before installing, make sure we strip\"\nchange lets\n\n\tmake install\n\njust work without \"strip\" given on the command line.\n\nIf users with such a config.mak type \"make install strip\", it will\nmake the recipe for \"install\" wait until \"strip\" is done, which is\nwhat we want, but \"strip\" on the command line for them is redundant,\nand there is no way for them to install unstripped binaries, which\nmay be a bit of downside.\n\nBut for those who do not always want to use INSTALL_STRIP, as Dscho\nsaid after I mentioned the \"make variable\" thing, we probably a\nwrong thing when they say \"make -j strip install\", as there is\nnothing to make recipe for \"install\" to wait for \"strip\", so it is\nnot a fully satisfactory solution.\n\nI think we want two things:\n\n (1) if a user says \"make [-j] strip install\", make sure \"install\"\n     won't start before \"strip\" finishes;\n\n (2) if a user wants to always install stripped binary, allow some\n     make variable in config.mak so that \"make install\" would do\n     that without an explicit \"strip\".\n\nOf course, if a user does not have (2) configured, \"make install\"\nshould install unstripped binaries, but that goes without saying.\n\nAnd after thinking it like this, perhaps a new \"install-stripped\"\ntarget that runs \"strip\" and then \"install\" as originally proposed\nin the thread that triggered this discussion may be the simplest\napproach.  We can control the optional dependency between \"strip\"\nand \"install\", those who want to install stripped binary can use\n\"install-stripped\" instead of \"install\", and they can on-demand\nchoose to install unstripped binary (which was a potential downside\nof the \"make variable\" approach under discussion here).\n\nThanks.\n\n"},{"id":"433294","messageId":"YSBhPdK8jYIQUNhP@danh.dev","threadId":"56332","inReplyTo":"xmqqwnogt20q.fsf@gitster.g","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-21T02:13:17Z","receivedAt":"2021-08-21T02:13:22Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-08-20 11:16:37-0700, Junio C Hamano <gitster@pobox.com> wrote:\n> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n> \n> >>  install: all\n> >> +ifdef INSTALL_STRIP\n> >> +\t$(MAKE) strip\n> >> +endif\n> >\n> > I believe it's better to write like this:\n> >\n> > ----- 8< ------\n> > ifdef INSTALL_STRIP\n> > install: strip\n> > endif\n> >\n> > install: all\n> > \t....\n> > ---- >8-------\n> >\n> > IOW, install depends on strip, not install invoke strip.\n> > I think it would work better for:\n> >\n> > \tmake install strip\n> \n> I think you meant \"it would work better than 'make install strip'\",\n> and if so, I tend to agree.  With\n> \n> \techo INSTALL_STRIP=YesPlease >>config.mak\n> \n> either Bagas's or your \"before installing, make sure we strip\"\n> change lets\n> \n> \tmake install\n> \n> just work without \"strip\" given on the command line.\n> \n> If users with such a config.mak type \"make install strip\", it will\n> make the recipe for \"install\" wait until \"strip\" is done, which is\n> what we want, but \"strip\" on the command line for them is redundant,\n> and there is no way for them to install unstripped binaries, which\n> may be a bit of downside.\n> \n> But for those who do not always want to use INSTALL_STRIP, as Dscho\n> said after I mentioned the \"make variable\" thing, we probably a\n> wrong thing when they say \"make -j strip install\", as there is\n> nothing to make recipe for \"install\" to wait for \"strip\", so it is\n> not a fully satisfactory solution.\n\nIn that case, we can use this construct (since we depends on GNU Make,\nanyway).\n\n---- 8< ------\nifneq ($(filter install,$(MAKECMDGOALS)),)\nifneq ($(filter strip,$(MAKECMDGOALS)),)\ninstall: strip\nendif\nendif\n---- >8 -----\n\nMAKECMDGOALS is available from at least GNU Make 3.75.1 in 1997.\n\nAnyway, maybe it's only me, but I think people may want to install\nfirst, then strip later for debug mapping.\n\n> \n> I think we want two things:\n> \n>  (1) if a user says \"make [-j] strip install\", make sure \"install\"\n>      won't start before \"strip\" finishes;\n> \n>  (2) if a user wants to always install stripped binary, allow some\n>      make variable in config.mak so that \"make install\" would do\n>      that without an explicit \"strip\".\n> \n> Of course, if a user does not have (2) configured, \"make install\"\n> should install unstripped binaries, but that goes without saying.\n> \n> And after thinking it like this, perhaps a new \"install-stripped\"\n> target that runs \"strip\" and then \"install\" as originally proposed\n> in the thread that triggered this discussion may be the simplest\n> approach.  We can control the optional dependency between \"strip\"\n> and \"install\", those who want to install stripped binary can use\n> \"install-stripped\" instead of \"install\", and they can on-demand\n> choose to install unstripped binary (which was a potential downside\n> of the \"make variable\" approach under discussion here).\n> \n> Thanks.\n> \n\n-- \nDanh\n"},{"id":"433308","messageId":"70e609f5-bf40-658a-a021-ab7a0ad4baa1@gmail.com","threadId":"56332","inReplyTo":"YR+Tp2AGeeKyRKoC@danh.dev","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-08-21T08:24:22Z","receivedAt":"2021-08-21T08:24:31Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 20/08/21 18.36, Đoàn Trần Công Danh wrote:\n> I believe it's better to write like this:\n> \n> ----- 8< ------\n> ifdef INSTALL_STRIP\n> install: strip\n> endif\n> \n> install: all\n> \t....\n> ---- >8-------\n> \n> IOW, install depends on strip, not install invoke strip.\n\nOh, I missed that.\n\n> I think it would work better for:\n> \n> \tmake install strip\n> \n\nWouldn't it install unstripped binaries to the prefix then stripping \nthem in the build directory?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"433309","messageId":"YSDW3nsG2iWPGakF@danh.dev","threadId":"56332","inReplyTo":"70e609f5-bf40-658a-a021-ab7a0ad4baa1@gmail.com","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-21T10:35:24Z","receivedAt":"2021-08-21T10:35:32Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-08-21 15:24:22+0700, Bagas Sanjaya <bagasdotme@gmail.com> wrote:\n> On 20/08/21 18.36, Đoàn Trần Công Danh wrote:\n> > I believe it's better to write like this:\n> > \n> > ----- 8< ------\n> > ifdef INSTALL_STRIP\n> > install: strip\n> > endif\n> > \n> > install: all\n> > \t....\n> > ---- >8-------\n> > \n> > IOW, install depends on strip, not install invoke strip.\n> \n> Oh, I missed that.\n> \n> > I think it would work better for:\n> > \n> > \tmake install strip\n> > \n> \n> Wouldn't it install unstripped binaries to the prefix then stripping them in\n> the build directory?\n\nNo, with:\n\n\tinstall: strip\n\nstrip is one of install's dependencies, then strip will be executed\nbefore install.\n\n-- \nDanh\n"},{"id":"433450","messageId":"xmqq5yvwrw8s.fsf@gitster.g","threadId":"56332","inReplyTo":"YSBhPdK8jYIQUNhP@danh.dev","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-23T15:55:47Z","receivedAt":"2021-08-23T15:55:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> In that case, we can use this construct (since we depends on GNU Make,\n> anyway).\n>\n> ---- 8< ------\n> ifneq ($(filter install,$(MAKECMDGOALS)),)\n> ifneq ($(filter strip,$(MAKECMDGOALS)),)\n> install: strip\n> endif\n> endif\n> ---- >8 -----\n>\n> MAKECMDGOALS is available from at least GNU Make 3.75.1 in 1997.\n\nOr the \"|\" thing Dscho floated earlier?\n\n> Anyway, maybe it's only me, but I think people may want to install\n> first, then strip later for debug mapping.\n\nPerhaps.  One bad thing with the current \"strip\" arrangement is that\nit is done in the built directory, and because \"make install\" would\nblindly install whatever in the built directory, if you truly care\nthat you install unstripped binaries, you need to see if they are\nstripped and rebuild them as needed, because \"make strip\" may or may\nnot have been done.  From that point of view, getting rid of the\ncurrent \"make strip\" and introducing either \"make strip-installed\"\n(\"we've installed things earlier---go strip them\") or \"make\ninstall-stripped\" (\"we've built (or if we haven't please build them\nfirst), now install them and strip them in the installed directory\")\nmay make more sense.  And for that, any idea that came up in this\ndiscussion that relies on the current \"strip\" target would not help.\n\nThanks.\n\n"},{"id":"433521","messageId":"6e0ef7f2-3dd0-09e4-5a1d-7e59b979d624@gmail.com","threadId":"56332","inReplyTo":"xmqq5yvwrw8s.fsf@gitster.g","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-08-24T09:39:44Z","receivedAt":"2021-08-24T09:39:51Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 23/08/21 22.55, Junio C Hamano wrote:\n> Perhaps.  One bad thing with the current \"strip\" arrangement is that\n> it is done in the built directory, and because \"make install\" would\n> blindly install whatever in the built directory, if you truly care\n> that you install unstripped binaries, you need to see if they are\n> stripped and rebuild them as needed, because \"make strip\" may or may\n> not have been done.  From that point of view, getting rid of the\n> current \"make strip\" and introducing either \"make strip-installed\"\n> (\"we've installed things earlier---go strip them\") or \"make\n> install-stripped\" (\"we've built (or if we haven't please build them\n> first), now install them and strip them in the installed directory\")\n> may make more sense.  And for that, any idea that came up in this\n> discussion that relies on the current \"strip\" target would not help.\n\nBut often the installed directory (install prefix) is owned by root,\nso one has to `sudo make install-strip`, right?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"433523","messageId":"nycvar.QRO.7.76.6.2108241148560.55@tvgsbejvaqbjf.bet","threadId":"56332","inReplyTo":"6e0ef7f2-3dd0-09e4-5a1d-7e59b979d624@gmail.com","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-24T09:49:25Z","receivedAt":"2021-08-24T09:49:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Bagas,\n\nOn Tue, 24 Aug 2021, Bagas Sanjaya wrote:\n\n> On 23/08/21 22.55, Junio C Hamano wrote:\n> > Perhaps.  One bad thing with the current \"strip\" arrangement is that\n> > it is done in the built directory, and because \"make install\" would\n> > blindly install whatever in the built directory, if you truly care\n> > that you install unstripped binaries, you need to see if they are\n> > stripped and rebuild them as needed, because \"make strip\" may or may\n> > not have been done.  From that point of view, getting rid of the\n> > current \"make strip\" and introducing either \"make strip-installed\"\n> > (\"we've installed things earlier---go strip them\") or \"make\n> > install-stripped\" (\"we've built (or if we haven't please build them\n> > first), now install them and strip them in the installed directory\")\n> > may make more sense.  And for that, any idea that came up in this\n> > discussion that relies on the current \"strip\" target would not help.\n>\n> But often the installed directory (install prefix) is owned by root,\n> so one has to `sudo make install-strip`, right?\n\nThe default for `make install` goes to `$HOME`. I would wager a bet that\nthat's the common case, too. It does not require `sudo`.\n\nCiao,\nDscho\n"},{"id":"433609","messageId":"xmqqv93uoek8.fsf@gitster.g","threadId":"56332","inReplyTo":"6e0ef7f2-3dd0-09e4-5a1d-7e59b979d624@gmail.com","subject":"Re: [PATCH] make: add INSTALL_STRIP variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-24T18:58:15Z","receivedAt":"2021-08-24T18:58: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> On 23/08/21 22.55, Junio C Hamano wrote:\n>> Perhaps.  One bad thing with the current \"strip\" arrangement is that\n>> it is done in the built directory, and because \"make install\" would\n>> blindly install whatever in the built directory, if you truly care\n>> that you install unstripped binaries, you need to see if they are\n>> stripped and rebuild them as needed, because \"make strip\" may or may\n>> not have been done.  From that point of view, getting rid of the\n>> current \"make strip\" and introducing either \"make strip-installed\"\n>> (\"we've installed things earlier---go strip them\") or \"make\n>> install-stripped\" (\"we've built (or if we haven't please build them\n>> first), now install them and strip them in the installed directory\")\n>> may make more sense.  And for that, any idea that came up in this\n>> discussion that relies on the current \"strip\" target would not help.\n>\n> But often the installed directory (install prefix) is owned by root,\n> so one has to `sudo make install-strip`, right?\n\nWhatever you would need to do for \"make install\" would be necessary,\nbecause you would be touching the system directories, so there isn't\nanything new, I would think.\n"}]}