Re: [PATCH] make: add INSTALL_STRIP variable

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 20, 2021, 18:16 UTC
Message-ID: <xmqqwnogt20q.fsf@gitster.g>
In-Reply-To: <YR+Tp2AGeeKyRKoC@danh.dev>

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

Show 20 quoted lines

>>  install: all
>> +ifdef INSTALL_STRIP
>> +	$(MAKE) strip
>> +endif
>
> I believe it's better to write like this:
>
> ----- 8< ------
> ifdef INSTALL_STRIP
> install: strip
> endif
>
> install: all
> 	....
> ---- >8-------
>
> IOW, install depends on strip, not install invoke strip.
> I think it would work better for:
>
> 	make install strip

I think you meant "it would work better than 'make install strip'", and if so, I tend to agree. With

	echo INSTALL_STRIP=YesPlease >>config.mak

either Bagas's or your "before installing, make sure we strip" change lets

	make install

just work without "strip" given on the command line.

If users with such a config.mak type "make install strip", it will make the recipe for "install" wait until "strip" is done, which is what we want, but "strip" on the command line for them is redundant, and there is no way for them to install unstripped binaries, which may be a bit of downside.

But for those who do not always want to use INSTALL_STRIP, as Dscho said after I mentioned the "make variable" thing, we probably a wrong thing when they say "make -j strip install", as there is nothing to make recipe for "install" to wait for "strip", so it is not a fully satisfactory solution.

I think we want two things:

 (1) if a user says "make [-j] strip install", make sure "install"
     won't start before "strip" finishes;

 (2) if a user wants to always install stripped binary, allow some
     make variable in config.mak so that "make install" would do
     that without an explicit "strip".

Of course, if a user does not have (2) configured, "make install" should install unstripped binaries, but that goes without saying.

And after thinking it like this, perhaps a new "install-stripped" target that runs "strip" and then "install" as originally proposed in the thread that triggered this discussion may be the simplest approach. We can control the optional dependency between "strip" and "install", those who want to install stripped binary can use "install-stripped" instead of "install", and they can on-demand choose to install unstripped binary (which was a potential downside of the "make variable" approach under discussion here).

Thanks.