Skip to contentSkip to conversation

sdl2: new package

PATCH New Guillaume GARDET 7 participants 26 messages Feb 26, 2015 – Oct 21, 2015

lore
Share & tools
lore

Guillaume GARDET

author
#0 lore raw

PATCH 1/1 sdl2: new package

Signed-off-by Guillaume GARDET <[email protected]>
---
 package/Config.in      |  1 +
 package/sdl2/Config.in | 22 +++++++++++++++++
 package/sdl2/sdl2.mk   | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 package/sdl2/Config.in
 create mode 100644 package/sdl2/sdl2.mk
package/Config.in +1 -0
index fe3d3d0..ad4d248 100644
@@ -215,6 +215,7 @@ endif
 	source "package/psplash/Config.in"
 	source "package/sawman/Config.in"
 	source "package/sdl/Config.in"
+	source "package/sdl2/Config.in"
 	source "package/sdl_gfx/Config.in"
 	source "package/sdl_image/Config.in"
 	source "package/sdl_mixer/Config.in"
package/sdl2/Config.in new file +22 -0
new file mode 100644
index 0000000..5d0fe01
@@ -0,0 +1,22 @@
+config BR2_PACKAGE_SDL2
+	bool "SDL2"
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/
+
+if BR2_PACKAGE_SDL2
+
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "SDL2 DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB
+
+config BR2_PACKAGE_SDL2_X11
+	bool "SDL2 X11 video driver"
+	depends on BR2_PACKAGE_XORG7
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXEXT
+
+endif
package/sdl2/sdl2.mk new file +67 -0

67 lines not on this page · raw message

Signature · 1 line
1.8.4.5
author
#1 lore raw

PATCH 1/1 sdl2: new package

Ping...

Guillaume

Le 26/02/2015 10:47, Guillaume GARDET a ?crit :

Quoted 123 lines
Signed-off-by Guillaume GARDET <[email protected]>
---
  package/Config.in      |  1 +
  package/sdl2/Config.in | 22 +++++++++++++++++
  package/sdl2/sdl2.mk   | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 90 insertions(+)

create mode 100644 package/sdl2/Config.in
create mode 100644 package/sdl2/sdl2.mk

package/Config.in +1 -0
index fe3d3d0..ad4d248 100644
@@ -215,6 +215,7 @@ endif
  	source "package/psplash/Config.in"
  	source "package/sawman/Config.in"
  	source "package/sdl/Config.in"
+	source "package/sdl2/Config.in"
  	source "package/sdl_gfx/Config.in"
  	source "package/sdl_image/Config.in"
  	source "package/sdl_mixer/Config.in"
package/sdl2/Config.in new file +22 -0
new file mode 100644
index 0000000..5d0fe01
@@ -0,0 +1,22 @@
+config BR2_PACKAGE_SDL2
+	bool "SDL2"
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/
+
+if BR2_PACKAGE_SDL2
+
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "SDL2 DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB
+
+config BR2_PACKAGE_SDL2_X11
+	bool "SDL2 X11 video driver"
+	depends on BR2_PACKAGE_XORG7
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXEXT
+
+endif
package/sdl2/sdl2.mk new file +67 -0
new file mode 100644
index 0000000..b9aa907
@@ -0,0 +1,67 @@
+################################################################################
+#
+# sdl2
+#
+################################################################################
+
+SDL2_VERSION = 2.0.3
+SDL2_SOURCE = SDL2-$(SDL2_VERSION).tar.gz
+SDL2_SITE = http://www.libsdl.org/release
+SDL2_LICENSE = zlib
+SDL2_LICENSE_FILES = COPYING
+SDL2_INSTALL_STAGING = YES
+
+# We must enable static build to get compilation successful.
+SDL2_CONF_OPTS = --enable-static
+
+ifeq ($(BR2_PACKAGE_SDL2_DIRECTFB),y)
+SDL2_DEPENDENCIES += directfb
+SDL2_CONF_OPTS += --enable-video-directfb=yes
+SDL2_CONF_ENV = ac_cv_path_DIRECTFBCONFIG=$(STAGING_DIR)/usr/bin/directfb-config
+else
+SDL2_CONF_OPTS += --enable-video-directfb=no
+endif
+
+ifeq ($(BR2_PACKAGE_SDL2_X11),y)
+SDL2_CONF_OPTS += --enable-video-x11=yes
+SDL2_DEPENDENCIES += \
+	xlib_libX11 xlib_libXext \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER), xlib_libXrender) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR), xlib_libXrandr)
+else
+SDL2_CONF_OPTS += --enable-video-x11=no
+endif
+
+ifeq ($(BR2_PACKAGE_TSLIB),y)
+SDL2_DEPENDENCIES += tslib
+endif
+
+ifeq ($(BR2_PACKAGE_ALSA_LIB),y)
+SDL2_DEPENDENCIES += alsa-lib
+endif
+
+ifeq ($(BR2_PACKAGE_MESA3D),y)
+SDL_DEPENDENCIES += mesa3d
+endif
+
+SDL2_CONF_OPTS += \
+	--enable-pulseaudio=no \
+	--disable-arts \
+	--disable-esd
+
+HOST_SDL2_CONF_OPTS += \
+	--enable-pulseaudio=no \
+	--enable-video-x11=no \
+	--disable-arts \
+	--disable-esd
+
+# Remove the -Wl,-rpath option.
+define SDL2_FIXUP_SDL2_CONFIG
+	$(SED) 's%-Wl,-rpath,\$${libdir}%%' \
+		$(STAGING_DIR)/usr/bin/sdl2-config
+endef
+
+SDL2_POST_INSTALL_STAGING_HOOKS += SDL2_FIXUP_SDL2_CONFIG
+
+$(eval $(autotools-package))
+$(eval $(host-autotools-package))
#2 lore raw

PATCH 1/1 sdl2: new package

In reply to Guillaume GARDET

Dear Guillaume GARDET,

Sorry for not replying earlier. I had a look at your patch some time ago and had some comments, but apparently never replied.

package/sdl2/Config.in line 2

Quoted 8 lines · Guillaume GARDET
package/sdl2/Config.in new file +2 -0
new file mode 100644
index 0000000..5d0fe01
@@ -0,0 +1,22 @@
+config BR2_PACKAGE_SDL2
+	bool "SDL2"

I know it's not the case for the existing SDL package, but we generally prefer to have package names in lower-case in menuconfig, for consistency. So s/SDL2/sdl2/ here.

package/sdl2/Config.in line 8

Quoted 6 lines
package/sdl2/Config.in new file +6 -0
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/

Did you check with very basic toolchains that SDL2 builds fine? I'm surprised you have really zero dependency on toolchain features. Can you check for example with the following configurations:

http://autobuild.buildroot.org/toolchains/configs/br-arm-basic.config
http://autobuild.buildroot.org/toolchains/configs/br-arm-full-nothread.config
http://autobuild.buildroot.org/toolchains/configs/bfin-linux-uclibc.config
http://autobuild.buildroot.org/toolchains/configs/bfin-uclinux.config

You may discover that sdl2 depends on certain toolchain options.

package/sdl2/Config.in line 14

Quoted 3 lines
package/sdl2/Config.in new file +3 -0
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "SDL2 DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB

No need to repeat "SDL2" here, since this option is indented below the "sdl2" main option.

Quoted 11 lines
package/sdl2/sdl2.mk new file +5 -0
new file mode 100644
index 0000000..b9aa907
@@ -0,0 +1,67 @@
+################################################################################
+#
+# sdl2
+#
+################################################################################

package/sdl2/sdl2.mk line 15

Quoted 10 lines
package/sdl2/sdl2.mk new file +10 -0
+
+SDL2_VERSION = 2.0.3
+SDL2_SOURCE = SDL2-$(SDL2_VERSION).tar.gz
+SDL2_SITE = http://www.libsdl.org/release
+SDL2_LICENSE = zlib
+SDL2_LICENSE_FILES = COPYING
+SDL2_INSTALL_STAGING = YES
+
+# We must enable static build to get compilation successful.
+SDL2_CONF_OPTS = --enable-static

Hum, this is a bit annoying, but OK.

Quoted 5 lines
package/sdl2/sdl2.mk new file +5 -0
+
+ifeq ($(BR2_PACKAGE_SDL2_DIRECTFB),y)
+SDL2_DEPENDENCIES += directfb
+SDL2_CONF_OPTS += --enable-video-directfb=yes
+SDL2_CONF_ENV = ac_cv_path_DIRECTFBCONFIG=$(STAGING_DIR)/usr/bin/directfb-config

package/sdl2/sdl2.mk line 30

Quoted 10 lines
package/sdl2/sdl2.mk new file +10 -0
+else
+SDL2_CONF_OPTS += --enable-video-directfb=no
+endif
+
+ifeq ($(BR2_PACKAGE_SDL2_X11),y)
+SDL2_CONF_OPTS += --enable-video-x11=yes
+SDL2_DEPENDENCIES += \
+	xlib_libX11 xlib_libXext \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER), xlib_libXrender) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR), xlib_libXrandr)

