build optimization

[Ronald Hoogenboom]

Hi,

I've attached a patch that removes the 3-mile-long compiler
commandlines, which vim's quickfix doesn't like so much. Instead of
putting all those -DXYZ='bla' on the compiler commandline, they are put
in a file called settings.h (as #define XYZ bla) and only a
--include=settings.h is put on the commandline.
This file is created unconditionally at the same time as when the
CPUFLAGS simply expanded make variable used to be created (not via a
target rule and dependency), so it shouldn't change anything.

Signed-off-by: Ronald Hoogenboom

[Corey Osgood]

I think you're missing part of the patch, there should be a settings.h
included, right?

-Corey

2009/4/1 Ronald Hoogenboom
> Hi,
>
> I've attached a patch that removes the 3-mile-long compiler
> commandlines, which vim's quickfix doesn't like so much. Instead of
> putting all those -DXYZ='bla' on the compiler commandline, they are put
> in a file called settings.h (as #define XYZ bla) and only a
> --include=settings.h is put on the commandline.
> This file is created unconditionally at the same time as when the
> CPUFLAGS simply expanded make variable used to be created (not via a
> target rule and dependency), so it shouldn't change anything.
>
> Signed-off-by: Ronald Hoogenboom

[ron minnich]

On Wed, Apr 1, 2009 at 4:42 PM, Corey Osgood  wrote:
> I think you're missing part of the patch, there should be a settings.h
> included, right?

no,that is generated by the >> in the patch.

I kind of like this. But, if the python is run twice, do you get the
file twice as large? There's no "zero out the file" step from what i
can see.

ron

[Ward Vandewege]

On Wed, Apr 01, 2009 at 04:45:44PM -0700, ron minnich wrote:
> On Wed, Apr 1, 2009 at 4:42 PM, Corey Osgood  wrote:
> > I think you're missing part of the patch, there should be a settings.h
> > included, right?
>
> no,that is generated by the >> in the patch.
>
> I kind of like this. But, if the python is run twice, do you get the
> file twice as large? There's no "zero out the file" step from what i
> can see.

Hmm. Seems like replacing the >> by > should fix that.

Thanks,
Ward.

[ron minnich]

On Wed, Apr 1, 2009 at 5:07 PM, Ward Vandewege  wrote:
> On Wed, Apr 01, 2009 at 04:45:44PM -0700, ron minnich wrote:
> > On Wed, Apr 1, 2009 at 4:42 PM, Corey Osgood  wrote:
> > > I think you're missing part of the patch, there should be a settings.h
> > > included, right?
> >
> > no,that is generated by the >> in the patch.
> >
> > I kind of like this. But, if the python is run twice, do you get the
> > file twice as large? There's no "zero out the file" step from what i
> > can see.
>
> Hmm. Seems like replacing the >> by > should fix that.

Seems that way to me, but it's worth testing :-)

ron

[Russell Whitaker]

On Wed, 1 Apr 2009, Ward Vandewege wrote:
> On Wed, Apr 01, 2009 at 04:45:44PM -0700, ron minnich wrote:
> > On Wed, Apr 1, 2009 at 4:42 PM, Corey Osgood  wrote:
> > > I think you're missing part of the patch, there should be a settings.h
> > > included, right?
> >
> > no,that is generated by the >> in the patch.
> >
> > I kind of like this. But, if the python is run twice, do you get the
> > file twice as large? There's no "zero out the file" step from what i
> > can see.
>
> Hmm. Seems like replacing the >> by > should fix that.

I suspect the patch should look something like:

+	file.write('/* autogenerated */' > settings.h)\n")
+	file.write("D_item = $(shell echo '$(if $(subst undefined,,$(origin $1)),\#define $1$(if $($1), $($1),),\#undef $1)' >> settings.h)\n\n")
+	file.write("CPUFLAGS := $(strip $(foreach _var_,$(VARIABLES),$(call D_item,$(_var_)))--include=settings.h)\n\n")

Now the first line forces a new copy and (if I didn't goof it up) the 2nd
and 3rd lines can add multiple lines to settings.h

Perhaps the nest step would be to combine the 2nd & 3rd lines.

Russ

[Ronald Hoogenboom]

On Wed, 2009-04-01 at 18:56 -0700, Russell Whitaker wrote:
> On Wed, 1 Apr 2009, Ward Vandewege wrote:
> > On Wed, Apr 01, 2009 at 04:45:44PM -0700, ron minnich wrote:
> > > On Wed, Apr 1, 2009 at 4:42 PM, Corey Osgood  wrote:
> > > > I think you're missing part of the patch, there should be a settings.h
> > > > included, right?
> > >
> > > no,that is generated by the >> in the patch.
> > >
> > > I kind of like this. But, if the python is run twice, do you get the
> > > file twice as large? There's no "zero out the file" step from what i
> > > can see.
> >
> > Hmm. Seems like replacing the >> by > should fix that.
>
> I suspect the patch should look something like:
> file.write('/* autogenerated */' > settings.h)\n")
> file.write("D_item = $(shell echo '$(if $(subst undefined,,$(origin $1)),\#define $1$(if $($1), $($1),),\#undef $1)' >> settings.h)\n\n")
> file.write("CPUFLAGS := $(strip $(foreach _var_,$(VARIABLES),$(call D_item,$(_var_)))--include=settings.h)\n\n")
>
> Now the first line forces a new copy and (if I didn't goof it up) the 2nd
> and 3rd lines can add multiple lines to settings.h
>
> Perhaps the nest step would be to combine the 2nd & 3rd lines.
> Russ

Hi guys,

The original patch has all of that already. Please test it before
complaining....

The settings.h is generated from scratch each time the Makefile is read,
because each time the CPUFLAGS will be set to its value (it's a simply
expanded variable!, see 'info make'). The first $(shell echo...) will
truncate the file (or create an new one) and put the 'autogenerated'
line in. The subsequent echo's in the D_item macro will append #define
lines to that file. These $(shell ) functions will only contribute
whitespace to the CPUFLAGS variable, because of the redirection. The
whitespace is later stripped off by the strip function.

Ronald.

[ron minnich]

On Thu, Apr 2, 2009 at 1:48 PM, Ronald Hoogenboom  wrote:
> The original patch has all of that already. Please test it before
> complaining....

yeah, I missed that, was not complaining, was asking.

Acked-by: Ronald G. Minnich

[Ronald Hoogenboom]

On Thu, 2009-04-02 at 14:24 -0700, ron minnich wrote:
> yeah, I missed that, was not complaining, was asking.

Ok, sorry for responding so slow. And I didn't expect so many responses
for such a small change...
> Acked-by: Ronald G. Minnich

Thanks.
Ronald.

[Carl-Daniel Hailfinger]

Hi Ronald,

On 02.04.2009 01:16, Ronald Hoogenboom wrote:
> I've attached a patch that removes the 3-mile-long compiler
> commandlines, which vim's quickfix doesn't like so much. Instead of
> putting all those -DXYZ='bla' on the compiler commandline, they are put
> in a file called settings.h (as #define XYZ bla) and only a
> --include=settings.h is put on the commandline.
> This file is created unconditionally at the same time as when the
> CPUFLAGS simply expanded make variable used to be created (not via a
> target rule and dependency), so it shouldn't change anything.
>
> Signed-off-by: Ronald Hoogenboom

This is definitely a great idea! Unfortunately, I don't understand the
v2 build system well enough to ack this patch.

Regards,
Carl-Daniel