From: Junio C Hamano <gitster@pobox.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>,
git@vger.kernel.org, Emily Shaffer <emilyshaffer@google.com>,
Eric Sunshine <sunshine@sunshineco.com>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] make: add INSTALL_STRIP variable
Date: Fri, 20 Aug 2021 11:16:37 -0700 [thread overview]
Message-ID: <xmqqwnogt20q.fsf@gitster.g> (raw)
In-Reply-To: <YR+Tp2AGeeKyRKoC@danh.dev> ("Đoàn Trần Công Danh"'s message of "Fri, 20 Aug 2021 18:36:07 +0700")
Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
>> 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.
next prev parent reply other threads:[~2021-08-20 18:16 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-20 10:50 [PATCH] make: add INSTALL_STRIP variable Bagas Sanjaya
2021-08-20 11:36 ` Đoàn Trần Công Danh
2021-08-20 18:16 ` Junio C Hamano [this message]
2021-08-21 2:13 ` Đoàn Trần Công Danh
2021-08-23 15:55 ` Junio C Hamano
2021-08-24 9:39 ` Bagas Sanjaya
2021-08-24 9:49 ` Johannes Schindelin
2021-08-24 18:58 ` Junio C Hamano
2021-08-21 8:24 ` Bagas Sanjaya
2021-08-21 10:35 ` Đoàn Trần Công Danh
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=xmqqwnogt20q.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=bagasdotme@gmail.com \
--cc=congdanhqx@gmail.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--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 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).