From: Jonathan Nieder <jrnieder@gmail.com>
To: Fredrik Kuivinen <frekui@gmail.com>
Cc: David Aguilar <davvid@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org
Subject: Re: [PATCH v2] Makefile: Improve compiler header dependency check
Date: Mon, 29 Aug 2011 23:05:15 -0500 [thread overview]
Message-ID: <20110830040515.GC6647@elie.gateway.2wire.net> (raw)
In-Reply-To: <CALx8hKTx3r=ow+=jsCyvZGRJ6Yr+w9TT7=Uyi4y4+beOou45AA@mail.gmail.com>
Hi,
Fredrik Kuivinen wrote:
> On Sat, Aug 27, 2011 at 23:00, David Aguilar <davvid@gmail.com> wrote:
>> I'm not sure if "sh -c" is necessary but I did notice that other
>> parts of the Makefile use $(SHELL_PATH). The check was adjusted
>> to use that as well.
>
> I'm not sure either. I just used what I saw at other places in the Makefile.
It is not needed, and imho it makes it harder to read. I believe the
current uses of "sh -c" near the top of the Makefile are to emphasize
that a POSIX shell has not been determined yet (so POSIXy constructs
cannot be used at that point on platforms like Solaris).
Aside from that, this seems good, though. While at it, the log
message could be simplified to something closer to the original
version:
The Makefile enables CHECK_HEADER_DEPENDENCIES when the
compiler supports generating header dependencies.
Make the check use the same flags as the invocation
to avoid a false positive when user-configured compiler
flags contain incompatible options.
For example, without this patch, trying to build universal
binaries on a Mac using CFLAGS='-arch i386 -arch x86_64'
produces
gcc-4.2: -E, -S, -save-temps and -M options are
not allowed with multiple -arch flags
While at it, remove "sh -c" in the command passed to $(shell);
at this point in the Makefile, SHELL has already been set to
a sensible shell and it is better not to override that.
Thanks again and sorry for the fuss.
Cheers,
Jonathan
next prev parent reply other threads:[~2011-08-30 4:05 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-27 8:41 [PATCH] Makefile: Improve compiler header dependency check David Aguilar
2011-08-27 16:26 ` Jonathan Nieder
2011-08-27 21:00 ` [PATCH v2] " David Aguilar
2011-08-28 11:47 ` Fredrik Kuivinen
2011-08-30 4:05 ` Jonathan Nieder [this message]
2011-08-30 8:27 ` [PATCH v3] " David Aguilar
2011-08-30 8:33 ` Jonathan Nieder
2011-08-30 17:17 ` Junio C Hamano
2011-08-27 21:19 ` [PATCH] " David Aguilar
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=20110830040515.GC6647@elie.gateway.2wire.net \
--to=jrnieder@gmail.com \
--cc=davvid@gmail.com \
--cc=frekui@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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).