No space after the comma.

package/sdl2/sdl2.mk line 37

Quoted 7 lines
package/sdl2/sdl2.mk new file +7 -0
+else
+SDL2_CONF_OPTS += --enable-video-x11=no
+endif
+
+ifeq ($(BR2_PACKAGE_TSLIB),y)
+SDL2_DEPENDENCIES += tslib
+endif

Please explicit the --enable-input-tslib / --disable-input-tslib.

package/sdl2/sdl2.mk line 41

Quoted 3 lines
package/sdl2/sdl2.mk new file +3 -0
+ifeq ($(BR2_PACKAGE_ALSA_LIB),y)
+SDL2_DEPENDENCIES += alsa-lib
+endif

Same: --enable-alsa / --disable-alsa.

package/sdl2/sdl2.mk line 45

Quoted 4 lines
package/sdl2/sdl2.mk new file +4 -0
+
+ifeq ($(BR2_PACKAGE_MESA3D),y)
+SDL_DEPENDENCIES += mesa3d
+endif

What feature is this supposed to be enabling? mesa3d is an OpenGL implementation, but it's not the only one. So if it's OpenGL related, most likely this is not the right thing to do.

package/sdl2/sdl2.mk line 50

Quoted 4 lines
package/sdl2/sdl2.mk new file +4 -0
+SDL2_CONF_OPTS += \
+	--enable-pulseaudio=no \
+	--disable-arts \
+	--disable-esd

Please put this earlier, where the first non-conditional SDL2_CONF_OPTS definition is.

package/sdl2/sdl2.mk line 56

Quoted 5 lines
package/sdl2/sdl2.mk new file +5 -0
+HOST_SDL2_CONF_OPTS += \
+	--enable-pulseaudio=no \
+	--enable-video-x11=no \
+	--disable-arts \
+	--disable-esd

Not needed I believe.

package/sdl2/sdl2.mk line 67

Quoted 11 lines
package/sdl2/sdl2.mk new file +11 -0
+
+# Remove the -Wl,-rpath option.
+define SDL2_FIXUP_SDL2_CONFIG
+	$(SED) 's%-Wl,-rpath,\$${libdir}%%' \
+		$(STAGING_DIR)/usr/bin/sdl2-config
+endef
+
+SDL2_POST_INSTALL_STAGING_HOOKS += SDL2_FIXUP_SDL2_CONFIG
+
+$(eval $(autotools-package))
+$(eval $(host-autotools-package))

Why do you have a host package? There is nothing that depends on it.

Could you look at those comments, and resubmit an updated version and/or comment on them as needed?

Thanks a lot!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#3 lore raw

PATCH 1/1 sdl2: new package

Hi,

Le 13/03/2015 17:14, Thomas Petazzoni a ?crit :

Quoted 7 lines

Dear Guillaume GARDET,

Sorry for not replying earlier. I had a look at your patch some time ago and had some comments, but apparently never replied.

On Thu, 26 Feb 2015 10:47:51 +0100, Guillaume GARDET wrote:

Quoted 11 lines

package/sdl2/Config.in line 2

Quoted 8 lines
package/sdl2/Config.in new file +2 -0
new file mode 100644
index 0000000..5d0fe01
@@ -0,0 +1,22 @@
+config BR2_PACKAGE_SDL2
+	bool "SDL2"

I know it's not the case for the existing SDL package, but we generally prefer to have package names in lower-case in menuconfig, for consistency. So s/SDL2/sdl2/ here.

ok

Quoted 11 lines

package/sdl2/Config.in line 8

Quoted 7 lines
package/sdl2/Config.in new file +6 -0
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/

Did you check with very basic toolchains that SDL2 builds fine? I'm surprised you have really zero dependency on toolchain features. Can you check for example with the following configurations:

Quoted 6 lines

http://autobuild.buildroot.org/toolchains/configs/br-arm-basic.config
http://autobuild.buildroot.org/toolchains/configs/br-arm-full-nothread.config
http://autobuild.buildroot.org/toolchains/configs/bfin-linux-uclibc.config
http://autobuild.buildroot.org/toolchains/configs/bfin-uclinux.config

You may discover that sdl2 depends on certain toolchain options.

I used sdl (1) as a template which has no wuch deps, but I will test.

Quoted 6 lines

package/sdl2/Config.in line 14

Quoted 4 lines
package/sdl2/Config.in new file +3 -0
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "SDL2 DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB

No need to repeat "SDL2" here, since this option is indented below the "sdl2" main option.

ok

Quoted 24 lines
Quoted 12 lines
package/sdl2/sdl2.mk new file +5 -0
new file mode 100644
index 0000000..b9aa907
@@ -0,0 +1,67 @@
+################################################################################
+#
+# sdl2
+#
+################################################################################

package/sdl2/sdl2.mk line 15

Quoted 10 lines
package/sdl2/sdl2.mk new file +10 -0
+
+SDL2_VERSION = 2.0.3
+SDL2_SOURCE = SDL2-$(SDL2_VERSION).tar.gz
+SDL2_SITE = http://www.libsdl.org/release
+SDL2_LICENSE = zlib
+SDL2_LICENSE_FILES = COPYING
+SDL2_INSTALL_STAGING = YES
+
+# We must enable static build to get compilation successful.
+SDL2_CONF_OPTS = --enable-static

Hum, this is a bit annoying, but OK.

Quoted 16 lines
Quoted 5 lines
package/sdl2/sdl2.mk new file +5 -0
+
+ifeq ($(BR2_PACKAGE_SDL2_DIRECTFB),y)
+SDL2_DEPENDENCIES += directfb
+SDL2_CONF_OPTS += --enable-video-directfb=yes
+SDL2_CONF_ENV = ac_cv_path_DIRECTFBCONFIG=$(STAGING_DIR)/usr/bin/directfb-config

package/sdl2/sdl2.mk line 30

Quoted 10 lines
package/sdl2/sdl2.mk new file +10 -0
+else
+SDL2_CONF_OPTS += --enable-video-directfb=no
+endif
+
+ifeq ($(BR2_PACKAGE_SDL2_X11),y)
+SDL2_CONF_OPTS += --enable-video-x11=yes
+SDL2_DEPENDENCIES += \
+	xlib_libX11 xlib_libXext \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER), xlib_libXrender) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR), xlib_libXrandr)

No space after the comma.

ok

Quoted 9 lines

package/sdl2/sdl2.mk line 37

Quoted 8 lines
package/sdl2/sdl2.mk new file +7 -0
+else
+SDL2_CONF_OPTS += --enable-video-x11=no
+endif
+
+ifeq ($(BR2_PACKAGE_TSLIB),y)
+SDL2_DEPENDENCIES += tslib
+endif

Please explicit the --enable-input-tslib / --disable-input-tslib.

ok

Quoted 5 lines

package/sdl2/sdl2.mk line 41

Quoted 4 lines
package/sdl2/sdl2.mk new file +3 -0
+ifeq ($(BR2_PACKAGE_ALSA_LIB),y)
+SDL2_DEPENDENCIES += alsa-lib
+endif

Same: --enable-alsa / --disable-alsa.

ok

Quoted 8 lines

package/sdl2/sdl2.mk line 45

Quoted 5 lines
package/sdl2/sdl2.mk new file +4 -0
+
+ifeq ($(BR2_PACKAGE_MESA3D),y)
+SDL_DEPENDENCIES += mesa3d
+endif

What feature is this supposed to be enabling? mesa3d is an OpenGL implementation, but it's not the only one. So if it's OpenGL related, most likely this is not the right thing to do.

Here, I also used sdl (1) package as a template. So, sdl (1) should be fixed too.

Quoted 7 lines

package/sdl2/sdl2.mk line 50

Quoted 5 lines
package/sdl2/sdl2.mk new file +4 -0
+SDL2_CONF_OPTS += \
+	--enable-pulseaudio=no \
+	--disable-arts \
+	--disable-esd

Please put this earlier, where the first non-conditional SDL2_CONF_OPTS definition is.

ok

Quoted 7 lines

package/sdl2/sdl2.mk line 56

Quoted 6 lines
package/sdl2/sdl2.mk new file +5 -0
+HOST_SDL2_CONF_OPTS += \
+	--enable-pulseaudio=no \
+	--enable-video-x11=no \
+	--disable-arts \
+	--disable-esd

Not needed I believe.

ok. sdl1 has it too.

Quoted 13 lines

package/sdl2/sdl2.mk line 67

Quoted 12 lines
package/sdl2/sdl2.mk new file +11 -0
+
+# Remove the -Wl,-rpath option.
+define SDL2_FIXUP_SDL2_CONFIG
+	$(SED) 's%-Wl,-rpath,\$${libdir}%%' \
+		$(STAGING_DIR)/usr/bin/sdl2-config
+endef
+
+SDL2_POST_INSTALL_STAGING_HOOKS += SDL2_FIXUP_SDL2_CONFIG
+
+$(eval $(autotools-package))
+$(eval $(host-autotools-package))

