Patchwork Broken include paths

There are a couple of boards that don't compile for me with crossgcc.
Here's a representative error:

src/mainboard/asus/a8v-e_se/mptable.c:23:54: error:
src/include/../../../southbridge/via/vt8237r/vt8237r.h: Permission
denied

The problem is that the compiler is looking for the file
../../../southbridge...  starting from src/include, since it was
specified with <>

There are many ways to fix it, but I'm not sure which one is the
correct (most future-proof) way.

Here are three of the possible fixes:

1. Use "" so the path is relative to the file.


They all compile for me.

Thanks,
Myles

Myles Watson wrote:
> There are many ways to fix it, but I'm not sure which one is the
> correct (most future-proof) way.
..
> 3. Just use the path

I think this is *by far* the cleanest approach!


//Peter

On Mon, Jul 26, 2010 at 11:09 AM, Peter Stuge <peter@stuge.se> wrote:
> Myles Watson wrote:
>> There are many ways to fix it, but I'm not sure which one is the
>> correct (most future-proof) way.
> ..
>> 3. Just use the path
>
> I think this is *by far* the cleanest approach!

I agree that it looks the best.  I'm worried that it introduces ambiguity.

#include <path/file.h>

Could look in src/path/file.h or src/include/path/file.h and others

Is that what we want?  Should we remove -I$(src) from the command line
in the long term?

from src/arch/i386/Makefile.bootblock.inc:	

$(CC) -MMD -x assembler-with-cpp -DASSEMBLY -E -I$(src)/include
-I$(src)/arch/i386/include -I$(obj) -I$(obj)/bootblock -include
$(obj)/config.h -I. -I$(src) $< -o $@

It seems like it could be simpler.  I also don't understand the order.

Thanks,
Myles

Myles Watson wrote:
> >> 3. Just use the path
> >
> > I think this is *by far* the cleanest approach!
> 
> I agree that it looks the best.  I'm worried that it introduces
> ambiguity.
> 
> #include <path/file.h>
> 
> Could look in src/path/file.h or src/include/path/file.h and others
> 
> Is that what we want?  Should we remove -I$(src) from the command line
> in the long term?

I'm not sure that I feel good about .h files outside include/ being
referenced from other parts of the code. They should probably be
moved to include/ if they are needed in more than one place..


> from src/arch/i386/Makefile.bootblock.inc:	
> 
> $(CC) -MMD -x assembler-with-cpp -DASSEMBLY -E -I$(src)/include
> -I$(src)/arch/i386/include -I$(obj) -I$(obj)/bootblock -include
> $(obj)/config.h -I. -I$(src) $< -o $@
> 
> It seems like it could be simpler.

I think simplifying the codebase will be a continuous effort.


> I also don't understand the order.

Me neither. And again, why are there include files in
src/arch/i386/include instead of include/arch-i386 or something?


//Peter

Am 26.07.2010 20:01, schrieb Peter Stuge:
> I'm not sure that I feel good about .h files outside include/ being
> referenced from other parts of the code. They should probably be
> moved to include/ if they are needed in more than one place..
Mainboards need to include chipset specific information, and I'd like to
avoid moving all that into include/ if possible.

>> I also don't understand the order.
> Me neither. And again, why are there include files in
> src/arch/i386/include instead of include/arch-i386 or something?
It was partially inherited from newconfig, partially "what works". And I
guess that was how it was handled throughout newconfig's lifecycle, too.


Patrick

On Mon, Jul 26, 2010 at 12:57 PM, Patrick Georgi <patrick@georgi-clan.de> wrote:
> Am 26.07.2010 20:01, schrieb Peter Stuge:
>> I'm not sure that I feel good about .h files outside include/ being
>> referenced from other parts of the code. They should probably be
>> moved to include/ if they are needed in more than one place..
> Mainboards need to include chipset specific information, and I'd like to
> avoid moving all that into include/ if possible.

I think that's a reasonable exception to the rule.  Some of that code
could move to the chipsets, but not all of it.

Thanks,
Myles

Patrick Georgi wrote:
> > .h files outside include/
> > They should probably be moved to include/
> 
> Mainboards need to include chipset specific information, and I'd
> like to avoid moving all that into include/ if possible.

Why not do it?


Myles Watson wrote:
> I think that's a reasonable exception to the rule.

Why have an exception at all?


> Some of that code could move to the chipsets, but not all of it.

Hmm, can you clarify?


//Peter

On Mon, Jul 26, 2010 at 1:22 PM, Peter Stuge <peter@stuge.se> wrote:
> Patrick Georgi wrote:
>> > .h files outside include/
>> > They should probably be moved to include/
>>
>> Mainboards need to include chipset specific information, and I'd
>> like to avoid moving all that into include/ if possible.
>
> Why not do it?
It's used in multiple places, but only two or three usually.


> Myles Watson wrote:
>> I think that's a reasonable exception to the rule.
>
> Why have an exception at all?
So src/include/ doesn't get cluttered.

>> Some of that code could move to the chipsets, but not all of it.
>
> Hmm, can you clarify?
I was thinking of some of the ACPI code, that is not
mainboard-dependent but chipset-dependent.  That's been slowly moving
to the chipset directories.

Some of the mainboard initialization code calls functions depending on
what's on the board.  That won't be moved.

Thanks,
Myles

On 7/26/10 9:28 PM, Myles Watson wrote:
> I was thinking of some of the ACPI code, that is not
> mainboard-dependent but chipset-dependent.  That's been slowly moving
> to the chipset directories.
>
If we can get rid of exceptions by cleaning more code up in this way we
should certainly do it.

Stefan