Message ID | 20210817110728.55842-1-bagasdotme@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | make: add install-strip target | expand |
Hi Bagas, On Tue, 17 Aug 2021, Bagas Sanjaya wrote: > diff --git a/Makefile b/Makefile > index 9573190f1d..8c4633ba8e 100644 > --- a/Makefile > +++ b/Makefile > @@ -3093,6 +3093,9 @@ endif > done && \ > ./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X" > > +install-strip: all strip Would those `all` and `strip` targets interfere with one another if `make -j2` was called? If not, wouldn't it be sufficient to let `install-strip` depend on `strip` alone? Ciao, Dscho > + $(MAKE) install > + > .PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf > .PHONY: quick-install-doc quick-install-man quick-install-html > install-gitweb: > @@ -3265,7 +3268,7 @@ ifdef MSVC > $(RM) compat/vcbuild/MSVC-DEFS-GEN > endif > > -.PHONY: all install profile-clean cocciclean clean strip > +.PHONY: all install install-strip profile-clean cocciclean clean strip > .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell > .PHONY: FORCE cscope > > > base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf > -- > 2.25.1 > >
On Tue, Aug 17, 2021 at 5:29 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > On Tue, 17 Aug 2021, Bagas Sanjaya wrote: > > diff --git a/Makefile b/Makefile > > @@ -3093,6 +3093,9 @@ endif > > +install-strip: all strip > > Would those `all` and `strip` targets interfere with one another if `make > -j2` was called? If not, wouldn't it be sufficient to let `install-strip` > depend on `strip` alone? A more pertinent question, perhaps, is why would we need `install-strip` at all? What benefit does it provide over simply typing `make strip install`?
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Would those `all` and `strip` targets interfere with one another if `make > -j2` was called? If not, wouldn't it be sufficient to let `install-strip` > depend on `strip` alone? Good question. I would have expected that this will *not* be a new target, but some sort of make variable (e.g. "make INSTALL_STRIP=yes install").
Hi Eric, On Tue, 17 Aug 2021, Eric Sunshine wrote: > On Tue, Aug 17, 2021 at 5:29 PM Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: > > On Tue, 17 Aug 2021, Bagas Sanjaya wrote: > > > diff --git a/Makefile b/Makefile > > > @@ -3093,6 +3093,9 @@ endif > > > +install-strip: all strip > > > > Would those `all` and `strip` targets interfere with one another if `make > > -j2` was called? If not, wouldn't it be sufficient to let `install-strip` > > depend on `strip` alone? > > A more pertinent question, perhaps, is why would we need > `install-strip` at all? What benefit does it provide over simply > typing `make strip install`? That would require an order-only prerequisite (see https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html) for `make -j2 strip install` to work correctly, i.e. something like this: -- snip -- diff --git a/Makefile b/Makefile index 2d5c822f7a8..9987f3b2c13 100644 --- a/Makefile +++ b/Makefile @@ -2990,7 +2990,7 @@ profile-install: profile profile-fast-install: profile-fast $(MAKE) install -install: all +install: all | strip $(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)' -- snap -- I am not quite certain that this is compatible with other `make` implementations we still might support (if there are any, I remember that we often have to rely on `gmake` because the native `make` does not understand our `Makefile`?), so that might need to be conditional on GNU Make. Ciao, Dscho
Hi Junio, On Tue, 17 Aug 2021, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > Would those `all` and `strip` targets interfere with one another if `make > > -j2` was called? If not, wouldn't it be sufficient to let `install-strip` > > depend on `strip` alone? > > Good question. > > I would have expected that this will *not* be a new target, but some > sort of make variable (e.g. "make INSTALL_STRIP=yes install"). That would work, too. At the same time: wouldn't it be nicer to let `make -j15 strip install` Do The Right Thing? Ciao, Dscho
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > That would work, too. At the same time: wouldn't it be nicer to let `make > -j15 strip install` Do The Right Thing? Oh, absolutely. With "make all install" and "make strip install", building (and optional stripping) should complete before the "install" target kicks in. It may be a bit tricky to implement, though. Making 'install' depend unconditionally on 'all' is trivial, but we want it to depend on 'strip' only when 'strip' is part of the targets requested.
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > -install: all > +install: all | strip > $(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)' > -- snap -- > > I am not quite certain that this is compatible with other `make` > implementations we still might support (if there are any, I remember that > we often have to rely on `gmake` because the native `make` does not > understand our `Makefile`?), so that might need to be conditional on GNU > Make. I think we are pretty-much dependent on GNU make already (it is possible to raise a weather balloon to confirm by renaming Makefile to GNUmakefile and observing if anybody complains, I think). But I am not sure what such a rule does for a .PHONY target like 'strip'. Does it do the right thing, i.e. "install recipe is run after 'strip' recipe has run, iff 'strip' is also asked for"? Thanks.
Hi Junio, On Thu, 19 Aug 2021, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > -install: all > > +install: all | strip > > $(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)' > > -- snap -- > > > > I am not quite certain that this is compatible with other `make` > > implementations we still might support (if there are any, I remember that > > we often have to rely on `gmake` because the native `make` does not > > understand our `Makefile`?), so that might need to be conditional on GNU > > Make. > > I think we are pretty-much dependent on GNU make already (it is > possible to raise a weather balloon to confirm by renaming Makefile > to GNUmakefile and observing if anybody complains, I think). > > But I am not sure what such a rule does for a .PHONY target like > 'strip'. Does it do the right thing, i.e. "install recipe is run > after 'strip' recipe has run, iff 'strip' is also asked for"? My reading of the documentation is that just as with regular dependencies, it does not matter whether order-only dependencies are .PHONY or not. The only difference between order-only vs regular dependencies seems to be that order-only dependencies are not necessarily built. But if they are, they are guaranteed to be built before the order-only dependencee. Granted, I did not have time to test it, but from an implementation point of view, I would be surprised if there was any more to it. Ciao, Dscho
diff --git a/INSTALL b/INSTALL index 66389ce059..6e6303d482 100644 --- a/INSTALL +++ b/INSTALL @@ -25,6 +25,11 @@ set up install paths (via config.mak.autogen), so you can write instead $ make all doc ;# as yourself # make install install-doc install-html;# as root +If you're tight on space (common on embedded systems), you can install +with debugging info stripped with + + # make install-strip + If you're willing to trade off (much) longer build time for a later faster git you can also do a profile feedback build with diff --git a/Makefile b/Makefile index 9573190f1d..8c4633ba8e 100644 --- a/Makefile +++ b/Makefile @@ -3093,6 +3093,9 @@ endif done && \ ./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X" +install-strip: all strip + $(MAKE) install + .PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf .PHONY: quick-install-doc quick-install-man quick-install-html install-gitweb: @@ -3265,7 +3268,7 @@ ifdef MSVC $(RM) compat/vcbuild/MSVC-DEFS-GEN endif -.PHONY: all install profile-clean cocciclean clean strip +.PHONY: all install install-strip profile-clean cocciclean clean strip .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell .PHONY: FORCE cscope
Previously to install Git with stripped binaries, users have to do `make all` then `make strip` before doing `make install`. It is nice to have `install-strip` target for convenience, so that they can simply type `make install-strip` and have Git with stripped binaries installed. On some environments where disk space and resources is limited (such as embedded systems), installed size can be smaller that with non-stripped binaries. Also mention the target in INSTALL. Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com> --- INSTALL | 5 +++++ Makefile | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf