[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[David Raeman]

HOST_CFLAGS includes a search path for HOST_DIR/usr/include using -I.  When
HOST_CFLAGS is used by a package, these flags are passed to the compiler ahead
of flags passed by the package's internal make system.  If a package has a
header file with the same name as a header file in HOST_DIR, this causes the
toolchain to prefer the file from the system include directory because its -I
appears first on the command line.  I believe conflicts should prefer the file
provided by the package.  This can be accomplished by using -isystem, which is
more appropriate then -I for system-level include paths.

Real-world example: I need libfdt present in my HOST_DIR to install a patched
version of QEMU.  Meanwhile, the u-boot package provides its own copy of
libfdt.h that is modified from upstream.  If I have libfdt installed into
HOST_DIR, then host-uboot-tools fails to build because it grabs the libfdt.h
from the HOST_DIR area instead of using the patched version from its own
source tree.  This patch corrects this issue.

This assumes the -isystem flag is supported by the host compiler.

Signed-off-by: David Raeman <draeman@bbn.com>
---
 package/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index afd5d3a..b0ef706 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -222,7 +222,7 @@ UNZIP := $(shell which unzip || type -p unzip) -q
 
 APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
 
-HOST_CPPFLAGS  = -I$(HOST_DIR)/usr/include
+HOST_CPPFLAGS  = -isystem $(HOST_DIR)/usr/include
 HOST_CFLAGS   ?= -O2
 HOST_CFLAGS   += $(HOST_CPPFLAGS)
 HOST_CXXFLAGS += $(HOST_CFLAGS)
-- 
2.7.4

[Buildroot] [PATCH 2/2] host-dtc: Install libftd and associated header files.

[David Raeman]

The dtc package currently does not install libfdt for the host install.
It can be useful to have libfdt on the host, such as for building QEMU
with the --enable-fdt configure switch.

Signed-off-by: David Raeman <draeman@bbn.com>
---
 package/dtc/dtc.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/package/dtc/dtc.mk b/package/dtc/dtc.mk
index 8fdcdbe..febd592 100644
--- a/package/dtc/dtc.mk
+++ b/package/dtc/dtc.mk
@@ -51,7 +51,9 @@ define HOST_DTC_BUILD_CMDS
 endef
 
 define HOST_DTC_INSTALL_CMDS
-	$(MAKE) -C $(@D) PREFIX=$(HOST_DIR)/usr install-bin
+	$(MAKE) -C $(@D) PREFIX=$(HOST_DIR)/usr install-bin install-lib \
+		install-includes
+
 endef
 
 $(eval $(generic-package))
-- 
2.7.4

[Buildroot] [PATCH 2/2] host-dtc: Install libftd and associated header files.

[Thomas Petazzoni]

Hello,

On Mon, 25 Jul 2016 15:52:27 -0400, David Raeman wrote:
> The dtc package currently does not install libfdt for the host install.
> It can be useful to have libfdt on the host, such as for building QEMU
> with the --enable-fdt configure switch.
> 
> Signed-off-by: David Raeman <draeman@bbn.com>

We already have the support for --enable-fdt for the host qemu:

ifeq ($(BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE),y)
HOST_QEMU_TARGETS += $(HOST_QEMU_ARCH)-softmmu
HOST_QEMU_OPTS += --enable-system --enable-fdt
HOST_QEMU_DEPENDENCIES += host-dtc
else
HOST_QEMU_OPTS += --disable-system
endif

Do you mean that it doesn't work?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] host-dtc: Install libftd and associated header files.

[David Raeman]

Hello,

Sorry for the confusion.  I did not mean to imply any problem with the qemu
package provided by buildroot.  I am emulating a Xilinx Zynq target, so I
use a custom package in my br2-external tree that fetches the version of
QEMU that is patched by Xilinx.  It is fetched from
https://github.com/Xilinx/qemu using the new github hook for the SITE macro:

        XLNX_QEMU_SITE = $(call github,Xilinx,qemu,$(XLNX_QEMU_VERSION))

The canonical QEMU repository embeds a ref to dtc's upstream repo in its
source tree using a git submodule.  The QEMU source tree fetched by
buildroot from http://wiki.qemu.org/download/ has that submodule content
populated as a part of the downloaded tarball.  So the qemu package is able
to build using its internal tree of dtc/libfdt, and thus it does not require
finding libfdt within HOST_DIR.

In my case, the Xilinx-patched version downloaded using $(call github ...)
is fetching a tarball from GitHub which does not have the embedded Git
submodule populated.  So the build is instead looking for libfdt within
HOST_DIR and fails if it does not find it.

If there is an aversion to allowing libfdt to be installed as a part of the
host package, I could instead change my br2-external package to download the
Xilinx-patched QEMU using the git:// scheme instead of grabbing the tarball
via $(call github ...).  Then it would grab the Git submodule reference and
I could fetch it by customizing EXTRACT_CMDS.  Either approach is equally
fine with me.

Cheers,
David


-----Original Message-----
From: Thomas Petazzoni [mailto:thomas.petazzoni at free-electrons.com] 
Sent: Monday, July 25, 2016 4:02 PM
To: David Raeman <draeman@bbn.com>
Cc: buildroot at buildroot.org
Subject: Re: [Buildroot] [PATCH 2/2] host-dtc: Install libftd and associated
header files.

Hello,

On Mon, 25 Jul 2016 15:52:27 -0400, David Raeman wrote:
> The dtc package currently does not install libfdt for the host install.
> It can be useful to have libfdt on the host, such as for building QEMU 
> with the --enable-fdt configure switch.
> 
> Signed-off-by: David Raeman <draeman@bbn.com>

We already have the support for --enable-fdt for the host qemu:

ifeq ($(BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE),y)
HOST_QEMU_TARGETS += $(HOST_QEMU_ARCH)-softmmu HOST_QEMU_OPTS +=
--enable-system --enable-fdt HOST_QEMU_DEPENDENCIES += host-dtc else
HOST_QEMU_OPTS += --disable-system endif

Do you mean that it doesn't work?

Thanks,

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering http://free-electrons.com

[Buildroot] [PATCH 2/2] host-dtc: Install libftd and associated header files.

[Yann E. MORIN]

David, All,

On 2016-07-25 16:26 -0400, David Raeman spake thusly:
> Sorry for the confusion.  I did not mean to imply any problem with the qemu
> package provided by buildroot.  I am emulating a Xilinx Zynq target, so I
> use a custom package in my br2-external tree that fetches the version of
> QEMU that is patched by Xilinx.  It is fetched from
> https://github.com/Xilinx/qemu using the new github hook for the SITE macro:
> 
>         XLNX_QEMU_SITE = $(call github,Xilinx,qemu,$(XLNX_QEMU_VERSION))
> 
> The canonical QEMU repository embeds a ref to dtc's upstream repo in its
> source tree using a git submodule.  The QEMU source tree fetched by
> buildroot from http://wiki.qemu.org/download/ has that submodule content
> populated as a part of the downloaded tarball.  So the qemu package is able
> to build using its internal tree of dtc/libfdt, and thus it does not require
> finding libfdt within HOST_DIR.

That's a shame they have a bundled version... Is it possible to tell
qemu to use the system-installed one (i.e. in our host-dir) instead of
its own bundled one?

> In my case, the Xilinx-patched version downloaded using $(call github ...)
> is fetching a tarball from GitHub which does not have the embedded Git
> submodule populated.  So the build is instead looking for libfdt within
> HOST_DIR and fails if it does not find it.
> 
> If there is an aversion to allowing libfdt to be installed as a part of the
> host package,

I think those explanations are enough to warant installation of libfdt
for the host-package. However, it (or a striped-down version of it)
should have been part of the commit log for your patch.

> I could instead change my br2-external package to download the
> Xilinx-patched QEMU using the git:// scheme instead of grabbing the tarball
> via $(call github ...).  Then it would grab the Git submodule reference and
> I could fetch it by customizing EXTRACT_CMDS.  Either approach is equally
> fine with me.

Note that we now have support for git-submodules since f109e7e
(support/download/git: add support for submodules) and documented in the
manual (nightly version):

    http://nightly.buildroot.org/

    LIBFOO_GIT_SUBMODULES, when LIBFOO_SITE_METHOD=git, will create an
    archive with the git submodules in the repository. Note that we try
    not to use such git submodules when they contain bundled libraries,
    in which case we prefer to use those libraries from their own package.

Regards,
Yann E. MORIN.

> Cheers,
> David
> 
> 
> -----Original Message-----
> From: Thomas Petazzoni [mailto:thomas.petazzoni at free-electrons.com] 
> Sent: Monday, July 25, 2016 4:02 PM
> To: David Raeman <draeman@bbn.com>
> Cc: buildroot at buildroot.org
> Subject: Re: [Buildroot] [PATCH 2/2] host-dtc: Install libftd and associated
> header files.
> 
> Hello,
> 
> On Mon, 25 Jul 2016 15:52:27 -0400, David Raeman wrote:
> > The dtc package currently does not install libfdt for the host install.
> > It can be useful to have libfdt on the host, such as for building QEMU 
> > with the --enable-fdt configure switch.
> > 
> > Signed-off-by: David Raeman <draeman@bbn.com>
> 
> We already have the support for --enable-fdt for the host qemu:
> 
> ifeq ($(BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE),y)
> HOST_QEMU_TARGETS += $(HOST_QEMU_ARCH)-softmmu HOST_QEMU_OPTS +=
> --enable-system --enable-fdt HOST_QEMU_DEPENDENCIES += host-dtc else
> HOST_QEMU_OPTS += --disable-system endif
> 
> Do you mean that it doesn't work?
> 
> Thanks,
> 
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Yann E. MORIN]

