[Buildroot] [PATCH v2] package/irrlicht: fix libraries linking

[Buildroot] [PATCH v2] package/irrlicht: fix libraries linking

[Bartosz Bilas]

These patches fix the irrlicht makefile which contains the paths
that point to the host system libraries that are not used and
are not available in Buildroot what's unsafe for cross-compilation.
In addition they fix linking to the X11 libraries and the following errors:

/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `XSetSelectionOwner'
/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `glXGetProcAddress'
/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `glXMakeCurrent'
/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `XF86VidModeSetViewPort'
/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `XF86VidModeSwitchToMode'
/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `glClearDepth'
/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `XGetVisualInfo'
/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `XGrabKeyboard'
/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: /home/bartekk/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib/libIrrlicht.so: undefined reference to `glMatrixMode'

Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
---
Changes v1 -> v2:
 - patches merged into one
 - explained commit log
 
IMPORTANT NOTE:
 Irrlicht uses CRLF line endings so the patches must be fixed when
 applying because patchwork drops all the CR and keeps on LF.

 .../0003-makefile-override-LDFLAGS.patch      | 29 ++++++++++++++++
 ...obsolete-X11R6-lib-include-directori.patch | 33 +++++++++++++++++++
 package/irrlicht/irrlicht.mk                  |  4 ++-
 3 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 package/irrlicht/0003-makefile-override-LDFLAGS.patch
 create mode 100644 package/irrlicht/0004-makefile-remove-obsolete-X11R6-lib-include-directori.patch

diff --git a/package/irrlicht/0003-makefile-override-LDFLAGS.patch b/package/irrlicht/0003-makefile-override-LDFLAGS.patch
new file mode 100644
index 0000000000..16c38d5baa
--- /dev/null
+++ b/package/irrlicht/0003-makefile-override-LDFLAGS.patch
@@ -0,0 +1,29 @@
+From f597ac417f0eefab8f388afe617a0d07d08d87bb Mon Sep 17 00:00:00 2001
+From: Bartosz Bilas <b.bilas@grinn-global.com>
+Date: Sun, 21 Jun 2020 12:57:47 +0200
+Subject: [PATCH] makefile: override LDFLAGS
+
+That's needed to fix proper linking to X11 libraries
+within package makefile.
+
+Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
+---
+ source/Irrlicht/Makefile | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/source/Irrlicht/Makefile b/source/Irrlicht/Makefile
+index 9a1bdbc..85eb264 100644
+--- a/source/Irrlicht/Makefile
++++ b/source/Irrlicht/Makefile
+@@ -88,7 +88,7 @@ STATIC_LIB = libIrrlicht.a
+ LIB_PATH = ../../lib/$(SYSTEM)
+ INSTALL_DIR = /usr/local/lib
+ sharedlib install: SHARED_LIB = libIrrlicht.so
+-sharedlib: LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
++sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
+ staticlib sharedlib: CXXINCS += -I/usr/X11R6/include
+ 
+ #OSX specific options
+-- 
+2.27.0
+
diff --git a/package/irrlicht/0004-makefile-remove-obsolete-X11R6-lib-include-directori.patch b/package/irrlicht/0004-makefile-remove-obsolete-X11R6-lib-include-directori.patch
new file mode 100644
index 0000000000..b551905890
--- /dev/null
+++ b/package/irrlicht/0004-makefile-remove-obsolete-X11R6-lib-include-directori.patch
@@ -0,0 +1,33 @@
+From 9a4109dfceccc2e702a6928ec612ee790a31dea7 Mon Sep 17 00:00:00 2001
+From: Bartosz Bilas <b.bilas@grinn-global.com>
+Date: Mon, 22 Jun 2020 20:26:51 +0200
+Subject: [PATCH] makefile: remove obsolete X11R6 lib/include directories
+
+Remove those non supported paths to lib/includes that are not used and
+not available in buildroot and in addition point to the host system
+libraries in a result cause the following warning:
+
+/home/bartekk/buildroot/output/host/opt/ext-toolchain/bin/../lib/gcc/arm-buildroot-linux-gnueabihf/9.2.0/../../../../arm-buildroot-linux-gnueabihf/bin/ld: warning: library search path "/usr/X11R6/lib" is unsafe for cross-compilation
+
+Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
+---
+ source/Irrlicht/Makefile | 3 +--
+ 1 file changed, 1 insertion(+), 2 deletions(-)
+
+diff --git a/source/Irrlicht/Makefile b/source/Irrlicht/Makefile
+index fed6c7e..b323237 100644
+--- a/source/Irrlicht/Makefile
++++ b/source/Irrlicht/Makefile
+@@ -88,8 +88,7 @@ STATIC_LIB = libIrrlicht.a
+ LIB_PATH = ../../lib/$(SYSTEM)
+ INSTALL_DIR = /usr/local/lib
+ sharedlib install: SHARED_LIB = libIrrlicht.so
+-sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
+-staticlib sharedlib: CXXINCS += -I/usr/X11R6/include
++sharedlib: override LDFLAGS += -lGL -lXxf86vm
+ 
+ #OSX specific options
+ staticlib_osx sharedlib_osx install_osx: SYSTEM = MacOSX
+-- 
+2.27.0
+
diff --git a/package/irrlicht/irrlicht.mk b/package/irrlicht/irrlicht.mk
index bd82815cde..e12d802ee7 100644
--- a/package/irrlicht/irrlicht.mk
+++ b/package/irrlicht/irrlicht.mk
@@ -38,12 +38,14 @@ ifeq ($(BR2_STATIC_LIBS),)
 IRRLICHT_CONF_OPTS += sharedlib
 endif
 
+# set correct path for libraries linking
+IRRLICHT_CONF_OPTS += LDFLAGS="$(TARGET_LDFLAGS) -L$(STAGING_DIR)/usr/lib"
 # Irrlicht fail to detect properly the NEON support on aarch64 or ARM with NEON FPU support.
 # While linking an application with libIrrlicht.so, we get an undefined reference to
 # png_init_filter_functions_neon.
 # Some files are missing in the libpng bundled in Irrlicht, in particular arm/arm_init.c,
 # so disable NEON support completely.
-IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0"
+IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0 -I$(STAGING_DIR)/usr/include/X11"
 
 define IRRLICHT_BUILD_CMDS
 	$(TARGET_MAKE_ENV)
-- 
2.27.0

[Buildroot] [PATCH v2] package/irrlicht: fix libraries linking

[Thomas Petazzoni]

On Mon, 22 Jun 2020 20:45:15 +0200
Bartosz Bilas <b.bilas@grinn-global.com> wrote:

>  .../0003-makefile-override-LDFLAGS.patch      | 29 ++++++++++++++++
>  ...obsolete-X11R6-lib-include-directori.patch | 33 +++++++++++++++++++
>  package/irrlicht/irrlicht.mk                  |  4 ++-
>  3 files changed, 65 insertions(+), 1 deletion(-)
>  create mode 100644 package/irrlicht/0003-makefile-override-LDFLAGS.patch
>  create mode 100644 package/irrlicht/0004-makefile-remove-obsolete-X11R6-lib-include-directori.patch
> 
> diff --git a/package/irrlicht/0003-makefile-override-LDFLAGS.patch b/package/irrlicht/0003-makefile-override-LDFLAGS.patch
> new file mode 100644
> index 0000000000..16c38d5baa
> --- /dev/null
> +++ b/package/irrlicht/0003-makefile-override-LDFLAGS.patch
> @@ -0,0 +1,29 @@
> +From f597ac417f0eefab8f388afe617a0d07d08d87bb Mon Sep 17 00:00:00 2001
> +From: Bartosz Bilas <b.bilas@grinn-global.com>
> +Date: Sun, 21 Jun 2020 12:57:47 +0200
> +Subject: [PATCH] makefile: override LDFLAGS
> +
> +That's needed to fix proper linking to X11 libraries
> +within package makefile.
> +
> +Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
> +---
> + source/Irrlicht/Makefile | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/source/Irrlicht/Makefile b/source/Irrlicht/Makefile
> +index 9a1bdbc..85eb264 100644
> +--- a/source/Irrlicht/Makefile
> ++++ b/source/Irrlicht/Makefile
> +@@ -88,7 +88,7 @@ STATIC_LIB = libIrrlicht.a
> + LIB_PATH = ../../lib/$(SYSTEM)
> + INSTALL_DIR = /usr/local/lib
> + sharedlib install: SHARED_LIB = libIrrlicht.so
> +-sharedlib: LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
> ++sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm

Why here are you changing the LDFLAGS to override LDFLAGS.


> +diff --git a/source/Irrlicht/Makefile b/source/Irrlicht/Makefile
> +index fed6c7e..b323237 100644
> +--- a/source/Irrlicht/Makefile
> ++++ b/source/Irrlicht/Makefile
> +@@ -88,8 +88,7 @@ STATIC_LIB = libIrrlicht.a
> + LIB_PATH = ../../lib/$(SYSTEM)
> + INSTALL_DIR = /usr/local/lib
> + sharedlib install: SHARED_LIB = libIrrlicht.so
> +-sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
> +-staticlib sharedlib: CXXINCS += -I/usr/X11R6/include
> ++sharedlib: override LDFLAGS += -lGL -lXxf86vm

And here you drop entirely the -L argument ?

Or is it two consecutive patches that have been merged upstream ?

> +# set correct path for libraries linking
> +IRRLICHT_CONF_OPTS += LDFLAGS="$(TARGET_LDFLAGS) -L$(STAGING_DIR)/usr/lib"

This should not be necessary, STAGING_DIR/usr/lib is already the
default search path for libraries in the Buildroot cross-compiler.
Could you test without this ?

>  # Irrlicht fail to detect properly the NEON support on aarch64 or ARM with NEON FPU support.
>  # While linking an application with libIrrlicht.so, we get an undefined reference to
>  # png_init_filter_functions_neon.
>  # Some files are missing in the libpng bundled in Irrlicht, in particular arm/arm_init.c,
>  # so disable NEON support completely.
> -IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0"
> +IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0 -I$(STAGING_DIR)/usr/include/X11"

It is a bit weird to have this below the NEON related comment.

Can we change that to:

IRRLICHT_CPPFLAGS = $(TARGET_CPPFLAGS)

# blabla about NEON
IRRLICHT_CPPFLAGS += -DPNG_ARM_NEON_OPT=0

# blabla about X11 includes
IRRLICHT_CPPFLAGS += -I$(STAGING_DIR)/usr/include

IRRLICHT_CONF_OPTS += CPPFLAGS="$(IRRLICHT_CPPFLAGS)"

It is a bit sad that they don't use pkg-config, but oh well, we can
live with this explicit -I flag.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2] package/irrlicht: fix libraries linking

[Bartosz Bilas]

Hello Thomas,

On 22.06.2020 21:26, Thomas Petazzoni wrote:
> On Mon, 22 Jun 2020 20:45:15 +0200
> Bartosz Bilas <b.bilas@grinn-global.com> wrote:
>
>>   .../0003-makefile-override-LDFLAGS.patch      | 29 ++++++++++++++++
>>   ...obsolete-X11R6-lib-include-directori.patch | 33 +++++++++++++++++++
>>   package/irrlicht/irrlicht.mk                  |  4 ++-
>>   3 files changed, 65 insertions(+), 1 deletion(-)
>>   create mode 100644 package/irrlicht/0003-makefile-override-LDFLAGS.patch
>>   create mode 100644 package/irrlicht/0004-makefile-remove-obsolete-X11R6-lib-include-directori.patch
>>
>> diff --git a/package/irrlicht/0003-makefile-override-LDFLAGS.patch b/package/irrlicht/0003-makefile-override-LDFLAGS.patch
>> new file mode 100644
>> index 0000000000..16c38d5baa
>> --- /dev/null
>> +++ b/package/irrlicht/0003-makefile-override-LDFLAGS.patch
>> @@ -0,0 +1,29 @@
>> +From f597ac417f0eefab8f388afe617a0d07d08d87bb Mon Sep 17 00:00:00 2001
>> +From: Bartosz Bilas <b.bilas@grinn-global.com>
>> +Date: Sun, 21 Jun 2020 12:57:47 +0200
>> +Subject: [PATCH] makefile: override LDFLAGS
>> +
>> +That's needed to fix proper linking to X11 libraries
>> +within package makefile.
>> +
>> +Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
>> +---
>> + source/Irrlicht/Makefile | 2 +-
>> + 1 file changed, 1 insertion(+), 1 deletion(-)
>> +
>> +diff --git a/source/Irrlicht/Makefile b/source/Irrlicht/Makefile
>> +index 9a1bdbc..85eb264 100644
>> +--- a/source/Irrlicht/Makefile
>> ++++ b/source/Irrlicht/Makefile
>> +@@ -88,7 +88,7 @@ STATIC_LIB = libIrrlicht.a
>> + LIB_PATH = ../../lib/$(SYSTEM)
>> + INSTALL_DIR = /usr/local/lib
>> + sharedlib install: SHARED_LIB = libIrrlicht.so
>> +-sharedlib: LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
>> ++sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
> Why here are you changing the LDFLAGS to override LDFLAGS.
>
>
>> +diff --git a/source/Irrlicht/Makefile b/source/Irrlicht/Makefile
>> +index fed6c7e..b323237 100644
>> +--- a/source/Irrlicht/Makefile
>> ++++ b/source/Irrlicht/Makefile
>> +@@ -88,8 +88,7 @@ STATIC_LIB = libIrrlicht.a
>> + LIB_PATH = ../../lib/$(SYSTEM)
>> + INSTALL_DIR = /usr/local/lib
>> + sharedlib install: SHARED_LIB = libIrrlicht.so
>> +-sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
>> +-staticlib sharedlib: CXXINCS += -I/usr/X11R6/include
>> ++sharedlib: override LDFLAGS += -lGL -lXxf86vm
> And here you drop entirely the -L argument ?
Due to the fact that it points to the host system libraries. This -L 
argument is set correctly within package makefile.
>
> Or is it two consecutive patches that have been merged upstream ?
No, they have not.
>
>> +# set correct path for libraries linking
>> +IRRLICHT_CONF_OPTS += LDFLAGS="$(TARGET_LDFLAGS) -L$(STAGING_DIR)/usr/lib"
> This should not be necessary, STAGING_DIR/usr/lib is already the
> default search path for libraries in the Buildroot cross-compiler.
> Could you test without this ?
It seems to work quite well without this line.
>
>>   # Irrlicht fail to detect properly the NEON support on aarch64 or ARM with NEON FPU support.
>>   # While linking an application with libIrrlicht.so, we get an undefined reference to
>>   # png_init_filter_functions_neon.
>>   # Some files are missing in the libpng bundled in Irrlicht, in particular arm/arm_init.c,
>>   # so disable NEON support completely.
>> -IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0"
>> +IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0 -I$(STAGING_DIR)/usr/include/X11"
> It is a bit weird to have this below the NEON related comment.
>
> Can we change that to:
>
> IRRLICHT_CPPFLAGS = $(TARGET_CPPFLAGS)
>
> # blabla about NEON
> IRRLICHT_CPPFLAGS += -DPNG_ARM_NEON_OPT=0
>
> # blabla about X11 includes
> IRRLICHT_CPPFLAGS += -I$(STAGING_DIR)/usr/include
>
> IRRLICHT_CONF_OPTS += CPPFLAGS="$(IRRLICHT_CPPFLAGS)"
Good hint, I was thinking about a bit different approach but it wasn't 
working as I expected therefore I did it as it's above.
> It is a bit sad that they don't use pkg-config, but oh well, we can 
> live with this explicit -I flag. Thanks! Thomas 
Best
Bartek

[Buildroot] [PATCH v2] package/irrlicht: fix libraries linking

[Thomas Petazzoni]

On Mon, 22 Jun 2020 22:23:35 +0200
Bartosz Bilas <b.bilas@grinn-global.com> wrote:

> >> +-sharedlib: LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
> >> ++sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm  
> > Why here are you changing the LDFLAGS to override LDFLAGS.
> >
> >  
> >> +diff --git a/source/Irrlicht/Makefile b/source/Irrlicht/Makefile
> >> +index fed6c7e..b323237 100644
> >> +--- a/source/Irrlicht/Makefile
> >> ++++ b/source/Irrlicht/Makefile
> >> +@@ -88,8 +88,7 @@ STATIC_LIB = libIrrlicht.a
> >> + LIB_PATH = ../../lib/$(SYSTEM)
> >> + INSTALL_DIR = /usr/local/lib
> >> + sharedlib install: SHARED_LIB = libIrrlicht.so
> >> +-sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
> >> +-staticlib sharedlib: CXXINCS += -I/usr/X11R6/include
> >> ++sharedlib: override LDFLAGS += -lGL -lXxf86vm  
> > And here you drop entirely the -L argument ?  
> Due to the fact that it points to the host system libraries. This -L 
> argument is set correctly within package makefile.

Yes, of course. My question is why do you have two patches? They touch
exactly the same line, simply do the change in two steps instead of
one. Am I missing something here ?


> >> +# set correct path for libraries linking
> >> +IRRLICHT_CONF_OPTS += LDFLAGS="$(TARGET_LDFLAGS) -L$(STAGING_DIR)/usr/lib"  
> > This should not be necessary, STAGING_DIR/usr/lib is already the
> > default search path for libraries in the Buildroot cross-compiler.
> > Could you test without this ?  
> It seems to work quite well without this line.

Yes, as I said, -L$(STAGING_DIR)/usr/lib is useless.

> >  
> >>   # Irrlicht fail to detect properly the NEON support on aarch64 or ARM with NEON FPU support.
> >>   # While linking an application with libIrrlicht.so, we get an undefined reference to
> >>   # png_init_filter_functions_neon.
> >>   # Some files are missing in the libpng bundled in Irrlicht, in particular arm/arm_init.c,
> >>   # so disable NEON support completely.
> >> -IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0"
> >> +IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0 -I$(STAGING_DIR)/usr/include/X11"  
> > It is a bit weird to have this below the NEON related comment.
> >
> > Can we change that to:
> >
> > IRRLICHT_CPPFLAGS = $(TARGET_CPPFLAGS)
> >
> > # blabla about NEON
> > IRRLICHT_CPPFLAGS += -DPNG_ARM_NEON_OPT=0
> >
> > # blabla about X11 includes
> > IRRLICHT_CPPFLAGS += -I$(STAGING_DIR)/usr/include
> >
> > IRRLICHT_CONF_OPTS += CPPFLAGS="$(IRRLICHT_CPPFLAGS)"  
> Good hint, I was thinking about a bit different approach but it wasn't 
> working as I expected therefore I did it as it's above.

So I guess you'll rework accordingly? If needed, you can also have a
preparation patch that reworks this IRRLICHT_CPPFLAGS thing for the
NEON stuff, and then another patch that fixes the library linking issue.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2] package/irrlicht: fix libraries linking

[Bartosz Bilas]

Hello Thomas,

On 22.06.2020 22:36, Thomas Petazzoni wrote:
> On Mon, 22 Jun 2020 22:23:35 +0200
> Bartosz Bilas <b.bilas@grinn-global.com> wrote:
>
>>>> +-sharedlib: LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
>>>> ++sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
>>> Why here are you changing the LDFLAGS to override LDFLAGS.
>>>
>>>   
>>>> +diff --git a/source/Irrlicht/Makefile b/source/Irrlicht/Makefile
>>>> +index fed6c7e..b323237 100644
>>>> +--- a/source/Irrlicht/Makefile
>>>> ++++ b/source/Irrlicht/Makefile
>>>> +@@ -88,8 +88,7 @@ STATIC_LIB = libIrrlicht.a
>>>> + LIB_PATH = ../../lib/$(SYSTEM)
>>>> + INSTALL_DIR = /usr/local/lib
>>>> + sharedlib install: SHARED_LIB = libIrrlicht.so
>>>> +-sharedlib: override LDFLAGS += -L/usr/X11R6/lib$(LIBSELECT) -lGL -lXxf86vm
>>>> +-staticlib sharedlib: CXXINCS += -I/usr/X11R6/include
>>>> ++sharedlib: override LDFLAGS += -lGL -lXxf86vm
>>> And here you drop entirely the -L argument ?
>> Due to the fact that it points to the host system libraries. This -L
>> argument is set correctly within package makefile.
> Yes, of course. My question is why do you have two patches? They touch
> exactly the same line, simply do the change in two steps instead of
> one. Am I missing something here ?
Ah, I didn't want to combine everything into one patch because there 
were 2 different changes (adding keyword and removing paths) but I'm 
gonna combine it into one patch then.
>
>>>> +# set correct path for libraries linking
>>>> +IRRLICHT_CONF_OPTS += LDFLAGS="$(TARGET_LDFLAGS) -L$(STAGING_DIR)/usr/lib"
>>> This should not be necessary, STAGING_DIR/usr/lib is already the
>>> default search path for libraries in the Buildroot cross-compiler.
>>> Could you test without this ?
>> It seems to work quite well without this line.
> Yes, as I said, -L$(STAGING_DIR)/usr/lib is useless.
>
>>>   
>>>>    # Irrlicht fail to detect properly the NEON support on aarch64 or ARM with NEON FPU support.
>>>>    # While linking an application with libIrrlicht.so, we get an undefined reference to
>>>>    # png_init_filter_functions_neon.
>>>>    # Some files are missing in the libpng bundled in Irrlicht, in particular arm/arm_init.c,
>>>>    # so disable NEON support completely.
>>>> -IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0"
>>>> +IRRLICHT_CONF_OPTS += CPPFLAGS="$(TARGET_CPPFLAGS) -DPNG_ARM_NEON_OPT=0 -I$(STAGING_DIR)/usr/include/X11"
>>> It is a bit weird to have this below the NEON related comment.
>>>
>>> Can we change that to:
>>>
>>> IRRLICHT_CPPFLAGS = $(TARGET_CPPFLAGS)
>>>
>>> # blabla about NEON
>>> IRRLICHT_CPPFLAGS += -DPNG_ARM_NEON_OPT=0
>>>
>>> # blabla about X11 includes
>>> IRRLICHT_CPPFLAGS += -I$(STAGING_DIR)/usr/include
>>>
>>> IRRLICHT_CONF_OPTS += CPPFLAGS="$(IRRLICHT_CPPFLAGS)"
>> Good hint, I was thinking about a bit different approach but it wasn't
>> working as I expected therefore I did it as it's above.
> So I guess you'll rework accordingly? If needed, you can also have a
> preparation patch that reworks this IRRLICHT_CPPFLAGS thing for the
> NEON stuff, and then another patch that fixes the library linking issue.
>
> Thomas

It turned out that -I$(STAGING_DIR)/usr/include isn't necessary anymore 
so I've decided not to touch that.

Best
Bartek

[Buildroot] [PATCH v2] package/irrlicht: fix libraries linking

[Thomas Petazzoni]

On Tue, 23 Jun 2020 20:08:47 +0200
Bartosz Bilas <b.bilas@grinn-global.com> wrote:

> >> Due to the fact that it points to the host system libraries. This -L
> >> argument is set correctly within package makefile.  
> > Yes, of course. My question is why do you have two patches? They touch
> > exactly the same line, simply do the change in two steps instead of
> > one. Am I missing something here ?  
> Ah, I didn't want to combine everything into one patch because there 
> were 2 different changes (adding keyword and removing paths) but I'm 
> gonna combine it into one patch then.

OK, it's true you can see it as: one allows to override the LDFLAGS,
the other drops a bogus hardcoded path.


> >>> IRRLICHT_CONF_OPTS += CPPFLAGS="$(IRRLICHT_CPPFLAGS)"  
> >> Good hint, I was thinking about a bit different approach but it wasn't
> >> working as I expected therefore I did it as it's above.  
> > So I guess you'll rework accordingly? If needed, you can also have a
> > preparation patch that reworks this IRRLICHT_CPPFLAGS thing for the
> > NEON stuff, and then another patch that fixes the library linking issue.
> >
> > Thomas  
> 
> It turned out that -I$(STAGING_DIR)/usr/include isn't necessary anymore 
> so I've decided not to touch that.

-I$(STAGING_DIR)/usr/include is definitely useless as it's the default
search path for headers for the Buildroot cross-compiler, but that's
not what you were adding: you were adding
-I$(STAGING_DIR)/usr/include/X11R6, which is not in the default search
path for headers.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2] package/irrlicht: fix libraries linking

[Bartosz Biłas]

Hi Thomas,

On 6/24/20 10:05 AM, Thomas Petazzoni wrote:
> On Tue, 23 Jun 2020 20:08:47 +0200
> Bartosz Bilas <b.bilas@grinn-global.com> wrote:
>
>>>> Due to the fact that it points to the host system libraries. This -L
>>>> argument is set correctly within package makefile.
>>> Yes, of course. My question is why do you have two patches? They touch
>>> exactly the same line, simply do the change in two steps instead of
>>> one. Am I missing something here ?
>> Ah, I didn't want to combine everything into one patch because there
>> were 2 different changes (adding keyword and removing paths) but I'm
>> gonna combine it into one patch then.
> OK, it's true you can see it as: one allows to override the LDFLAGS,
> the other drops a bogus hardcoded path.
>
>
>>>>> IRRLICHT_CONF_OPTS += CPPFLAGS="$(IRRLICHT_CPPFLAGS)"
>>>> Good hint, I was thinking about a bit different approach but it wasn't
>>>> working as I expected therefore I did it as it's above.
>>> So I guess you'll rework accordingly? If needed, you can also have a
>>> preparation patch that reworks this IRRLICHT_CPPFLAGS thing for the
>>> NEON stuff, and then another patch that fixes the library linking issue.
>>>
>>> Thomas
>> It turned out that -I$(STAGING_DIR)/usr/include isn't necessary anymore
>> so I've decided not to touch that.
> -I$(STAGING_DIR)/usr/include is definitely useless as it's the default 
> search path for headers for the Buildroot cross-compiler, but that's 
> not what you were adding: you were adding 
> -I$(STAGING_DIR)/usr/include/X11R6, which is not in the default search 
> path for headers. Best regards, Thomas
No, previously I was adding -I$(STAGING_DIR)/usr/include/X11 but this 
path wasn't necessary therefore in the v3 there is no this path anymore.
This -I$(STAGING_DIR)/usr/include/X11R6 path was the default one which 
I've removed from the makefile.

I hope that everything is clear now ;)

Best
Bartek