On 18.05.2008 15:21, Peter Lemenkov wrote:
2008/5/18 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@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