[coreboot] [patch] Cleanups in Makefile for flashrom and superiotool
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Sun May 18 15:46:18 CEST 2008
On 18.05.2008 15:21, Peter Lemenkov wrote:
> 2008/5/18 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>:
>
>
>>>> I can understand setting PREFIX, but the reason why you need to redefine
>>>> CFLAGS is a mystery to me.
>>>>
>
>
>>> We use our custom CFLAGS.
>>>
>
>
>>> -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
>>> -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386
>>> -mtune=generic -fasynchronous-unwind-tables
>>>
>
>
>> And these custom CFLAGS are missing -DDISABLE_DOC in the flashrom case
>> (harmful). Furthermore, they are missing $(SVNDEF) for superiotool
>> (wrong, but only a nuisance).
>>
>
> No.
>
> CFLAGS='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
> -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386
> -mtune=generic -fasynchronous-unwind-tables
> -D'\''FLASHROM_VERSION="3332"'\'' -DDISABLE_DOC'
>
> We provide them in form of CFLAGC=%some_predefined_part%
> -D'\''FLASHROM_VERSION="3332"'\'' -DDISABLE_DOC'
>
> So we do define FLASHROM_VERSION and DISABLE_DOC.
>
OK, you may do that, but what happens if someone defines CFLAGS='-O2'
and leaves it at that? With the patch you proposed, -DDISABLE_DOC would
be missing then.
> But, as I mentioned above, the best way to do so is to define then in
> a conditional way.
>
-DDISABLE_DOC must be specified regardless of the CFLAGS passed in. I'm
sure there are ways to do that. Having $SVNDEF in the unconditional
CFLAGS is desirable as well. You could make SVNDEF a conditional define
to solve problems with that.
>>>> Will the modified install section work with a non-FSF install program as
>>>> well, i.e. is it truly cross-platform?
>>>>
>
>
>>> No idea.
>>>
>
>
>> Sorry, then that part is a no-go. We are currently preparing flashrom
>> for FreeBSD compatibility and I don't want to ruin that.
>>
>
> OK, understood and agree with this.
>
> Apart from install-section other things (trick with conditional defines) are OK?
>
If you can solve the conditional define problem as outlined above, I
think the patch can be Acked. Makefile stuff is very brittle and we had
several accidents in the past, so getting 2 Acks is a minimum.
Regards,
Carl-Daniel
More information about the coreboot
mailing list