Why do you have a host package? There is nothing that depends on it.

Indeed. I used sdl1 has a template as said earlier. :(

Quoted 3 lines

Could you look at those comments, and resubmit an updated version and/or comment on them as needed?

Will send a V2.

Guillaume

author
#4 lore raw

PATCH 1/1 sdl2: new package

In reply to Thomas Petazzoni

Hi,

Le 13/03/2015 17:14, Thomas Petazzoni a ?crit :

Quoted 10 lines

package/sdl2/Config.in line 8

Quoted 6 lines
package/sdl2/Config.in new file +6 -0
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/

Did you check with very basic toolchains that SDL2 builds fine? I'm surprised you have really zero dependency on toolchain features. Can you check for example with the following configurations:

Quoted 3 lines

http://autobuild.buildroot.org/toolchains/configs/br-arm-basic.config
http://autobuild.buildroot.org/toolchains/configs/br-arm-full-nothread.config
http://autobuild.buildroot.org/toolchains/configs/bfin-linux-uclibc.config

The 3 are ok.

Quoted 1 line

http://autobuild.buildroot.org/toolchains/configs/bfin-uclinux.config

This one fails with:
"dlfcn.h: No such file or directory"

WHat would be the right "dpends on" option?

Guillaume

#5 lore raw

PATCH 1/1 sdl2: new package

Dear Guillaume GARDET - Olis?o,

Quoted 6 lines · Guillaume GARDET - Olis?o
Quoted 1 line

http://autobuild.buildroot.org/toolchains/configs/bfin-uclinux.config

This one fails with:
"dlfcn.h: No such file or directory"

WHat would be the right "dpends on" option?

depends on !BR2_STATIC_LIBS

and the appropriate comment. See section "17.2.2. Dependencies on target and toolchain options" in the Buildroot manual.

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#6 lore raw

PATCH v2 1/1 sdl2: new package

Changes in V2:
* Fix SDL2 case: s/SDL2/sdl2/
* Add dependency on toolchain with dynamic library
* Remove unneeded 'SDL2' in sub-options names
* Remove unneeded space after comma in if statement
* Add explicit enable/disable options for tslib and alsa
* Remove Mesa3D optional part
* Move unconditionnal SDL2_CONF_OPTS upper
* Remove host build

Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 package/Config.in      |  1 +
 package/sdl2/Config.in | 26 +++++++++++++++++++++
 package/sdl2/sdl2.mk   | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+)
 create mode 100644 package/sdl2/Config.in
 create mode 100644 package/sdl2/sdl2.mk
package/Config.in +1 -0
index 566a78a..cf4949a 100644
@@ -219,6 +219,7 @@ endif
 	source "package/psplash/Config.in"
 	source "package/sawman/Config.in"
 	source "package/sdl/Config.in"
+	source "package/sdl2/Config.in"
 	source "package/sdl_gfx/Config.in"
 	source "package/sdl_image/Config.in"
 	source "package/sdl_mixer/Config.in"
package/sdl2/Config.in new file +26 -0
new file mode 100644
index 0000000..18e4f75
@@ -0,0 +1,26 @@
+config BR2_PACKAGE_SDL2
+	depends on !BR2_STATIC_LIBS
+	bool "sdl2"
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/
+
+comment "sdl2 needs a toolchain w/ dynamic library"
+        depends on BR2_STATIC_LIBS
+
+if BR2_PACKAGE_SDL2
+
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB
+
+config BR2_PACKAGE_SDL2_X11
+	bool "X11 video driver"
+	depends on BR2_PACKAGE_XORG7
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXEXT
+
+endif
package/sdl2/sdl2.mk new file +62 -0

62 lines not on this page · raw message

Signature · 1 line
1.8.4.5
author
#7 lore raw

PATCH v2 1/1 sdl2: new package

Ping.

Guillaume

Le 30/03/2015 15:14, Guillaume GARDET a ?crit :

Quoted 133 lines

Changes in V2:
* Fix SDL2 case: s/SDL2/sdl2/
* Add dependency on toolchain with dynamic library
* Remove unneeded 'SDL2' in sub-options names
* Remove unneeded space after comma in if statement
* Add explicit enable/disable options for tslib and alsa
* Remove Mesa3D optional part
* Move unconditionnal SDL2_CONF_OPTS upper
* Remove host build

Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
  package/Config.in      |  1 +
  package/sdl2/Config.in | 26 +++++++++++++++++++++
  package/sdl2/sdl2.mk   | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 89 insertions(+)

create mode 100644 package/sdl2/Config.in
create mode 100644 package/sdl2/sdl2.mk

package/Config.in +1 -0
index 566a78a..cf4949a 100644
@@ -219,6 +219,7 @@ endif
  	source "package/psplash/Config.in"
  	source "package/sawman/Config.in"
  	source "package/sdl/Config.in"
+	source "package/sdl2/Config.in"
  	source "package/sdl_gfx/Config.in"
  	source "package/sdl_image/Config.in"
  	source "package/sdl_mixer/Config.in"
package/sdl2/Config.in new file +26 -0
new file mode 100644
index 0000000..18e4f75
@@ -0,0 +1,26 @@
+config BR2_PACKAGE_SDL2
+	depends on !BR2_STATIC_LIBS
+	bool "sdl2"
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/
+
+comment "sdl2 needs a toolchain w/ dynamic library"
+        depends on BR2_STATIC_LIBS
+
+if BR2_PACKAGE_SDL2
+
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB
+
+config BR2_PACKAGE_SDL2_X11
+	bool "X11 video driver"
+	depends on BR2_PACKAGE_XORG7
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXEXT
+
+endif
package/sdl2/sdl2.mk new file +62 -0
new file mode 100644
index 0000000..6a3caaf
@@ -0,0 +1,62 @@
+################################################################################
+#
+# sdl2
+#
+################################################################################
+
+SDL2_VERSION = 2.0.3
+SDL2_SOURCE = SDL2-$(SDL2_VERSION).tar.gz
+SDL2_SITE = http://www.libsdl.org/release
+SDL2_LICENSE = zlib
+SDL2_LICENSE_FILES = COPYING
+SDL2_INSTALL_STAGING = YES
+
+SDL2_CONF_OPTS += \
+	--enable-pulseaudio=no \
+	--disable-arts \
+	--disable-esd
+
+# We must enable static build to get compilation succesful.
+SDL2_CONF_OPTS += --enable-static
+
+ifeq ($(BR2_PACKAGE_SDL2_DIRECTFB),y)
+SDL2_DEPENDENCIES += directfb
+SDL2_CONF_OPTS += --enable-video-directfb=yes
+SDL2_CONF_ENV = ac_cv_path_DIRECTFBCONFIG=$(STAGING_DIR)/usr/bin/directfb-config
+else
+SDL2_CONF_OPTS += --enable-video-directfb=no
+endif
+
+ifeq ($(BR2_PACKAGE_SDL2_X11),y)
+SDL2_CONF_OPTS += --enable-video-x11=yes
+SDL2_DEPENDENCIES += \
+	xlib_libX11 xlib_libXext \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER),xlib_libXrender) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR),xlib_libXrandr)
+else
+SDL2_CONF_OPTS += --enable-video-x11=no
+endif
+
+ifeq ($(BR2_PACKAGE_TSLIB),y)
+SDL2_DEPENDENCIES += tslib
+SDL2_CONF_OPTS += --enable-input-tslib
+else
+SDL2_CONF_OPTS += --disable-input-tslib
+endif
+
+ifeq ($(BR2_PACKAGE_ALSA_LIB),y)
+SDL2_DEPENDENCIES += alsa-lib
+SDL2_CONF_OPTS += --enable-alsa
+else
+SDL2_CONF_OPTS += --disable-alsa
+endif
+
+# Remove the -Wl,-rpath option.
+define SDL2_FIXUP_SDL2_CONFIG
+	$(SED) 's%-Wl,-rpath,\$${libdir}%%' \
+		$(STAGING_DIR)/usr/bin/sdl2-config
+endef
+
+SDL2_POST_INSTALL_STAGING_HOOKS += SDL2_FIXUP_SDL2_CONFIG
+
+$(eval $(autotools-package))
#8 lore raw

PATCH v2 1/1 sdl2: new package

In reply to Guillaume GARDET

Hi Guillaume,

Thanks for your patch, I have some comments

Le 30/03/2015 15:14, Guillaume GARDET a ?crit :

Quoted 10 lines

Changes in V2:
* Fix SDL2 case: s/SDL2/sdl2/
* Add dependency on toolchain with dynamic library
* Remove unneeded 'SDL2' in sub-options names
* Remove unneeded space after comma in if statement
* Add explicit enable/disable options for tslib and alsa
* Remove Mesa3D optional part
* Move unconditionnal SDL2_CONF_OPTS upper
* Remove host build

