[1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally

As the $(HOST_DIR) is the buildroot's system install directory, and that
all the builds should use that one in priority (it's even mandatory for
uboot and linux), it make sense to use '-isystem' to ensure it is searched
right before the system directories and still let the possibility for the
packages to use specific ones using '-I'.

 Now, changing '-I' into '-isystem' carries the risk that a package will add
'-I/usr/include' to CFLAGS and thus give priority to system headers over
host headers. However, this turns out not to be the case, because gcc will
remove the '-I' options that add system includes:

  $ gcc -v -c -isystem $PWD/output/host/include -I/usr/include \
    support/kconfig/expr.c
  ...
  ignoring duplicate directory "/usr/include" as it is a non-system \
    directory that duplicates a system directory
  #include "..." search starts here:
  #include <...> search starts here:
   {output}/host/include
   /usr/lib/gcc/x86_64-linux-gnu/8/include
   /usr/local/include
   /usr/lib/gcc/x86_64-linux-gnu/8/include-fixed
   /usr/include/x86_64-linux-gnu
   /usr/include
  End of search list.
  ...

Signed-off-by: David De Grave (Essensium/Mind) <david.degrave@mind.be>
---
 package/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hello David,

On Fri, 31 Aug 2018 16:20:22 +0200, David De Grave (Essensium/Mind)
wrote:
>  As the $(HOST_DIR) is the buildroot's system install directory, and that
> all the builds should use that one in priority (it's even mandatory for
> uboot and linux), it make sense to use '-isystem' to ensure it is searched
> right before the system directories and still let the possibility for the
> packages to use specific ones using '-I'.

Could you give a few more details about the motivation for this change.
Is it just because it is "cleaner" to refer to Buildroot
$(HOST_DIR)/include using -isystem, or does this fix some actual
problem/build issue ?

Doing such a change is pretty invasive, it could potentially cause
some build failures, so I'd like to understand the motivation.

Thanks!

Thomas

On 09/09/2018 15:32, Thomas Petazzoni wrote:
> Hello David,
> 
> On Fri, 31 Aug 2018 16:20:22 +0200, David De Grave (Essensium/Mind)
> wrote:
>>  As the $(HOST_DIR) is the buildroot's system install directory, and that
>> all the builds should use that one in priority (it's even mandatory for
>> uboot and linux), it make sense to use '-isystem' to ensure it is searched
>> right before the system directories and still let the possibility for the
>> packages to use specific ones using '-I'.
> 
> Could you give a few more details about the motivation for this change.
> Is it just because it is "cleaner" to refer to Buildroot
> $(HOST_DIR)/include using -isystem, or does this fix some actual
> problem/build issue ?

 It fixes the existing problem for u-boot and linux in a more generic way. Also,
the same issue might exist for any host package that we have. The exported
headers are often in an 'include' directory that is accessed with a -I flag. If
that -I flag is added after the CFLAGS passed in by us, then the compiler will
first find the host-installed headers rather than the local headers.
Autobuilders will usually not find such an issue, because they have a rather
minimal set of devel packages installed on the system.

 It's true that these issues are rare: normally the local -I flags are added
*before* the CFLAGS passed in by the user. However, I'm pretty sure that
packages do exist that don't do this (e.g. if the Makefile just does 'CFLAGS +=
...'). It's pretty hard to detect such cases, because *usually* it doesn't give
an error (because the system headers are probably compatible with the local
headers).

 Regards,
 Arnout

> 
> Doing such a change is pretty invasive, it could potentially cause
> some build failures, so I'd like to understand the motivation.
> 
> Thanks!
> 
> Thomas
>

Hello Thomas, Arnout,

On Mon, Sep 10, 2018 at 11:51 AM, Arnout Vandecappelle <arnout@mind.be>
wrote:


> > Could you give a few more details about the motivation for this change.
> > Is it just because it is "cleaner" to refer to Buildroot
> > $(HOST_DIR)/include using -isystem, or does this fix some actual
> > problem/build issue ?
>

More than making things cleaner/logic, it fixes a build issue I've with the
beagle bone
black's Linux kernel: https://github.com/beagleboard/linux.git

They implemented a driver who handle DTBs overlays and what they call
"capes"
(PCB extensions to the bbb mother board). Since the update of the DTC
package few
weeks ago, my build is throwing compilation errors in linux when it starts
to compile
their own version of the dtc. Due to the way they include their headers,
some of them
were taken from the host while some others are taken from local dir.
Moreover, their
dtc version use different names in their defines (a missing underscore) and
this why
at a certain point, it throws a "function redefinition error". Maybe other
kernels are
suffering from the same problem, I don't know...

I first fixed the issue by adding a patch to my project to change the
involved sources
in the Kernel tree...

But then I thought that other people may suffers from this problem too and
decided to
make it more generic by patching buildroot itself, so that I could save
them few hours
of investigations. This is why I submitted my first patch who applied the
same kind of
change in the linux.mk file.

Then, I discussed this with Arnout and we thought it would be even more
benefit to
make it global as it's already a requirement for uboot and now linux too.
Moreover it
sounds more logic/cleaner to make it that way, but that was not the main
idea.

So, that's why I submitted this patch series.

> Doing such a change is pretty invasive, it could potentially cause
> > some build failures, so I'd like to understand the motivation.
>

I understand, no problem :-)

