Re: [PATCH v2] Makefile: Improve compiler header dependency check

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 30, 2011, 04:05 UTC
Message-ID: <20110830040515.GC6647@elie.gateway.2wire.net>
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:

Show 5 quoted lines

>> 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