The history of the patch changes must be bellow "---" line ...

Quoted 4 lines
Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>

---

here:
Changes in V2:

Quoted 33 lines
 package/Config.in      |  1 +
 package/sdl2/Config.in | 26 +++++++++++++++++++++
 package/sdl2/sdl2.mk   | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+)
 create mode 100644 package/sdl2/Config.in
 create mode 100644 package/sdl2/sdl2.mk
package/Config.in +1 -0
index 566a78a..cf4949a 100644
@@ -219,6 +219,7 @@ endif
 	source "package/psplash/Config.in"
 	source "package/sawman/Config.in"
 	source "package/sdl/Config.in"
+	source "package/sdl2/Config.in"
 	source "package/sdl_gfx/Config.in"
 	source "package/sdl_image/Config.in"
 	source "package/sdl_mixer/Config.in"
package/sdl2/Config.in new file +8 -0
new file mode 100644
index 0000000..18e4f75
@@ -0,0 +1,26 @@
+config BR2_PACKAGE_SDL2
+	depends on !BR2_STATIC_LIBS
+	bool "sdl2"
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+

package/sdl2/Config.in line 18

Quoted 10 lines
package/sdl2/Config.in new file +10 -0
+	  http://www.libsdl.org/
+
+comment "sdl2 needs a toolchain w/ dynamic library"
+        depends on BR2_STATIC_LIBS
+
+if BR2_PACKAGE_SDL2
+
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB

You can add a comment here to say that directfb support needs directfb package to be enabled.

comment "DirectFB video driver needs directfb"
depends on !BR2_PACKAGE_DIRECTFB

package/sdl2/Config.in line 24

Quoted 6 lines
package/sdl2/Config.in new file +6 -0
+
+config BR2_PACKAGE_SDL2_X11
+	bool "X11 video driver"
+	depends on BR2_PACKAGE_XORG7
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXEXT

Same for X11

Quoted 9 lines
package/sdl2/Config.in new file +2 -0
+
+endif
package/sdl2/sdl2.mk new file +1 -0
new file mode 100644
index 0000000..6a3caaf
@@ -0,0 +1,62 @@
+################################################################################

package/sdl2/sdl2.mk line 11

Quoted 10 lines
package/sdl2/sdl2.mk new file +10 -0
+#
+# sdl2
+#
+################################################################################
+
+SDL2_VERSION = 2.0.3
+SDL2_SOURCE = SDL2-$(SDL2_VERSION).tar.gz
+SDL2_SITE = http://www.libsdl.org/release
+SDL2_LICENSE = zlib
+SDL2_LICENSE_FILES = COPYING

It's COPYING.txt

Can you check the libudev dependency since libudev support is set by default ?

package/sdl2/sdl2.mk line 15

Quoted 4 lines
package/sdl2/sdl2.mk new file +4 -0
+SDL2_INSTALL_STAGING = YES
+
+SDL2_CONF_OPTS += \
+	--enable-pulseaudio=no \

Why do you disable pulseaudio support ? we have a pulsaudio package for the target. Otherwise, ok the pulseaudio support can be added in a follow up patch.

package/sdl2/sdl2.mk line 19

Quoted 4 lines
package/sdl2/sdl2.mk new file +4 -0
+	--disable-arts \
+	--disable-esd
+
+# We must enable static build to get compilation succesful.

s/succesful/successful/

I used this patch to enable sdl support in the efl package bumped to 1.14-beta1 (patch series to be sent). At least it build fine with your sdl2 package, but I need to do some runtime testing.

Quoted 6 lines
package/sdl2/sdl2.mk new file +6 -0
+SDL2_CONF_OPTS += --enable-static
+
+ifeq ($(BR2_PACKAGE_SDL2_DIRECTFB),y)
+SDL2_DEPENDENCIES += directfb
+SDL2_CONF_OPTS += --enable-video-directfb=yes
+SDL2_CONF_ENV = ac_cv_path_DIRECTFBCONFIG=$(STAGING_DIR)/usr/bin/directfb-config

package/sdl2/sdl2.mk line 35

Quoted 10 lines
package/sdl2/sdl2.mk new file +10 -0
+else
+SDL2_CONF_OPTS += --enable-video-directfb=no
+endif
+
+ifeq ($(BR2_PACKAGE_SDL2_X11),y)
+SDL2_CONF_OPTS += --enable-video-x11=yes
+SDL2_DEPENDENCIES += \
+	xlib_libX11 xlib_libXext \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER),xlib_libXrender) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR),xlib_libXrandr)