David, All,

On 2016-07-25 15:52 -0400, David Raeman spake thusly:
> HOST_CFLAGS includes a search path for HOST_DIR/usr/include using -I.  When
> HOST_CFLAGS is used by a package, these flags are passed to the compiler ahead
> of flags passed by the package's internal make system.  If a package has a
> header file with the same name as a header file in HOST_DIR, this causes the
> toolchain to prefer the file from the system include directory because its -I
> appears first on the command line.  I believe conflicts should prefer the file
                                     ^^^
Please, no personal messages like this. Use alternate, neutral
sentences, maybe something like:

    In such a case, the package-provided file should take precedence
    over any system-installed header.

> provided by the package.  This can be accomplished by using -isystem, which is
> more appropriate then -I for system-level include paths.
> 
> Real-world example: I need libfdt present in my HOST_DIR to install a patched
> version of QEMU.  Meanwhile, the u-boot package provides its own copy of
> libfdt.h that is modified from upstream.  If I have libfdt installed into
> HOST_DIR, then host-uboot-tools fails to build because it grabs the libfdt.h
> from the HOST_DIR area instead of using the patched version from its own
> source tree.  This patch corrects this issue.

Same here, please rephrase so that the sentences are neutral:

    For example, qemu from the Foo repository does not bundle a libfdt,
    and will use the system one when installed. However, qemu has a
    libfdt.h header, when libfdt installs its own libfdt.h.

> This assumes the -isystem flag is supported by the host compiler.

After discussing with Thomas on IRC, this is virtually the case for all
current gcc versions: even gcc-3.x had it, and it's long been deprecated
now.
 
