Re: [PATCH 2/9] makefile and run_tests tweaks

[Andrew Jones <drjones@redhat.com>]

On Sat, Dec 28, 2013 at 10:30:03PM -0800, Christoffer Dall wrote:
> On Wed, Dec 04, 2013 at 05:42:50PM +0100, Andrew Jones wrote:
> 
> This is a really short commit message, for anyone not super-familiar
> with the make system in this toolset, it makes it quite hard to
> understand the change.  See some examples below:
> 
> > - remove a redundant '-display none'
> > - remove a redundant -g from CFLAGS
> > - remove a useless -I../include/x86 from CFLAGS
> 
> why is this include useless?

The directory doesn't exist.

> 
> > - remove lib autodep files on make clean
> 
> why?

There should be a way to get rid of them. Maybe add a 'realclean'
target? I think that would be overkill though, as 'make clean'
isn't something I expect people to run all that frequently anyway.

> 
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  Makefile              |  8 ++++----
> >  config-x86-common.mak | 16 +++++++---------
> >  run_tests.sh          |  2 +-
> >  3 files changed, 12 insertions(+), 14 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 278791dbbef23..697fc2a766966 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -12,9 +12,9 @@ libgcc := $(shell $(CC) --print-libgcc-file-name)
> >  
> >  libcflat := lib/libcflat.a
> >  cflatobjs := \
> > +	lib/argv.o \
> >  	lib/printf.o \
> >  	lib/string.o
> > -cflatobjs += lib/argv.o
> >  
> >  #include architecure specific make rules
> >  include config-$(ARCH).mak
> > @@ -25,8 +25,8 @@ include config-$(ARCH).mak
> >  cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
> >                > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> >  
> > -CFLAGS += -O1
> 
> seems like a good idea to compile test code without optimizations, is
> that what you're trying to achieve here?  Seems to me this should be a
> separate change or at least motivated in the commit text.

No, I keep the -O1, but it gets moved to an x86 specific file in order
to give each arch their own choice.

> 
> > -CFLAGS += $(autodepend-flags) -g -fomit-frame-pointer -Wall
> 
> I like removing the redundant -g but wouldn't it be cleaner to set the
> CFLAGS to an empty string in the beginning of the file and then have a
> seprate line for -g where all the CFLAGS are set?

OK, done for v3.

drew