Some X11 dependencies are missing:
x11-xcursor
x11-xinerama
x11-xinput
x11-scrnsaver
x11-xshape (I don't think we have a package for it in Buildroot)

package/sdl2/sdl2.mk line 38

Quoted 3 lines
package/sdl2/sdl2.mk new file +3 -0
+else
+SDL2_CONF_OPTS += --enable-video-x11=no
+endif

Also, sdl2 may depends on virtual package libgl or libgles in order to enable video-opengl and video-opengles features.

If you don't want to enable these options right now, you need to disable them explicitly since they are enabled by default. Doing this avoid issues with build dependency order.

Quoted 10 lines
package/sdl2/sdl2.mk new file +10 -0
+
+ifeq ($(BR2_PACKAGE_TSLIB),y)
+SDL2_DEPENDENCIES += tslib
+SDL2_CONF_OPTS += --enable-input-tslib
+else
+SDL2_CONF_OPTS += --disable-input-tslib
+endif
+
+ifeq ($(BR2_PACKAGE_ALSA_LIB),y)
+SDL2_DEPENDENCIES += alsa-lib

package/sdl2/sdl2.mk line 58

Quoted 10 lines
package/sdl2/sdl2.mk new file +10 -0
+SDL2_CONF_OPTS += --enable-alsa
+else
+SDL2_CONF_OPTS += --disable-alsa
+endif
+
+# Remove the -Wl,-rpath option.
+define SDL2_FIXUP_SDL2_CONFIG
+	$(SED) 's%-Wl,-rpath,\$${libdir}%%' \
+		$(STAGING_DIR)/usr/bin/sdl2-config
+endef

Did you try to use --disable-rpath instead ?

package/sdl2/sdl2.mk line 62

Quoted 5 lines
package/sdl2/sdl2.mk new file +4 -0
+
+SDL2_POST_INSTALL_STAGING_HOOKS += SDL2_FIXUP_SDL2_CONFIG
+
+$(eval $(autotools-package))

Thanks,
Romain Naour

author
#9 lore raw

PATCH v3 1/1 sdl2: new package

Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>

---

Changes in V3:
* Add dependenies comments for DirectFB and X11 options
* Fix license filename
* Add explicit enable/disable options for libudev
* Fix typo: s/succesful/successful/
* Add more X11 dependencies: x11-xcursor, x11-xinerama, x11-xinput, x11-scrnsaver
* Disbale opengl/opengles video diver explicitly
* Use --disable-rpath option option instead of manual fix

Changes in V2:
* Fix SDL2 case: s/SDL2/sdl2/
* Add dependency on toolchain with dynamic library
* Remove unneeded 'SDL2' in sub-options names
* Remove unneeded space after comma in if statement
* Add explicit enable/disable options for tslib and alsa
* Remove Mesa3D optional part
* Move unconditionnal SDL2_CONF_OPTS upper
* Remove host build

 package/Config.in      |  1 +
 package/sdl2/Config.in | 32 ++++++++++++++++++++++++
 package/sdl2/sdl2.mk   | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 package/sdl2/Config.in
 create mode 100644 package/sdl2/sdl2.mk
package/Config.in +1 -0
index c2f6524..f16a114 100644
@@ -245,6 +245,7 @@ endif
 	source "package/psplash/Config.in"
 	source "package/sawman/Config.in"
 	source "package/sdl/Config.in"
+	source "package/sdl2/Config.in"
 	source "package/sdl_gfx/Config.in"
 	source "package/sdl_image/Config.in"
 	source "package/sdl_mixer/Config.in"
package/sdl2/Config.in new file +32 -0
new file mode 100644
index 0000000..c3f61f5
@@ -0,0 +1,32 @@
+config BR2_PACKAGE_SDL2
+	depends on !BR2_STATIC_LIBS
+	bool "sdl2"
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/
+
+comment "sdl2 needs a toolchain w/ dynamic library"
+        depends on BR2_STATIC_LIBS
+
+if BR2_PACKAGE_SDL2
+
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB
+
+comment "DirectFB video driver needs directfb"
+        depends on !BR2_PACKAGE_DIRECTFB
+
+config BR2_PACKAGE_SDL2_X11
+	bool "X11 video driver"
+	depends on BR2_PACKAGE_XORG7
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXEXT
+
+comment "X11 video driver needs X.org"
+        depends on !BR2_PACKAGE_XORG7
+
+endif
package/sdl2/sdl2.mk new file +68 -0

68 lines not on this page · raw message

Signature · 1 line
1.8.4.5
#10 lore raw

PATCH v3 1/1 sdl2: new package

Guillaume, All,

On 2015-06-29 22:30 +0200, Guillaume GARDET spake thusly:

Quoted 3 lines
Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>

[--SNIP--]

Quoted 8 lines
package/sdl2/Config.in new file +2 -0
new file mode 100644
index 0000000..c3f61f5
@@ -0,0 +1,32 @@
+config BR2_PACKAGE_SDL2
+	depends on !BR2_STATIC_LIBS

package/sdl2/Config.in line 12

Quoted 10 lines
package/sdl2/Config.in new file +10 -0
+	bool "sdl2"
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/
+
+comment "sdl2 needs a toolchain w/ dynamic library"
+        depends on BR2_STATIC_LIBS

Either you put that comment at the beginning of the file, or at the end, not in the middle.

The reason is that Kconfig, when it detects a dependnecy chain between two adjacent symbols, will render the second symbol indented below the first, like so:

config FOO
bool "foo"

config BAR
boll "bar'
depends on FOO

will be render in the menuconfig as:

[*] foo
[ ] bar

To show that 'bar' is a sub-option of 'foo'.

If you put the comment in between FOO nad BAR, then you break the "dependency chain" and the symbols will be rendred as:

    [*] foo
    [ ] bar

which is less than ideal.

[I haven't looked at the rest of the file, as I know about nothing about sdl2]

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#11 lore raw

PATCH v4 1/1 sdl2: new package

Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>
Cc Yann E. Morin <[email protected]>

---

Changes in V4:
* Fix comments places in Config.in

Changes in V3:
* Add dependenies comments for DirectFB and X11 options
* Fix license filename
* Add explicit enable/disable options for libudev
* Fix typo: s/succesful/successful/
* Add more X11 dependencies: x11-xcursor, x11-xinerama, x11-xinput, x11-scrnsaver
* Disbale opengl/opengles video diver explicitly
* Use --disable-rpath option option instead of manual fix

Changes in V2:
* Fix SDL2 case: s/SDL2/sdl2/
* Add dependency on toolchain with dynamic library
* Remove unneeded 'SDL2' in sub-options names
* Remove unneeded space after comma in if statement
* Add explicit enable/disable options for tslib and alsa
* Remove Mesa3D optional part
* Move unconditionnal SDL2_CONF_OPTS upper
* Remove host build

 package/Config.in      |  1 +
 package/sdl2/Config.in | 32 ++++++++++++++++++++++++
 package/sdl2/sdl2.mk   | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 package/sdl2/Config.in
 create mode 100644 package/sdl2/sdl2.mk
package/Config.in +1 -0
index 13a7e74..3a1b5c6 100644
@@ -246,6 +246,7 @@ endif
 	source "package/psplash/Config.in"
 	source "package/sawman/Config.in"
 	source "package/sdl/Config.in"
+	source "package/sdl2/Config.in"
 	source "package/sdl_gfx/Config.in"
 	source "package/sdl_image/Config.in"
 	source "package/sdl_mixer/Config.in"
package/sdl2/Config.in new file +32 -0
new file mode 100644
index 0000000..fe30650
@@ -0,0 +1,32 @@
+config BR2_PACKAGE_SDL2
+	depends on !BR2_STATIC_LIBS
+	bool "sdl2"
+	help
+	  Simple DirectMedia Layer 2 - SDL2 is a library that allows
+	  programs portable low level access to a video framebuffer,
+	  audio output, mouse, and keyboard. It is not compatible with SDL1.
+
+	  http://www.libsdl.org/
+
+if BR2_PACKAGE_SDL2
+
+config BR2_PACKAGE_SDL2_DIRECTFB
+	bool "DirectFB video driver"
+	depends on BR2_PACKAGE_DIRECTFB
+
+config BR2_PACKAGE_SDL2_X11
+	bool "X11 video driver"
+	depends on BR2_PACKAGE_XORG7
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXEXT
+
+comment "DirectFB video driver needs directfb"
+	depends on !BR2_PACKAGE_DIRECTFB
+
+comment "X11 video driver needs X.org"
+	depends on !BR2_PACKAGE_XORG7
+
+endif
+
+comment "sdl2 needs a toolchain w/ dynamic library"
+	depends on BR2_STATIC_LIBS
package/sdl2/sdl2.mk new file +68 -0

68 lines not on this page · raw message

Signature · 1 line
1.8.4.5
#12 lore raw

PATCH v4 1/1 sdl2: new package

Hi Guillaume,

Le 02/07/2015 10:16, Guillaume GARDET a ?crit :

Quoted 37 lines
Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>
Cc Yann E. Morin <[email protected]>

---

Changes in V4:
* Fix comments places in Config.in

Changes in V3:
* Add dependenies comments for DirectFB and X11 options
* Fix license filename
* Add explicit enable/disable options for libudev
* Fix typo: s/succesful/successful/
* Add more X11 dependencies: x11-xcursor, x11-xinerama, x11-xinput, x11-scrnsaver
* Disbale opengl/opengles video diver explicitly
* Use --disable-rpath option option instead of manual fix

Changes in V2:
* Fix SDL2 case: s/SDL2/sdl2/
* Add dependency on toolchain with dynamic library
* Remove unneeded 'SDL2' in sub-options names
* Remove unneeded space after comma in if statement
* Add explicit enable/disable options for tslib and alsa
* Remove Mesa3D optional part
* Move unconditionnal SDL2_CONF_OPTS upper
* Remove host build

 package/Config.in      |  1 +
 package/sdl2/Config.in | 32 ++++++++++++++++++++++++
 package/sdl2/sdl2.mk   | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 package/sdl2/Config.in
 create mode 100644 package/sdl2/sdl2.mk

[snip]

package/sdl2/sdl2.mk line 49

Quoted 8 lines
package/sdl2/sdl2.mk new file +8 -0
+SDL2_DEPENDENCIES += \
+	xlib_libX11 xlib_libXext \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER),xlib_libXrender) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR),xlib_libXrandr) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXCURSOR,xlib_libXcursor) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXINERAMA,xlib_libXinerama) \
+	$(if $(BR2_PACKAGE_XPROTO_INPUTPROTO,xproto_inputproto) \
+	$(if $(BR2_PACKAGE_XPROTO_SCRNSAVERPROTO,xproto_scrnsaverproto)

There are missing ',' and ')' here.

This part should look like:

SDL2_DEPENDENCIES += \
	xlib_libX11 xlib_libXext \
	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER),xlib_libXrender,) \
	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR),xlib_libXrandr,) \
	$(if $(BR2_PACKAGE_XLIB_LIBXCURSOR),xlib_libXcursor,) \
	$(if $(BR2_PACKAGE_XLIB_LIBXINERAMA),xlib_libXinerama,) \
	$(if $(BR2_PACKAGE_XPROTO_INPUTPROTO),xproto_inputproto,) \
	$(if $(BR2_PACKAGE_XPROTO_SCRNSAVERPROTO),xproto_scrnsaverproto,)

sdl2 is seems complicated package, I think it would be easier to review if you add it progressively by using a patch series:

PATCH1: sdl2 new package (all mandatory dependencies enabled)
PATCH2: sdl2 add X11 support
PATCH3: sdl2 add directfb support
...

I'll review your patch latter new week end.

Best regards,
Romain

author
#13 lore raw

PATCH v4 1/1 sdl2: new package

Hi,

Le 05/07/2015 20:21, Romain Naour a ?crit :

Quoted 42 lines

Hi Guillaume,

Le 02/07/2015 10:16, Guillaume GARDET a ?crit :

Quoted 37 lines
Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>
Cc Yann E. Morin <[email protected]>

---

Changes in V4:
* Fix comments places in Config.in

Changes in V3:
* Add dependenies comments for DirectFB and X11 options
* Fix license filename
* Add explicit enable/disable options for libudev
* Fix typo: s/succesful/successful/
* Add more X11 dependencies: x11-xcursor, x11-xinerama, x11-xinput, x11-scrnsaver
* Disbale opengl/opengles video diver explicitly
* Use --disable-rpath option option instead of manual fix

Changes in V2:
* Fix SDL2 case: s/SDL2/sdl2/
* Add dependency on toolchain with dynamic library
* Remove unneeded 'SDL2' in sub-options names
* Remove unneeded space after comma in if statement
* Add explicit enable/disable options for tslib and alsa
* Remove Mesa3D optional part
* Move unconditionnal SDL2_CONF_OPTS upper
* Remove host build

  package/Config.in      |  1 +
  package/sdl2/Config.in | 32 ++++++++++++++++++++++++
  package/sdl2/sdl2.mk   | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 101 insertions(+)

create mode 100644 package/sdl2/Config.in
create mode 100644 package/sdl2/sdl2.mk

[snip]