> Signed-off-by: David Raeman <draeman@bbn.com>
> ---
>  package/Makefile.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index afd5d3a..b0ef706 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -222,7 +222,7 @@ UNZIP := $(shell which unzip || type -p unzip) -q
>  
>  APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
>  
> -HOST_CPPFLAGS  = -I$(HOST_DIR)/usr/include
> +HOST_CPPFLAGS  = -isystem $(HOST_DIR)/usr/include

As I was telling Thomas: the idea totally makes sense, but I can't
assess how it would break...

Still, we can't know if we don;t apply it, so:

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>o

Regards,
Yann E. MORIN.

>  HOST_CFLAGS   ?= -O2
>  HOST_CFLAGS   += $(HOST_CPPFLAGS)
>  HOST_CXXFLAGS += $(HOST_CFLAGS)
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Thomas Petazzoni]

Hello,

On Mon, 25 Jul 2016 15:52:26 -0400, David Raeman wrote:
> HOST_CFLAGS includes a search path for HOST_DIR/usr/include using -I.  When
> HOST_CFLAGS is used by a package, these flags are passed to the compiler ahead
> of flags passed by the package's internal make system.  If a package has a
> header file with the same name as a header file in HOST_DIR, this causes the
> toolchain to prefer the file from the system include directory because its -I
> appears first on the command line.  I believe conflicts should prefer the file
> provided by the package.  This can be accomplished by using -isystem, which is
> more appropriate then -I for system-level include paths.
> 
> Real-world example: I need libfdt present in my HOST_DIR to install a patched
> version of QEMU.  Meanwhile, the u-boot package provides its own copy of
> libfdt.h that is modified from upstream.  If I have libfdt installed into
> HOST_DIR, then host-uboot-tools fails to build because it grabs the libfdt.h
> from the HOST_DIR area instead of using the patched version from its own
> source tree.  This patch corrects this issue.
> 
> This assumes the -isystem flag is supported by the host compiler.
> 
> Signed-off-by: David Raeman <draeman@bbn.com>
> ---
>  package/Makefile.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

It makes sense, and -isystem exists since at least gcc 3.0. I've
slightly tweaked your commit log, and applied. We'll see what our
autobuilders say, since it's really hard to judge all the possible
impacts of such a subtle change.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Yann E. MORIN]

David, All,

On 2016-07-25 15:52 -0400, David Raeman spake thusly:
> HOST_CFLAGS includes a search path for HOST_DIR/usr/include using -I.  When
> HOST_CFLAGS is used by a package, these flags are passed to the compiler ahead
> of flags passed by the package's internal make system.  If a package has a
> header file with the same name as a header file in HOST_DIR, this causes the
> toolchain to prefer the file from the system include directory because its -I
> appears first on the command line.  I believe conflicts should prefer the file
> provided by the package.  This can be accomplished by using -isystem, which is
> more appropriate then -I for system-level include paths.

This breaks building the host-python:

    Python build finished, but the necessary bits to build these modules were not found:
    _bsddb             _curses            _curses_panel
    _sqlite3           _ssl               _tkinter
    bsddb185           bz2                dbm
    dl                 gdbm               imageop
    readline           sunaudiodev        zlib
    To find the necessary bits, look in setup.py in detect_modules() for the module's name.

which in turn makes host-python-setuptoolss fail to build as well:

    http://autobuild.buildroot.org/?reason=host-python-setuptools-18.7.1
    http://autobuild.buildroot.org/results/caf/cafe38622a592ac2df96306dfd9b2c9c0450e4f0/build-end.log

But the failure is really due to Python not finding the jost-zlib that
is installed.

Could you have a look at this please?

Regards,
Yann E. MORIN.

> Real-world example: I need libfdt present in my HOST_DIR to install a patched
> version of QEMU.  Meanwhile, the u-boot package provides its own copy of
> libfdt.h that is modified from upstream.  If I have libfdt installed into
> HOST_DIR, then host-uboot-tools fails to build because it grabs the libfdt.h
> from the HOST_DIR area instead of using the patched version from its own
> source tree.  This patch corrects this issue.
> 
> This assumes the -isystem flag is supported by the host compiler.
> 
> Signed-off-by: David Raeman <draeman@bbn.com>
> ---
>  package/Makefile.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index afd5d3a..b0ef706 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -222,7 +222,7 @@ UNZIP := $(shell which unzip || type -p unzip) -q
>  
>  APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
>  
> -HOST_CPPFLAGS  = -I$(HOST_DIR)/usr/include
> +HOST_CPPFLAGS  = -isystem $(HOST_DIR)/usr/include
>  HOST_CFLAGS   ?= -O2
>  HOST_CFLAGS   += $(HOST_CPPFLAGS)
>  HOST_CXXFLAGS += $(HOST_CFLAGS)
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Khem Raj]

> On Jul 28, 2016, at 3:00 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> 
> David, All,
> 
> On 2016-07-25 15:52 -0400, David Raeman spake thusly:
>> HOST_CFLAGS includes a search path for HOST_DIR/usr/include using -I.  When
>> HOST_CFLAGS is used by a package, these flags are passed to the compiler ahead
>> of flags passed by the package's internal make system.  If a package has a
>> header file with the same name as a header file in HOST_DIR, this causes the
>> toolchain to prefer the file from the system include directory because its -I
>> appears first on the command line.  I believe conflicts should prefer the file
>> provided by the package.  This can be accomplished by using -isystem, which is
>> more appropriate then -I for system-level include paths.
> 
> This breaks building the host-python:
> 
>    Python build finished, but the necessary bits to build these modules were not found:
>    _bsddb             _curses            _curses_panel
>    _sqlite3           _ssl               _tkinter
>    bsddb185           bz2                dbm
>    dl                 gdbm               imageop
>    readline           sunaudiodev        zlib
>    To find the necessary bits, look in setup.py in detect_modules() for the module's name.
> 
> which in turn makes host-python-setuptoolss fail to build as well:
> 
>    http://autobuild.buildroot.org/?reason=host-python-setuptools-18.7.1
>    http://autobuild.buildroot.org/results/caf/cafe38622a592ac2df96306dfd9b2c9c0450e4f0/build-end.log
> 
> But the failure is really due to Python not finding the jost-zlib that
> is installed.
> 
> Could you have a look at this please?
> 
> Regards,
> Yann E. MORIN.
> 
>> Real-world example: I need libfdt present in my HOST_DIR to install a patched
>> version of QEMU.  Meanwhile, the u-boot package provides its own copy of
>> libfdt.h that is modified from upstream.  If I have libfdt installed into
>> HOST_DIR, then host-uboot-tools fails to build because it grabs the libfdt.h
>> from the HOST_DIR area instead of using the patched version from its own
>> source tree.  This patch corrects this issue.
>> 
>> This assumes the -isystem flag is supported by the host compiler.
>> 
>> Signed-off-by: David Raeman <draeman@bbn.com>
>> ---
>> package/Makefile.in | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/package/Makefile.in b/package/Makefile.in
>> index afd5d3a..b0ef706 100644
>> --- a/package/Makefile.in
>> +++ b/package/Makefile.in
>> @@ -222,7 +222,7 @@ UNZIP := $(shell which unzip || type -p unzip) -q
>> 
>> APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
>> 
>> -HOST_CPPFLAGS  = -I$(HOST_DIR)/usr/include
>> +HOST_CPPFLAGS  = -isystem $(HOST_DIR)/usr/include

This is quite intrusive change. avoid -isystems since it interferes with the
default include ordering of gcc and will cause compile failure since it wont
be able to find headers in some cases. So many packages will fail to build.
especially gcc-6.x will hit hard

>> HOST_CFLAGS   ?= -O2
>> HOST_CFLAGS   += $(HOST_CPPFLAGS)
>> HOST_CXXFLAGS += $(HOST_CFLAGS)
>> --
>> 2.7.4
>> 
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 204 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160728/2146c0aa/attachment.asc>

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[David Raeman]

Yann, All,

I am unable to reproduce the failure even using the defconfig from the autobuild entry.  However, I poked around in setup.py and believe I found the reason for the failure and perhaps the reason why I cannot reproduce it on my host system.

The root of the issue is in host-python-2.7.12/setup.py, around line 478.  The code is grabbing the CPPFLAGS environment variable and is building a list of include directories by grabbing all instances of the -I argument from that environment variable.  Of course that means it will miss the directory that is being provided with -isystem.

That list of include directories is then used to search for header files to determine which system libraries are available, and to enable or disable respective modules.  Around line 526, the directory /usr/include is automatically added to the list if not cross-compiling (since this is a host package).  Since my host system has zlib installed natively, this file-searching logic will find /usr/include/zlib.h and knows that zlib is on the host system, even though it did not search the proper area at .../buildroot/output/host/usr/include.  If the autobuild system does not have zlib header natively at /usr/include/zlib.h, that would explain why autobuild fails and yet I cannot reproduce the failure.

The solution should be a one-line patch to setup.py so that it also looks for the -isystem flag in CPPFLAGS.  I will follow-up with a patch.

Cheers,
David


-----Original Message-----
From: Yann E. MORIN [mailto:yann.morin.1998 at free.fr] 
Sent: Thursday, July 28, 2016 6:01 PM
To: David Raeman <draeman@bbn.com>
Cc: buildroot at buildroot.org
Subject: Re: [Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

David, All,

On 2016-07-25 15:52 -0400, David Raeman spake thusly:
> HOST_CFLAGS includes a search path for HOST_DIR/usr/include using -I.  
> When HOST_CFLAGS is used by a package, these flags are passed to the 
> compiler ahead of flags passed by the package's internal make system.  
> If a package has a header file with the same name as a header file in 
> HOST_DIR, this causes the toolchain to prefer the file from the system 
> include directory because its -I appears first on the command line.  I 
> believe conflicts should prefer the file provided by the package.  
> This can be accomplished by using -isystem, which is more appropriate then -I for system-level include paths.

This breaks building the host-python:

    Python build finished, but the necessary bits to build these modules were not found:
    _bsddb             _curses            _curses_panel
    _sqlite3           _ssl               _tkinter
    bsddb185           bz2                dbm
    dl                 gdbm               imageop
    readline           sunaudiodev        zlib
    To find the necessary bits, look in setup.py in detect_modules() for the module's name.

which in turn makes host-python-setuptoolss fail to build as well:

    http://autobuild.buildroot.org/?reason=host-python-setuptools-18.7.1
    http://autobuild.buildroot.org/results/caf/cafe38622a592ac2df96306dfd9b2c9c0450e4f0/build-end.log

But the failure is really due to Python not finding the jost-zlib that is installed.

Could you have a look at this please?

Regards,
Yann E. MORIN.

> Real-world example: I need libfdt present in my HOST_DIR to install a 
> patched version of QEMU.  Meanwhile, the u-boot package provides its 
> own copy of libfdt.h that is modified from upstream.  If I have libfdt 
> installed into HOST_DIR, then host-uboot-tools fails to build because 
> it grabs the libfdt.h from the HOST_DIR area instead of using the 
> patched version from its own source tree.  This patch corrects this issue.
> 
> This assumes the -isystem flag is supported by the host compiler.
> 
> Signed-off-by: David Raeman <draeman@bbn.com>
> ---
>  package/Makefile.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in index 
> afd5d3a..b0ef706 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -222,7 +222,7 @@ UNZIP := $(shell which unzip || type -p unzip) -q
>  
>  APPLY_PATCHES = support/scripts/apply-patches.sh $(if $(QUIET),-s)
>  
> -HOST_CPPFLAGS  = -I$(HOST_DIR)/usr/include
> +HOST_CPPFLAGS  = -isystem $(HOST_DIR)/usr/include
>  HOST_CFLAGS   ?= -O2
>  HOST_CFLAGS   += $(HOST_CPPFLAGS)
>  HOST_CXXFLAGS += $(HOST_CFLAGS)
> --
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

--
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' 
| conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] host-python: Find headers provided by -isystem flag in CPPFLAGS

[David Raeman]

The Python setup.py logic will enable and disable modules for compilation
based on which libraries it believes are available.  The logic in this
script parses certain arguments out of CPPFLAGS to determine search
directories, and then it checks for existence of various system libaries
and headers within those search directories.

This patch will allow the logic to consider include directories that
are specified using the -isystem flag.  The default logic only considers
include directories specified using the -I flag.

Signed-off-by: David Raeman <draeman@bbn.com>
---
 package/python/116-search-isystem-inc.patch | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 package/python/116-search-isystem-inc.patch

diff --git a/package/python/116-search-isystem-inc.patch b/package/python/116-search-isystem-inc.patch
new file mode 100644
index 0000000..af0c788
--- /dev/null
+++ b/package/python/116-search-isystem-inc.patch
@@ -0,0 +1,24 @@
+Allow Python to find system header files in locations specified with
+flag -isystem in addition to the default behavior of searching -I.
+
+These directories are not used for complication, but are used to
+perform existence-checks for library header files so that Python
+can determine which modules should be built for the system.
+
+Signed-off-by: David Raeman <draeman@bbn.com>
+
+Index: b/setup.py
+===================================================================
+--- a/setup.py	2016-07-28 23:46:37.574969153 -0400
++++ b/setup.py	2016-07-29 00:01:02.928906258 -0400
+@@ -478,6 +478,10 @@
+                 ('CPPFLAGS', '-I', self.compiler.include_dirs)):
+             env_val = sysconfig.get_config_var(env_var)
+             if env_val:
++                # For the purpose of finding search directories, treat the
++                # -isystem flag as if it were a -I flag.
++                env_val = re.sub(r'(?i)(^|\s+)-isystem\s*', ' -I', env_val)
++
+                 # To prevent optparse from raising an exception about any
+                 # options in env_val that it doesn't know about we strip out
+                 # all double dashes and any dashes followed by a character
-- 
2.9.2

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Thomas Petazzoni]

Hello,

On Thu, 28 Jul 2016 15:04:18 -0700, Khem Raj wrote:

> >> -HOST_CPPFLAGS  = -I$(HOST_DIR)/usr/include
> >> +HOST_CPPFLAGS  = -isystem $(HOST_DIR)/usr/include  
> 
> This is quite intrusive change. avoid -isystems since it interferes with the
> default include ordering of gcc and will cause compile failure since it wont
> be able to find headers in some cases. So many packages will fail to build.
> especially gcc-6.x will hit hard

Can you give some more details? Did you look at the original commit log
for this change? It pointed out some real problems when using -I, which
in theory should be resolved by using -isystem.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Khem Raj]

On Fri, Jul 29, 2016 at 12:32 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Thu, 28 Jul 2016 15:04:18 -0700, Khem Raj wrote:
>
>> >> -HOST_CPPFLAGS  = -I$(HOST_DIR)/usr/include
>> >> +HOST_CPPFLAGS  = -isystem $(HOST_DIR)/usr/include
>>
>> This is quite intrusive change. avoid -isystems since it interferes with the
>> default include ordering of gcc and will cause compile failure since it wont
>> be able to find headers in some cases. So many packages will fail to build.
>> especially gcc-6.x will hit hard
>
> Can you give some more details? Did you look at the original commit log
> for this change? It pointed out some real problems when using -I, which
> in theory should be resolved by using -isystem.

see
http://cgit.openembedded.org/cgit.cgi/openembedded-core/commit/?id=0a6e162c47017ecf51b466218fb549e0e199f4c4

If packages use include_next they will have problems.

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Thomas Petazzoni]

Hello,

On Fri, 29 Jul 2016 01:16:24 -0700, Khem Raj wrote:

> > Can you give some more details? Did you look at the original commit log
> > for this change? It pointed out some real problems when using -I, which
> > in theory should be resolved by using -isystem.  
> 
> see
> http://cgit.openembedded.org/cgit.cgi/openembedded-core/commit/?id=0a6e162c47017ecf51b466218fb549e0e199f4c4
> 
> If packages use include_next they will have problems.

Right, but like your patch shows, problems occur when packages are
broken, so they should "simply" be fixed.

I don't really see another option than using -isystem in fact. What do
you suggest?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Khem Raj]

On Fri, Jul 29, 2016 at 2:23 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Fri, 29 Jul 2016 01:16:24 -0700, Khem Raj wrote:
>
>> > Can you give some more details? Did you look at the original commit log
>> > for this change? It pointed out some real problems when using -I, which
>> > in theory should be resolved by using -isystem.
>>
>> see
>> http://cgit.openembedded.org/cgit.cgi/openembedded-core/commit/?id=0a6e162c47017ecf51b466218fb549e0e199f4c4
>>
>> If packages use include_next they will have problems.
>
> Right, but like your patch shows, problems occur when packages are
> broken, so they should "simply" be fixed.

yes

>
> I don't really see another option than using -isystem in fact. What do
> you suggest?


with specifying your own -isystem you will be introducing the issue too see
this thread for more
https://lists.fedoraproject.org/archives/list/devel at lists.fedoraproject.org/thread/Q5SWCUUMWQ4EMS7CU2CBOZHV3WZYOOTT/

I had to remove it from OE all around


>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[David Raeman]

Hello,

On Fri, Jul 29, 2016 at 11:09 AM, Khem Raj <raj.khem@gmail.com> wrote:
> with specifying your own -isystem you will be introducing the issue too see
> this thread for more
> https://lists.fedoraproject.org/archives/list/devel at lists.fedoraproject.org/t
> hread/Q5SWCUUMWQ4EMS7CU2CBOZHV3WZYOOTT/

Yes, I see what you mean.  GCC-6 is now using #include_next in its C++ standard library header files to help the C++ STL header files properly find their own special copy of stdlib.h.  The behavior of #include_next will change depending on the order of the directories in the include search path.  Based on these tickets, it looks like a problem is introduced if you use -isystem to specify a directory that gcc would already have considered as part of its default system include directories.  Doing so causes that particular directory to be moved up earlier in the search order, so the internal use of #include_next ends up failing because it is relying on the "known" order of the default system include directories.  Note that this is not a problem with the -I flag, because gcc will ignore the -I flag if the provided directory is in the default search path.  It's only -isystem that might rearrange the order of default directories.

Here is another ticket in the GCC tracker for this issue, marked WONTFIX: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129.  The recommendation is to not use -isystem for directories that gcc already includes in the default search path.

It's not clear to me that the specific use of -isystem introduced in this patch would violate this assertion.  Are there configurations for buildroot where "$(HOST_DIR)/usr/include" can resolve to a default include directory as listed in https://gcc.gnu.org/onlinedocs/cpp/Search-Path.html ?  It's not possible for my particular use, but I don't know all the uses of buildroot well enough to make that statement in the blanket general case.

However, I still very much appreciate the overall point you make - that use of the flag can cause subtle and unintended consequences.  If the maintainers choose to back the patch out, I can find another way to resolve my original issue by submitting a patch to u-boot to ensure it prefers its modified copy of libfdt.h instead of the possibly-installed system copy.  I still believe the use of -isystem here is correct in a strict technical sense, because "$(HOST_DIR)/usr/include" is truly a system include directory at a non-standard location.  But I understand that some packages incorrectly assume that -I is the only way to add include paths and don't anticipate the use of -isystem.  Case in point, the Python package I just submitted a patch for.

Cheers,
David

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Yann E. MORIN]

David, All,

[Please, wrap lines at ~72 chars; it's easier to read.]

On 2016-07-29 15:35 -0400, David Raeman spake thusly:
[--SNIP--]
> If the maintainers choose to back the patch out,

This is what I think is the best solution. Currently, all builds that
want to build a setuptools-based python package will fail; which means
that:

  - users will be very disapointed, and that's not good;

  - we are potentially missing on catching other build failures in our
    autobuilders, which is not good either, especially since we are soon
    to enter the stabilisation -rc phase in early August.

So, I think we should revert this patch (see below).

> I can find another way
> to resolve my original issue by submitting a patch to u-boot to ensure

U-Boot or Qemu? I thought you needed that for your custom Qemu...

So, because in Buildroot, we do not have a mean to use that custom Qemu,
we do not (yet?) have the issue, so we should not try to fix it. For now.

Also, it seems that a proper fix will be really complex, if at all
possible, given that systems (e.g. rolling or bleeding-edge distros)
with a gcc-6 are broken with this change (and not specifically because
of host-python).

And thus we should revert that patch. Will you send a patch to do the
revert, please?

> it prefers its modified copy of libfdt.h instead of the
> possibly-installed system copy.

I have had a similar issue quite recently, when working on the
linux-gpib package, especially the kernel parts.

Their buildsystem is not very clean, and they have headers with the same
names of other headers from the kernel, because they need to #define
legacy macros for backward compatibility, and then trampoline to the
original headers by way of #include_next.

Here is the patch I did (basically: rename the shadowing headers):

    https://gitlab.com/ymorin/buildroot/blob/yem/linux-gpib/package/linux-gpib/0005-drivers-gpib-rename-kernel-override-includes.patch

So, I think the best solution would be for U-Boot^WQemu to rename
their header.

Regards,
Yann E. MORIN.

> I still believe the use of -isystem here is correct in a strict technical
> sense, because "$(HOST_DIR)/usr/include" is truly a system include
> directory at a non-standard location.  But I understand that some packages incorrectly
> assume that -I is the only way to add include paths and don't anticipate
> the use of -isystem.  Case in point, the Python package I just submitted
> a patch for.
> 
> Cheers,
> David
> 
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[David Raeman]

Yann, All,

On Fri, Jul 29, 2016 at 5:16 PM, Yann E. MORIN wrote:
> U-Boot or Qemu? I thought you needed that for your custom Qemu

The problem was revealed when I added a br2-external package for 
custom Qemu, but I traced the root issue to be with u-boot and how it
is impacted by the include directory added by HOST_CPPFLAGS.

The custom Qemu required me to install libfdt into the host area -
nothing wrong with doing that.  But having libfdt installed in
HOST_DIR causes u-boot to fail, because u-boot was finding libfdt.h 
in HOST_DIR/usr/include prior to searching its own directory for its
local modified copy of that same filename.

> And thus we should revert that patch. Will you send a patch to do the revert,
> please?

Yes I will.  I believe the patch to revert should also back out patch 
2/2 that installed libfdt to HOST_DIR as a part of host-dtc.  As 
mentioned above, if you keep that change and only revert the change to
HOST_CPPFLAGS, then u-boot to fail unless something else is done.  Do
you agree I should revert both changes with this new patch, to ensure
there is no leftover breakage?

> So, I think the best solution would be for U-Boot^WQemu to rename their
> header.

Given the various considerations, I agree this is the lowest-risk
solution.  Keeping in mind that any other package with a local header
file in conflict with a HOST_DIR header file can experience this same
problem.  The root issue is that usage of CPPFLAGS causes packages
to search HOST_DIR/usr/include before they search their local source
directories.

I may be a bit of time before I can submit a new patch to rename the
header file within u-boot.

Cheers,
David

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Yann E. MORIN]

David, All,

On 2016-07-25 15:52 -0400, David Raeman spake thusly:
> HOST_CFLAGS includes a search path for HOST_DIR/usr/include using -I.  When
> HOST_CFLAGS is used by a package, these flags are passed to the compiler ahead
> of flags passed by the package's internal make system.  If a package has a
> header file with the same name as a header file in HOST_DIR, this causes the
> toolchain to prefer the file from the system include directory because its -I
> appears first on the command line.  I believe conflicts should prefer the file
> provided by the package.  This can be accomplished by using -isystem, which is
> more appropriate then -I for system-level include paths.
[--SNIP--]

In fact, this does not work, and the patch was reverted (for background).

What we need is that gcc follows the following sequence when looking for
headers:

 1- search paths specified with -I by the package

 2- search our $(HOST_DIR)/usr/include

 3- search the host system /usr/include

However, I can't find anything in the gcc manpage that allows to add the
search path we need in (2).

All I can see is -idirafter, to add search paths after all other
non-system and system paths.

So, probably the following would not be very far from what we'd have to
do:

    -nostdinc -idirafter $(HOST_DIR)/usr/include -idirafter /usr/include

This tells gcc to not look at the system include path, so it is left
with only the paths specified by the package, and then as a fallback,
out $(HOST_DIR)/usr/include, and then as a last resort, the host-system
/usr/include dirs, since the last two are -idirafter:

    -idirafter dir
        Search dir for header files, but do it after all directories
        specified with -I and the standard system directories have
        been exhausted. dir is  treated as a system include directory.

Thoughts?

David, would you care to see if:

 1- it fixes your own problem?

 2- it fixes the python issues we've had with -isystem?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Yann E. MORIN]

David, All,

On 2016-08-01 23:59 +0200, Yann E. MORIN spake thusly:
> On 2016-07-25 15:52 -0400, David Raeman spake thusly:
> > HOST_CFLAGS includes a search path for HOST_DIR/usr/include using -I.  When
> > HOST_CFLAGS is used by a package, these flags are passed to the compiler ahead
> > of flags passed by the package's internal make system.  If a package has a
> > header file with the same name as a header file in HOST_DIR, this causes the
> > toolchain to prefer the file from the system include directory because its -I
> > appears first on the command line.  I believe conflicts should prefer the file
> > provided by the package.  This can be accomplished by using -isystem, which is
> > more appropriate then -I for system-level include paths.
> [--SNIP--]
> 
> In fact, this does not work, and the patch was reverted (for background).
> 
> What we need is that gcc follows the following sequence when looking for
> headers:
> 
>  1- search paths specified with -I by the package
> 
>  2- search our $(HOST_DIR)/usr/include
> 
>  3- search the host system /usr/include
> 
> However, I can't find anything in the gcc manpage that allows to add the
> search path we need in (2).
> 
> All I can see is -idirafter, to add search paths after all other
> non-system and system paths.
> 
> So, probably the following would not be very far from what we'd have to
> do:
> 
>     -nostdinc -idirafter $(HOST_DIR)/usr/include -idirafter /usr/include
> 
> This tells gcc to not look at the system include path, so it is left
> with only the paths specified by the package, and then as a fallback,
> out $(HOST_DIR)/usr/include, and then as a last resort, the host-system
> /usr/include dirs, since the last two are -idirafter:
> 
>     -idirafter dir
>         Search dir for header files, but do it after all directories
>         specified with -I and the standard system directories have
>         been exhausted. dir is  treated as a system include directory.
> 
> Thoughts?
> 
> David, would you care to see if:
> 
>  1- it fixes your own problem?

No, it does not work as-is, because there is more that just /usr/include
as the standard headers search path. On my machine, gcc looks for:

    $ printf "" |`gcc -print-prog-name=cc1` -v -o /dev/null >/dev/null
    #include "..." search starts here:
    #include <...> search starts here:
     /usr/lib/gcc/x86_64-linux-gnu/5/include
     /usr/local/include
     /usr/lib/gcc/x86_64-linux-gnu/5/include-fixed
     /usr/include
    End of search list.
    [--SNIP--]

Adding those four makes the build suceeds, though...

But there is no simple way to get that list of search path either, so
that's a no-no for now... :-/

Furthermore, the C++ search dirs are even different:

    $ printf "" |`gcc -print-prog-name=cc1` -v -o /dev/null >/dev/null
    #include "..." search starts here:
    #include <...> search starts here:
     /usr/include/c++/5
     /usr/include/x86_64-linux-gnu/c++/5
     /usr/include/c++/5/backward
     /usr/lib/gcc/x86_64-linux-gnu/5/include
     /usr/local/include
     /usr/lib/gcc/x86_64-linux-gnu/5/include-fixed
     /usr/include
    End of search list.

Gah... :-(

Forget it, that was not a good idea... :-(

Regards,
Yann E. MORIN.

>  2- it fixes the python issues we've had with -isystem?
> 
> Regards,
> Yann E. MORIN.
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[David Raeman]

Yann, All,

On Mon, Aug 1, 2016 at 5:59 PM, Yann E. MORIN wrote:
>  2- it fixes the python issues we've had with -isystem?

I see that you already backed away from this idea, but I thought I'd
still chime in on the python question.  You may have missed my email
about what is causing the python issue - please see:
http://lists.busybox.net/pipermail/buildroot/2016-July/168710.html.

In a nutshell, the python setup.py script is peeking in the CPPFLAGS
environment variable and is extracting any uses of the -I flag.  Then
it's checking whether system libraries are installed in any of those
locations, to know which modules can be enabled by the build.

So using any flag to specify include directories other than -I will
not work without patching the python setup.py script.

Cheers,
David

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Thomas Petazzoni]

Hello,

On Thu, 28 Jul 2016 15:04:18 -0700, Khem Raj wrote:

> >> -HOST_CPPFLAGS  = -I$(HOST_DIR)/usr/include
> >> +HOST_CPPFLAGS  = -isystem $(HOST_DIR)/usr/include  
> 
> This is quite intrusive change. avoid -isystems since it interferes with the
> default include ordering of gcc and will cause compile failure since it wont
> be able to find headers in some cases. So many packages will fail to build.
> especially gcc-6.x will hit hard

After reading your links, and reading
http://stackoverflow.com/a/37218954, I believe your concerns do not
apply to our case.

The problem with -isystem occurs when you pass -isystem with a
directory that is *already* in the default search path of the compiler.
In this case, you indeed modify the order of the directories, and
therefore cause problems for #include_next directives.

However, in our case, we want to prepend to the default search path a
completely different directory, $(HOST_DIR)/usr/include. Therefore, any
#include_next will continue to work fine.

See the stackoverflow link above, it explains this very well.

Your webkitgtk example was adding -isystem /usr/include, which is
obviously wrong. And your second link on the Fedora mailing list is
also about adding -isystem /usr/include.

So, I believe that we may see -isystem parsing issues like the Python
one fixed by David, but that generally speaking, using -isystem is the
right thing to do to solve our problem.

What do you think?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Khem Raj]

On 7/29/16 12:35 PM, David Raeman wrote:
> Hello,
> 
> On Fri, Jul 29, 2016 at 11:09 AM, Khem Raj <raj.khem@gmail.com> wrote:
>> with specifying your own -isystem you will be introducing the issue too see
>> this thread for more
>> https://lists.fedoraproject.org/archives/list/devel at lists.fedoraproject.org/t
>> hread/Q5SWCUUMWQ4EMS7CU2CBOZHV3WZYOOTT/
> 
> Yes, I see what you mean.  GCC-6 is now using #include_next in its C++ standard library header files to help the C++ STL header files properly find their own special copy of stdlib.h.  The behavior of #include_next will change depending on the order of the directories in the include search path.  Based on these tickets, it looks like a problem is introduced if you use -isystem to specify a directory that gcc would already have considered as part of its default system include directories.  Doing so causes that particular directory to be moved up earlier in the search order, so the internal use of #include_next ends up failing because it is relying on the "known" order of the default system include directories.  Note that this is not a problem with the -I flag, because gcc will ignore the -I flag if the provided directory is in the default search path.  It's only -isystem that might rearrange the order of default directories.
> 
> Here is another ticket in the GCC tracker for this issue, marked WONTFIX: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129.  The recommendation is to not use -isystem for directories that gcc already includes in the default search path.
> 
> It's not clear to me that the specific use of -isystem introduced in this patch would violate this assertion.  Are there configurations for buildroot where "$(HOST_DIR)/usr/include" can resolve to a default include directory as listed in https://gcc.gnu.org/onlinedocs/cpp/Search-Path.html ?  It's not possible for my particular use, but I don't know all the uses of buildroot well enough to make that statement in the blanket general case.

it will. Since now it will become a system path and will interfere with
gccs search order for include_next

> 
> However, I still very much appreciate the overall point you make - that use of the flag can cause subtle and unintended consequences.  If the maintainers choose to back the patch out, I can find another way to resolve my original issue by submitting a patch to u-boot to ensure it prefers its modified copy of libfdt.h instead of the possibly-installed system copy.  I still believe the use of -isystem here is correct in a strict technical sense, because "$(HOST_DIR)/usr/include" is truly a system include directory at a non-standard location.  But I understand that some packages incorrectly assume that -I is the only way to add include paths and don't anticipate the use of -isystem.  Case in point, the Python package I just submitted a patch for.
> 
> Cheers,
> David
> 
> 

[Buildroot] [PATCH 1/2] package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.

[Khem Raj]

On 7/30/16 7:57 AM, David Raeman wrote:
> Yann, All,
> 
> On Fri, Jul 29, 2016 at 5:16 PM, Yann E. MORIN wrote:
>> U-Boot or Qemu? I thought you needed that for your custom Qemu
> 
> The problem was revealed when I added a br2-external package for 
> custom Qemu, but I traced the root issue to be with u-boot and how it
> is impacted by the include directory added by HOST_CPPFLAGS.
> 
> The custom Qemu required me to install libfdt into the host area -
> nothing wrong with doing that.  But having libfdt installed in
> HOST_DIR causes u-boot to fail, because u-boot was finding libfdt.h 
> in HOST_DIR/usr/include prior to searching its own directory for its
> local modified copy of that same filename.

I think thats a problem in u-boot's build system. So its better to
address it there.

> 
>> And thus we should revert that patch. Will you send a patch to do the revert,
>> please?
> 
> Yes I will.  I believe the patch to revert should also back out patch 
> 2/2 that installed libfdt to HOST_DIR as a part of host-dtc.  As 
> mentioned above, if you keep that change and only revert the change to
> HOST_CPPFLAGS, then u-boot to fail unless something else is done.  Do
> you agree I should revert both changes with this new patch, to ensure
> there is no leftover breakage?
> 
>> So, I think the best solution would be for U-Boot^WQemu to rename their
>> header.
> 
> Given the various considerations, I agree this is the lowest-risk
> solution.  Keeping in mind that any other package with a local header
> file in conflict with a HOST_DIR header file can experience this same
> problem.  The root issue is that usage of CPPFLAGS causes packages
> to search HOST_DIR/usr/include before they search their local source
> directories.
> 
> I may be a bit of time before I can submit a new patch to rename the
> header file within u-boot.
> 
> Cheers,
> David
> 
>