On Wed, Jul 15, 2009 at 9:37 AM, Mike Frysinger<[email protected]> wrote:
> On Sunday 12 July 2009 01:49:52 Garrett Cooper wrote:
>> -AC_CONFIG_FILES([config.mk m4/Makefile])
>> +AC_CONFIG_FILES([ include/mk/config.mk lib/ltp.pc ltp-devel.spec
>> m4/Makefile \ +README.ltp-devel ])
>
> if you're going to start splitting lines, might as well do all of them and
> keep them sorted
> AC_CONFIG_FILES([ \
>        file1 \
>        file2 \
>        file3 \
> ])

Done.

>> --- /dev/null 1 Jan 1970 00:00:00 -0000
>> +++ ltp-devel.spec.in 12 Jul 2009 05:46:28 -0000
>
> each of the .in files should have a target in the related Makefile so that it
> gets updated if the source is modified
>
> ltp-devel.spec: $(srcdir)/ltp-devel.spec.in
>        ./config.status $@

Ok.

>> +++ include/mk/config.mk.in   12 Jul 2009 05:46:28 -0000
>> +AIO_LIBS     = @AIO_LIBS@
>> +CRYPTO_LIB   = @CRYPTO_LIB@
>> +SELINUX_LIBS = @SELINUX_LIBS@
>
> there should be a note here about standardizing the naming.  it should be
> "FOO_LIBS", not "_LIB".

Ok. I'll fix that then, even though I didn't cause it.

>> +# Avoid overwriting definitions in leaf callers.
>> +CPPFLAGS     ?= @CPPFLAGS@ -I$(includedir)
>> +CFLAGS               ?= @CFLAGS@
>> +LDLIBS               ?= @LIBS@
>> +LDFLAGS              ?= @LDFLAGS@ -L$(libdir)
>
> this looks like it'll cause problems if you set these in your environment
> export CPPFLAGS='' CFLAGS='-pipe' LDFLAGS=''
> make
>
> i would use $(origin ...) to determine whether the variables are coming from
> the leaf Makefile

Ah, yes. Excellent point.

So what's the appropriate protocol then:

If set in environment, ignore?
Or:
If set in Makefile, ignore?

> also, please add '-pipe' to default CFLAGS

Sure.

>> +DEBUG_CFLAGS ?= -g
>> +DEBUG_CXXFLAGS       ?= $(DEBUG_CFLAGS)
>> +
>> +CFLAGS               += $(DEBUG_CFLAGS)
>> +CXXFLAGS     += $(DEBUG_CXXFLAGS)
>> +
>> +# There have been several bugs in the past related to -O2+ when
>> +# -fno-strict-aliasing is not specified.
>> +OPT_CFLAGS   ?= -O2 -fno-strict-aliasing
>> +OPT_CXXFLAGS ?= $(OPT_CFLAGS)
>
> in general, i recall only like 1 bug that was a gcc issue.  all the others
> were broken source code.  i dont think we should live with code that violates
> aliasing rules.

See my previous comment. There are ~10 different bugs spanning
compilers from v3.3.x to 4.4.x, which deal with broken
-fstrict-aliasing optimizations in the tree optimizer.

>> +#export AR CC CFLAGS CPPFLAGS LDLIBS LDFLAGS RANLIB
>
> forgot to delete ?

Yeah ><.

>> --- m4/GNUmakefile    18 Jan 2009 22:22:40 -0000      1.1
>> +++ m4/GNUmakefile    12 Jul 2009 05:46:28 -0000
>
> i think we only need a top level GNUmakefile.  all the other subdirs can stay
> Makefile.

Well, ok. I was just leaving it as-is, because that's the way it is today :).

Like in rock-paper-scissors:

GNUmakefile beats Makefile, which beats makefile.

>> +LFLAGS                       += -l -w
>> +CFLAGS                       += -w
>
> punt these

Will do.

Thanks!
-Garrett

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time, 
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize  
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to