Regards,
David.

David, All,

On 2018-09-10 15:00 +0200, David De Grave spake thusly:
> Hello Thomas, Arnout,
> On Mon, Sep 10, 2018 at 11:51 AM, Arnout Vandecappelle < [1]arnout@mind.be> wrote:
>  
> 
>   > Could you give a few more details about the motivation for this change.
>   > Is it just because it is "cleaner" to refer to Buildroot
>   > $(HOST_DIR)/include using -isystem, or does this fix some actual
>   > problem/build issue ?
> 
> More than making things cleaner/logic, it fixes a build issue I've with the beagle bone
> black's Linux kernel: [2]https://github.com/beagleboard/linux.git
> They implemented a driver who handle DTBs overlays and what they call "capes"
> (PCB extensions to the bbb mother board). Since the update of the DTC package few
> weeks ago, my build is throwing compilation errors in linux when it starts to compile
> their own version of the dtc. Due to the way they include their headers, some of them
> were taken from the host while some others are taken from local dir. Moreover, their
> dtc version use different names in their defines (a missing underscore) and this why
> at a certain point, it throws a "function redefinition error". Maybe other kernels are
> suffering from the same problem, I don't know...
> I first fixed the issue by adding a patch to my project to change the involved sources
> in the Kernel tree...
> But then I thought that other people may suffers from this problem too and decided to
> make it more generic by patching buildroot itself, so that I could save them few hours
> of investigations. This is why I submitted my first patch who applied the same kind of
> change in the [3]linux.mk file.
> Then, I discussed this with Arnout and we thought it would be even more benefit to
> make it global as it's already a requirement for uboot and now linux too. Moreover it
> sounds more logic/cleaner to make it that way, but that was not the main idea.
> So, that's why I submitted this patch series.

The -isystem has already been tried, and lately resurfaced in another
thread, in which it was noticed that it broke at least a few packages,
please see:

    http://lists.busybox.net/pipermail/buildroot/2018-October/232615.html

So, before we reinstate the use of -isystem, a few packages will have to
be fixed first.

Thanks!

Regards,
Yann E. MORIN.

>   > Doing such a change is pretty invasive, it could potentially cause
>   > some build failures, so I'd like to understand the motivation.
> 
> I understand, no problem :-)
> Regards,
> David.
> --
> 
> +--------------------------------------------------------------------------------------------------------+
> |        | David De Grave                                                                                |
> |   | Senior Embedded Software Developer                                                            |
> |        | Gsm : +32(0)496.364.960 | Tel : +32-16-28.65.00 | Fax : +32-16-28a.65.01  |
> |        | [4]Essensium-Mind - Gaston Geenslaan 9, B-3001 Leuven, Belgium                                |
> +--------------------------------------------------------------------------------------------------------+
> 
> Links:
> 1. mailto:arnout@mind.be
> 2. https://github.com/beagleboard/linux.git
> 3. http://linux.mk
> 4. https://www.mind.be/

> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot