* [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally @ 2018-08-31 14:20 David De Grave 2018-08-31 14:20 ` [Buildroot] [PATCH 2/3] Revert "linux/linux.mk: Use '-isystem' instead of '-I' in HOST_CFLAGS" David De Grave ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: David De Grave @ 2018-08-31 14:20 UTC (permalink / raw) To: buildroot 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(-) diff --git a/package/Makefile.in b/package/Makefile.in index 91b3e8f936..5ed70f0a65 100644 --- a/package/Makefile.in +++ b/package/Makefile.in @@ -223,7 +223,7 @@ UNZIP := $(shell which unzip || type -p unzip) -q APPLY_PATCHES = PATH=$(HOST_DIR)/bin:$$PATH support/scripts/apply-patches.sh $(if $(QUIET),-s) -HOST_CPPFLAGS = -I$(HOST_DIR)/include +HOST_CPPFLAGS = -isystem $(HOST_DIR)/include HOST_CFLAGS ?= -O2 HOST_CFLAGS += $(HOST_CPPFLAGS) HOST_CXXFLAGS += $(HOST_CFLAGS) -- 2.13.6 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 2/3] Revert "linux/linux.mk: Use '-isystem' instead of '-I' in HOST_CFLAGS" 2018-08-31 14:20 [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally David De Grave @ 2018-08-31 14:20 ` David De Grave 2018-08-31 14:20 ` [Buildroot] [PATCH 3/3] boot/uboot/uboot.mk: substitution for '-I/' to '-isystem /' no longer needed David De Grave 2018-09-09 13:32 ` [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally Thomas Petazzoni 2 siblings, 0 replies; 7+ messages in thread From: David De Grave @ 2018-08-31 14:20 UTC (permalink / raw) To: buildroot Since we made the use of '-isystem' global in a previous commit for HOST_CFLAGS, that change is no longer needed. This reverts commit 14a842315c12983eb492dfa310a9c358b7336da6. Signed-off-by: David De Grave (Essensium/Mind) <david.degrave@mind.be> --- linux/linux.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/linux.mk b/linux/linux.mk index a31617ca22..675e7906a8 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -115,7 +115,7 @@ LINUX_EXTRA_DOWNLOADS += $(ARCH_XTENSA_OVERLAY_URL) endif LINUX_MAKE_FLAGS = \ - HOSTCC="$(HOSTCC) $(patsubst -I%,-isystem %,$(HOST_CFLAGS)) $(HOST_LDFLAGS)" \ + HOSTCC="$(HOSTCC) $(HOST_CFLAGS) $(HOST_LDFLAGS)" \ ARCH=$(KERNEL_ARCH) \ INSTALL_MOD_PATH=$(TARGET_DIR) \ CROSS_COMPILE="$(TARGET_CROSS)" \ -- 2.13.6 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 3/3] boot/uboot/uboot.mk: substitution for '-I/' to '-isystem /' no longer needed 2018-08-31 14:20 [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally David De Grave 2018-08-31 14:20 ` [Buildroot] [PATCH 2/3] Revert "linux/linux.mk: Use '-isystem' instead of '-I' in HOST_CFLAGS" David De Grave @ 2018-08-31 14:20 ` David De Grave 2018-09-09 13:32 ` [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally Thomas Petazzoni 2 siblings, 0 replies; 7+ messages in thread From: David De Grave @ 2018-08-31 14:20 UTC (permalink / raw) To: buildroot As the '-isystem' is now used globally, the uboot's substitution to define HOSTCC is no longer needed too. This reverts commit 0bf80e4bcd5277e3ad935e03c632deba4c7316f2 and commit baae5156ce37e8b2775f04710f7d1c8e97e4114c Signed-off-by: David De Grave (Essensium/Mind) <david.degrave@mind.be> --- boot/uboot/uboot.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk index bddafe234d..7cb9859565 100644 --- a/boot/uboot/uboot.mk +++ b/boot/uboot/uboot.mk @@ -132,7 +132,7 @@ endif UBOOT_MAKE_OPTS += \ CROSS_COMPILE="$(TARGET_CROSS)" \ ARCH=$(UBOOT_ARCH) \ - HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem /,$(HOST_CFLAGS)))" \ + HOSTCC="$(HOSTCC) $(HOST_CFLAGS)" \ HOSTLDFLAGS="$(HOST_LDFLAGS)" ifeq ($(BR2_TARGET_UBOOT_NEEDS_ATF_BL31),y) -- 2.13.6 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally 2018-08-31 14:20 [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally David De Grave 2018-08-31 14:20 ` [Buildroot] [PATCH 2/3] Revert "linux/linux.mk: Use '-isystem' instead of '-I' in HOST_CFLAGS" David De Grave 2018-08-31 14:20 ` [Buildroot] [PATCH 3/3] boot/uboot/uboot.mk: substitution for '-I/' to '-isystem /' no longer needed David De Grave @ 2018-09-09 13:32 ` Thomas Petazzoni 2018-09-10 9:51 ` Arnout Vandecappelle 2 siblings, 1 reply; 7+ messages in thread From: Thomas Petazzoni @ 2018-09-09 13:32 UTC (permalink / raw) To: buildroot 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 -- Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally 2018-09-09 13:32 ` [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally Thomas Petazzoni @ 2018-09-10 9:51 ` Arnout Vandecappelle 2018-09-10 13:00 ` David De Grave 0 siblings, 1 reply; 7+ messages in thread From: Arnout Vandecappelle @ 2018-09-10 9:51 UTC (permalink / raw) To: buildroot 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 > -- Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally 2018-09-10 9:51 ` Arnout Vandecappelle @ 2018-09-10 13:00 ` David De Grave 2018-10-21 14:18 ` Yann E. MORIN 0 siblings, 1 reply; 7+ messages in thread From: David De Grave @ 2018-09-10 13:00 UTC (permalink / raw) To: buildroot 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 De Grave* Senior Embedded Software Developer Gsm : +32(0)496.364.960 | Tel : +32-16-28.65.00 | Fax : +32-16-28a.65.01 Essensium-Mind <https://www.mind.be/> - Gaston Geenslaan 9, B-3001 Leuven, Belgium -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20180910/1fdc15a7/attachment.html> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally 2018-09-10 13:00 ` David De Grave @ 2018-10-21 14:18 ` Yann E. MORIN 0 siblings, 0 replies; 7+ messages in thread From: Yann E. MORIN @ 2018-10-21 14:18 UTC (permalink / raw) To: buildroot 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 at mind.be > 2. https://github.com/beagleboard/linux.git > 3. http://linux.mk > 4. https://www.mind.be/ > _______________________________________________ > 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. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-10-21 14:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-31 14:20 [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally David De Grave 2018-08-31 14:20 ` [Buildroot] [PATCH 2/3] Revert "linux/linux.mk: Use '-isystem' instead of '-I' in HOST_CFLAGS" David De Grave 2018-08-31 14:20 ` [Buildroot] [PATCH 3/3] boot/uboot/uboot.mk: substitution for '-I/' to '-isystem /' no longer needed David De Grave 2018-09-09 13:32 ` [Buildroot] [PATCH 1/3] package/Makefile.in: Use '-isystem' instead of '-I' in HOST_CFLAGS globally Thomas Petazzoni 2018-09-10 9:51 ` Arnout Vandecappelle 2018-09-10 13:00 ` David De Grave 2018-10-21 14:18 ` Yann E. MORIN
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.