Quoted 9 lines

package/sdl2/sdl2.mk line 49

Quoted 8 lines
package/sdl2/sdl2.mk new file +8 -0
+SDL2_DEPENDENCIES += \
+	xlib_libX11 xlib_libXext \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER),xlib_libXrender) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR),xlib_libXrandr) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXCURSOR,xlib_libXcursor) \
+	$(if $(BR2_PACKAGE_XLIB_LIBXINERAMA,xlib_libXinerama) \
+	$(if $(BR2_PACKAGE_XPROTO_INPUTPROTO,xproto_inputproto) \
+	$(if $(BR2_PACKAGE_XPROTO_SCRNSAVERPROTO,xproto_scrnsaverproto)

There are missing ',' and ')' here.

Ok for ')', it is clearly a typo, but why do you want to add an extra comma? It is not the case for other packages.

Quoted 12 lines

This part should look like:

SDL2_DEPENDENCIES += \
	xlib_libX11 xlib_libXext \
	$(if $(BR2_PACKAGE_XLIB_LIBXRENDER),xlib_libXrender,) \
	$(if $(BR2_PACKAGE_XLIB_LIBXRANDR),xlib_libXrandr,) \
	$(if $(BR2_PACKAGE_XLIB_LIBXCURSOR),xlib_libXcursor,) \
	$(if $(BR2_PACKAGE_XLIB_LIBXINERAMA),xlib_libXinerama,) \
	$(if $(BR2_PACKAGE_XPROTO_INPUTPROTO),xproto_inputproto,) \
	$(if $(BR2_PACKAGE_XPROTO_SCRNSAVERPROTO),xproto_scrnsaverproto,)
Quoted 6 lines

sdl2 is seems complicated package, I think it would be easier to review if you add it progressively by using a patch series:

PATCH1: sdl2 new package (all mandatory dependencies enabled)
PATCH2: sdl2 add X11 support
PATCH3: sdl2 add directfb support

No, I won't. It is extra work for nothing. I think it is not so complicated to review only 2 files to add a new package. Splitting packages for u-boot or Linux kernel make sense most of the time, but, here, clearly no. Buildroot maintainers/reviewers should send their comments from the 1st version, not 1 guy comments on V1, someone else on V2 etc., again the first guy for V3 on another part of the patch which was not commented the 1st time.

I contribute to lots of open source / free software and contributing to Buildroot is really painful compared to other projects. So please, all send your comments at the same time. For SDL2, please send your comments this week, and I will send a V5 next week fixing the remaining bits and if it does not fit to another guy again, I will stop sending patches to Buildroot. Now, I understand better why some people are maintaining a buildroot fork instead of upstreaming their patches. So, please improve your patch submission/review process.

Guillaume

Quoted 8 lines

...

I'll review your patch latter new week end.

Best regards,
Romain

#14 lore raw

PATCH v4 1/1 sdl2: new package

Hello,

Quoted 6 lines · Guillaume GARDET - Olis?o

No, I won't. It is extra work for nothing. I think it is not so complicated to review only 2 files to add a new package. Splitting packages for u-boot or Linux kernel make sense most of the time, but, here, clearly no. Buildroot maintainers/reviewers should send their comments from the 1st version, not 1 guy comments on V1, someone else on V2 etc., again the first guy for V3 on another part of the patch which was not commented the 1st time.

I contribute to lots of open source / free software and contributing to Buildroot is really painful compared to other projects. So please, all send your comments at the same time. For SDL2, please send your comments this week, and I will send a V5 next week fixing the remaining bits and if it does not fit to another guy again, I will stop sending patches to Buildroot. Now, I understand better why some people are maintaining a buildroot fork instead of upstreaming their patches. So, please improve your patch submission/review process.

Could you use an e-mail client that wraps lines at a reasonable length?

Regarding the patch submission/review process, we merge tons and tons of patches all the time. However, it is true that we tend to merge patches from people who also help in reviewing patches from others. I don't think I've seen patch reviews/tests from you. It would definitely help in increasing the trust that the maintainers have in your patches, and therefore make them look at your patches faster.

Did you look at http://patchwork.ozlabs.org/project/buildroot/list/ ? This is the complete list of pending patches we have, which as you can see is quite long. Your help is definitely welcome to reduce this list, by reviewing, testing and commenting patches from others.

And if you think that the entire community can synchronize to send all the comments on the v1, then it clearly means that you have never contributed to the Linux kernel, or at least not significant patches. It is very common to get some comments on v1, then some other comments on v2 (from different persons, or even from the same person who realized some more issues). That's definitely not something you can expect from an open-source community.

However, I do agree that we could do better to reply to patches in a timely fashion. And to make this happen, we need *your* help to look at patches from others.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#15 lore raw

PATCH v4 1/1 sdl2: new package

Le 06/07/2015 14:26, Thomas Petazzoni a ?crit :

Quoted 10 lines

Hello,

Quoted 6 lines · Guillaume GARDET - Olis?o

No, I won't. It is extra work for nothing. I think it is not so complicated to review only 2 files to add a new package. Splitting packages for u-boot or Linux kernel make sense most of the time, but, here, clearly no. Buildroot maintainers/reviewers should send their comments from the 1st version, not 1 guy comments on V1, someone else on V2 etc., again the first guy for V3 on another part of the patch which was not commented the 1st time.

I contribute to lots of open source / free software and contributing to Buildroot is really painful compared to other projects. So please, all send your comments at the same time. For SDL2, please send your comments this week, and I will send a V5 next week fixing the remaining bits and if it does not fit to another guy again, I will stop sending patches to Buildroot. Now, I understand better why some people are maintaining a buildroot fork instead of upstreaming their patches. So, please improve your patch submission/review process.

Quoted 8 lines

Could you use an e-mail client that wraps lines at a reasonable length?

Regarding the patch submission/review process, we merge tons and tons of patches all the time. However, it is true that we tend to merge patches from people who also help in reviewing patches from others. I don't think I've seen patch reviews/tests from you. It would definitely help in increasing the trust that the maintainers have in your patches, and therefore make them look at your patches faster.

I do not claim to be a Buildroot maintainer/reviewer, even if I use it quite often (as a user) and train people to use it.
I guess you understand we cannot be deeply involved in all open source / free software we contribute and submit patches is already a good contribution. Lots of people keep their patches and never send them upstream (not only for Buildroot).

Quoted 2 lines

Did you look at http://patchwork.ozlabs.org/project/buildroot/list/ ?

Yes. This is quite small compared to u-boot one:
https://patchwork.ozlabs.org/project/uboot/list/

Quoted 3 lines

This is the complete list of pending patches we have, which as you can see is quite long. Your help is definitely welcome to reduce this list, by reviewing, testing and commenting patches from others.

I guess it is.
But as I wrote previously, it is not possible to be deeply involved in all open source projects we contribute.

Quoted 8 lines

And if you think that the entire community can synchronize to send all the comments on the v1, then it clearly means that you have never contributed to the Linux kernel, or at least not significant patches. It is very common to get some comments on v1, then some other comments on v2 (from different persons, or even from the same person who realized some more issues). That's definitely not something you can expect from an open-source community.

It happens, yes, but not on every new patch version and the patches are generally much more complicated.

Quoted 4 lines

However, I do agree that we could do better to reply to patches in a timely fashion. And to make this happen, we need *your* help to look at patches from others.

From my point of view, other projects, like u-boot, are more patch-friendly than Buildroot. Especially for new contributors.
You cannot ask all patch senders to become reviewers or testers.

It could make sense to split Buildroot tree with maintainers for each part as it is done for the kernel or u-boot. It will avoid that people comments here and there. Another solution would be that 1 person/team flags a given patch and this person/team follows the patch submission flow. Maybe using the "Deleguate" flag in patchwork tool. It may be fine to merge a patch even if it is not really perfect (e.g. does not offer all options) and then ask for updates/improvements. That way, it allows someone else to do the following patch.

I write these to help you to improve your work flow, not just to say it is better elsewhere.

That said, do you really think that 5 versions (if I include the next one) is really something normal for a simple single package which does not affect others? I think 3 or 4 should be enough.

Guillaume

#16 lore raw

PATCH v4 1/1 sdl2: new package

Hello,

Again, please configure Thunderbird to make it wrap the very long lines you write.

Quoted 3 lines · Guillaume GARDET - Olis?o

I do not claim to be a Buildroot maintainer/reviewer, even if I use it quite often (as a user) and train people to use it.
I guess you understand we cannot be deeply involved in all open source / free software we contribute and submit patches is already a good contribution. Lots of people keep their patches and never send them upstream (not only for Buildroot).

That is indeed true, but there's not much more we can do I believe:
people interested in doing review/testing are doing as much of it as they can. And trust me, a number of us are already spending a crazy amount of time doing reviews.

Quoted 4 lines
Quoted 1 line

Did you look at http://patchwork.ozlabs.org/project/buildroot/list/ ?

Yes. This is quite small compared to u-boot one:
https://patchwork.ozlabs.org/project/uboot/list/

Is the U-Boot patchwork actually kept up-to-date with the patches that have been merged? Because there are lots of patchwork instances that exist, but that are not actively used by the project.

Quoted 5 lines
Quoted 3 lines

However, I do agree that we could do better to reply to patches in a timely fashion. And to make this happen, we need *your* help to look at patches from others.

From my point of view, other projects, like u-boot, are more patch-friendly than Buildroot. Especially for new contributors.

I believe it really depends on the specific patch you send. Some new contributors in Buildroot see their patch applied very quickly, some other new contributors have a less satisfying experience.

Quoted 2 lines

It could make sense to split Buildroot tree with maintainers for each part as it is done for the kernel or u-boot. It will avoid that people comments here and there. Another solution would be that 1 person/team flags a given patch and this person/team follows the patch submission flow. Maybe using the "Deleguate" flag in patchwork tool.

Yeah, we have been talking since a while to add the notion of maintainers. But that would only work for existing packages. For new packages it's quite difficult to see to which maintainer they would belong to. And actually, the original author of the package should become its maintainer, so there must anyway be someone else in the Buildroot community reviewing the initial package.

See http://lists.busybox.net/pipermail/buildroot/2015-March/123032.html
for a patch series from Yann Morin proposing to add the notion of maintainers.

Quoted 1 line

It may be fine to merge a patch even if it is not really perfect (e.g. does not offer all options) and then ask for updates/improvements. That way, it allows someone else to do the following patch.

I am all for that, and I tend to suggest this a lot. I have no problem merging a package that does not offer all the possibilities, as long as it at least explicitly disables the stuff it doesn't use (to avoid misdetection).

Quoted 2 lines

That said, do you really think that 5 versions (if I include the next one) is really something normal for a simple single package which does not affect others? I think 3 or 4 should be enough.

Impossible to make a rule on this. It depends on good/bad the initial submission is, it depends how deep the review is done, it depends how complex the package is.

But I agree that something relatively simple such as sdl2 shouldn't take 5 months to get merged, that's for sure. Especially when the submitter (you!) has been very active and responsive to take into account the comments.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#17 lore raw

PATCH v4 1/1 sdl2: new package

In reply to Guillaume GARDET - Oliséo

Hi Guillaume,

Let me start with thanking you for this feedback, since we are indeed
struggling with improving our patch contribution flow.

Quoted 8 lines · Guillaume GARDET - Olis?o

Le 06/07/2015 14:26, Thomas Petazzoni a ?crit :

Quoted 3 lines

However, I do agree that we could do better to reply to patches in a timely fashion. And to make this happen, we need *your* help to look at patches from others.

From my point of view, other projects, like u-boot, are more patch-friendly than Buildroot. Especially for new contributors.

Could you explain what aspect(s) of the U-Boot contribution process make it
more patch-friendly? It indeed looks like their patch queue is shorter even though they seem to get the same order of magnitude of contributions.

Are they less critical about patches?

Are patches taken over by maintainters so the original contributor doesn't have
to respin it too often?

Do they make sure review happens quickly?

Are they able to avoid that patches that had no review for a week are
completely ignored for a long time? How do they do that?

Quoted 1 line

You cannot ask all patch senders to become reviewers or testers.

We certainly can ask. We can't *require*.

Quoted 7 lines

It could make sense to split Buildroot tree with maintainers for each part as it is done for the kernel or u-boot. It will avoid that people comments here and there.
Another solution would be that 1 person/team flags a given patch and this person/team follows the patch submission flow. Maybe using the "Deleguate" flag in patchwork tool.

We do try to make sure of that (i.e. the first reviewer continues to look after
the patch), but indeed we have no formal process for it and it's all based on goodwill.

Quoted 3 lines

It may be fine to merge a patch even if it is not really perfect (e.g. does not offer all options) and then ask for updates/improvements. That way, it allows someone else to do the following patch.

I think the fear is that the improvements will never materialize, and we get
stuck with imperfect packages. A typical package will not be looked at again for a very long time, but on the other hand every package may be one that a new contributor is looking at as an example of how things should be done...

Quoted 3 lines

I write these to help you to improve your work flow, not just to say it is better elsewhere.

Again, we really appreciate this.

Quoted 4 lines

That said, do you really think that 5 versions (if I include the next one) is really something normal for a simple single package which does not affect others? I think 3 or 4 should be enough.

Ideally, 1 iteration should be enough :-)

Regards,
Arnout

Signature · 6 lines
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
#18 lore raw

PATCH v4 1/1 sdl2: new package

In reply to Guillaume GARDET - Oliséo

Hi Guillaume,

Le 06/07/2015 14:17, Guillaume GARDET - Olis?o a ?crit :

Quoted 54 lines

Hi,

Le 05/07/2015 20:21, Romain Naour a ?crit :

Quoted 51 lines

Hi Guillaume,

Le 02/07/2015 10:16, Guillaume GARDET a ?crit :

Quoted 38 lines
Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>
Cc Yann E. Morin <[email protected]>

---

Changes in V4:
* Fix comments places in Config.in

Changes in V3:
* Add dependenies comments for DirectFB and X11 options
* Fix license filename
* Add explicit enable/disable options for libudev
* Fix typo: s/succesful/successful/
* Add more X11 dependencies: x11-xcursor, x11-xinerama, x11-xinput, x11-scrnsaver
* Disbale opengl/opengles video diver explicitly
* Use --disable-rpath option option instead of manual fix

Changes in V2:
* Fix SDL2 case: s/SDL2/sdl2/
* Add dependency on toolchain with dynamic library
* Remove unneeded 'SDL2' in sub-options names
* Remove unneeded space after comma in if statement
* Add explicit enable/disable options for tslib and alsa
* Remove Mesa3D optional part
* Move unconditionnal SDL2_CONF_OPTS upper
* Remove host build

  package/Config.in      |  1 +
  package/sdl2/Config.in | 32 ++++++++++++++++++++++++
  package/sdl2/sdl2.mk   | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 101 insertions(+)

create mode 100644 package/sdl2/Config.in
create mode 100644 package/sdl2/sdl2.mk

[snip]

Quoted 8 lines
package/sdl2/sdl2.mk new file +8 -0
+SDL2_DEPENDENCIES += \
+    xlib_libX11 xlib_libXext \
+    $(if $(BR2_PACKAGE_XLIB_LIBXRENDER),xlib_libXrender) \
+    $(if $(BR2_PACKAGE_XLIB_LIBXRANDR),xlib_libXrandr) \
+    $(if $(BR2_PACKAGE_XLIB_LIBXCURSOR,xlib_libXcursor) \
+    $(if $(BR2_PACKAGE_XLIB_LIBXINERAMA,xlib_libXinerama) \
+    $(if $(BR2_PACKAGE_XPROTO_INPUTPROTO,xproto_inputproto) \
+    $(if $(BR2_PACKAGE_XPROTO_SCRNSAVERPROTO,xproto_scrnsaverproto)
Quoted 4 lines
Quoted 1 line

There are missing ',' and ')' here.

Ok for ')', it is clearly a typo, but why do you want to add an extra comma? It is not the case for other packages.

Ok, my mistake.

Quoted 4 lines
Quoted 4 lines

This part should look like:

Quoted 8 lines
Quoted 8 lines
SDL2_DEPENDENCIES += \
    xlib_libX11 xlib_libXext \
    $(if $(BR2_PACKAGE_XLIB_LIBXRENDER),xlib_libXrender,) \
    $(if $(BR2_PACKAGE_XLIB_LIBXRANDR),xlib_libXrandr,) \
    $(if $(BR2_PACKAGE_XLIB_LIBXCURSOR),xlib_libXcursor,) \
    $(if $(BR2_PACKAGE_XLIB_LIBXINERAMA),xlib_libXinerama,) \
    $(if $(BR2_PACKAGE_XPROTO_INPUTPROTO),xproto_inputproto,) \
    $(if $(BR2_PACKAGE_XPROTO_SCRNSAVERPROTO),xproto_scrnsaverproto,)

Actually it's not correct ether...

We prefer also add autotools --enable-*/--disable-* options to explicitly enable/disable the optional package.

Something like:
ifeq ($(BR2_PACKAGE_XLIB_LIBXCURSOR),y)
SDL2_DEPENDENCIES += xlib_libXcursor
SDL2_CONF_OPTS += --enable-video-x11-xcursor
else
SDL2_CONF_OPTS += --disable-video-x11-xcursor
endif

Quoted 10 lines
Quoted 7 lines

sdl2 is seems complicated package, I think it would be easier to review if you add it progressively by using a patch series:

PATCH1: sdl2 new package (all mandatory dependencies enabled)
PATCH2: sdl2 add X11 support
PATCH3: sdl2 add directfb support

No, I won't. It is extra work for nothing. I think it is not so complicated to review only 2 files to add a new package.

This is not only review two files, the review process is also to check the package build system to avoid build failures. Especially, when a package depends on X11 or OpenGL (from my point of view).

Until now, I had no time to review deeply the sdl2 package. Even during the hackaton this week.

So, I reviewed your package today and checked for missing package/toolchains dependencies, and made several changes:

[Romain:
- Wrap Config.in help text at 72 columns.
- Move sdl2 package after sdl modules in Config.in. (Arnout)
- Explicitly disable dbus and wayland.
- Remove double underscore (SDL2__*).
- Unify autotools options to use --enable/--disable.
- Use x-includes and x-libraries to avoid path poisoning.
- Remove xlib_libXrender, xproto_inputproto and xproto_scrnsaverproto dependencies since the build system doesn't depend on them.
- Add Xlib_libXi, xlib_libScrnSaver and xlib_libXxf86vm dependencies.
- Handle autotools options (--enable/--disable) for each X11 dependencies.]

I'll send an updated version, care to take a look and review please ?

Splitting packages for u-boot or Linux

Quoted 8 lines

kernel make sense most of the time, but, here, clearly no. Buildroot maintainers/reviewers should send their comments from the 1st version, not 1 guy comments on V1, someone else on V2 etc., again the first guy for V3 on another part of the patch which was not commented the 1st time.

I contribute to lots of open source / free software and contributing to Buildroot is really painful compared to other projects. So please, all send your comments at the same time.

I have nothing to say to complete Thomas's and Arnout's answers. Feel free to come to the Buildroot Developer Days and see how patches are discussed and reviewed:

http://elinux.org/Buildroot:DeveloperDaysELCE2015

Quoted 6 lines

For SDL2, please send your comments this week, and I will send a V5 next week fixing the remaining bits and if it does not fit to another guy again, I will stop sending patches to Buildroot.
Now, I understand better why some people are maintaining a buildroot fork instead of upstreaming their patches. So, please improve your patch submission/review process.

Yes, upstreaming is difficult but the final result is really better. See changes that have been made to your c-icap and c-icap-modules packages.

Best regards,
Romain

Quoted 14 lines

Guillaume

Quoted 8 lines

...

I'll review your patch latter new week end.

Best regards,
Romain

#19 lore raw

PATCH v4 1/1 sdl2: new package

Hi Guillaume and Romain,

Thank you for your effort on this nice sdl2 buildroot patch. I hope you are still working on it.

While trying v5 of the path recently, as found in buildroot patchwork, I needed to add the following line to sdl2.mk:

SDL2_CONFIG_SCRIPTS = sdl2-config

Without this line, the include path returned by sdl2-config --cflags will be something like /usr/include/SDL2, instead of pointing to the buildroot staging dir.

You might want to add it to a v6, maybe?

Apart from that, it works fine for me, thanks!

Best regards,

V.

#20 lore raw

PATCH v4 1/1 sdl2: new package

Hi Vincent, All,

Le 19/10/2015 15:45, Vincent Stehl? a ?crit :

Quoted 4 lines

Hi Guillaume and Romain,

Thank you for your effort on this nice sdl2 buildroot patch. I hope you are still working on it.

Actually no, I only reviewed it after the Buildroot summer camp and sent an updated version. Since then it's waiting for a reviewer or tester ;-)

Quoted 11 lines

While trying v5 of the path recently, as found in buildroot patchwork, I needed to add the following line to sdl2.mk:

SDL2_CONFIG_SCRIPTS = sdl2-config

Without this line, the include path returned by sdl2-config --cflags will be something like /usr/include/SDL2, instead of pointing to the buildroot staging dir.

You might want to add it to a v6, maybe?

Thanks for the feedback ! You're probably right, I need to rebuild it locally to check.

I used it to enable the sdl2 support during the development of my efl patches series, I didn't see any build issue even with the paranoid toolchain wrapper enabled... (but the sdl2-config is probably not used)

Quoted 2 lines

Apart from that, it works fine for me, thanks!

Great!
I'll send an updated version with your fix, can you send your tested and/or reviewed tag ;-)

Best regards,
Romain

Quoted 5 lines

Best regards,

V.

#21 lore raw

PATCH v4 1/1 sdl2: new package

Hi Vincent, All,

Le 21/10/2015 22:56, Romain Naour a ?crit :

Quoted 11 lines

Hi Vincent, All,

Quoted 10 lines

While trying v5 of the path recently, as found in buildroot patchwork, I needed to add the following line to sdl2.mk:

SDL2_CONFIG_SCRIPTS = sdl2-config

Without this line, the include path returned by sdl2-config --cflags will be something like /usr/include/SDL2, instead of pointing to the buildroot staging dir.

I confirm there is a poisoned path returned by this script:

sdl2-config --cflags
-I/usr/include/SDL2 -I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include -D_REENTRANT

sdl2-config --libs
-L/usr/lib -lSDL2 -lpthread

And thanks to your fix:

sdl2-config --cflags -I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include/SDL2 -I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include -D_REENTRANT

sdl2-config --libs -L/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/lib -lSDL2 -lpthread

Good catch !

Best regards,
Romain

#22 lore raw

PATCH v4 1/1 sdl2: new package

Romain,

Quoted 18 lines · Romain Naour

I confirm there is a poisoned path returned by this script:

sdl2-config --cflags

(quoted diff) +0 -3
-I/usr/include/SDL2
-I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include
-D_REENTRANT

sdl2-config --libs
-L/usr/lib -lSDL2 -lpthread

And thanks to your fix:

sdl2-config --cflags

(quoted diff) +0 -3
-I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include/SDL2
-I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include
-D_REENTRANT

sdl2-config --libs

Review

Quoted 2 lines
(quoted diff) +0 -2
-L/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/lib
-lSDL2 -lpthread

I'm finally looking at sdl2, and added the SDL2_CONFIG_SCRIPTS line suggested by Vincent. If that's the only issue I find, I'll apply the patch with this change.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#23 lore raw

PATCH v4 1/1 sdl2: new package

Hi Thomas,

Le 21/10/2015 23:22, Thomas Petazzoni a ?crit :

Quoted 21 lines

Romain,

Quoted 17 lines · Romain Naour

I confirm there is a poisoned path returned by this script:

sdl2-config --cflags

(quoted diff) +0 -3
-I/usr/include/SDL2
-I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include
-D_REENTRANT

sdl2-config --libs
-L/usr/lib -lSDL2 -lpthread

And thanks to your fix:

sdl2-config --cflags

(quoted diff) +0 -3
-I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include/SDL2
-I/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/include
-D_REENTRANT
Quoted 7 lines

Review

Quoted 3 lines

sdl2-config --libs

(quoted diff) +0 -2
-L/home/naourr/buildroot/test/efl-1.15.2-v2/host/usr/i686-buildroot-linux-gnu/sysroot/usr/lib
-lSDL2 -lpthread

I'm finally looking at sdl2, and added the SDL2_CONFIG_SCRIPTS line suggested by Vincent. If that's the only issue I find, I'll apply the patch with this change.

Ok, thanks !

Best regards,
Romain

Quoted 3 lines

Thomas

#24 lore raw

PATCH v4 1/1 sdl2: new package

In reply to Guillaume GARDET
Quoted 4 lines · Guillaume GARDET
Signed-off-by Guillaume GARDET <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>
Cc Yann E. Morin <[email protected]>

Since you're anyway spinning a v5, one more comment:

Quoted 2 lines

---

[snip]

package/Config.in endif · line 249

Quoted 9 lines
package/Config.in +1 -0
index 13a7e74..3a1b5c6 100644
@@ -246,6 +246,7 @@ endif
 	source "package/psplash/Config.in"
 	source "package/sawman/Config.in"
 	source "package/sdl/Config.in"
+	source "package/sdl2/Config.in"

Even though this is correct alphabetical ordering, it's not so good. Currently,
when you select sdl, the SDL modules, below, will be indented in menuconfig so it is clear they belong to the SDL package. By putting sdl2 in-between, the indentation will be gone.

Actually, the sdl_* packages should not have this 'depends on ' line but
instead there should be a condition here, then it would be much clearer. But it's not a perfect world...

Regards,
Arnout

Quoted 3 lines
package/Config.in +0 -0
 	source "package/sdl_gfx/Config.in"
 	source "package/sdl_image/Config.in"
 	source "package/sdl_mixer/Config.in"

[snip]

Signature · 6 lines
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
#25 lore raw

PATCH v2 1/1 sdl2: new package

In reply to Guillaume GARDET

Guillaume GARDET <[email protected]>
wrote in news:1427721269-8267-1-git-send-email-guillaume.gardet at oliseo.fr:

Quoted 1 line

Changes in V2:

Hi,

I used your patch to enable sdl2 support in the fs-uae (http://fs-uae.net)
package I am currently coding (patch series to be sent). At least it build fine with your sdl2 package, but I need to do some runtime testing.

Regards, Bernd