sdl2: new package
PATCH 1/1 sdl2: new package
--- 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
@@ -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
@@ -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
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 100644package/sdl2/sdl2.mkpackage/Config.in +1
-0@@ -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@@ -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 + +endifpackage/sdl2/sdl2.mk new file +67
-0@@ -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))
PATCH 1/1 sdl2: new package
In reply to Guillaume GARDETDear 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@@ -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@@ -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
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@@ -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.configYou 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_DIRECTFBNo 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@@ -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-staticHum, 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-configpackage/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 +endifPlease 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 +endifSame: --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 +endifWhat 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-esdPlease put this earlier, where the first non-conditional
SDL2_CONF_OPTSdefinition 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-esdNot 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
PATCH 1/1 sdl2: new package
In reply to Thomas PetazzoniHi,
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.
This one fails with:
"dlfcn.h: No such file or directory"
WHat would be the right "dpends on" option?
Guillaume
PATCH 1/1 sdl2: new package
Dear Guillaume GARDET - Olis?o,
Quoted 6 lines · Guillaume GARDET - Olis?o
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
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
--- 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
@@ -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
@@ -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
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 unconditionnalSDL2_CONF_OPTSupper
* Remove host buildSigned-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 100644package/sdl2/sdl2.mkpackage/Config.in +1
-0@@ -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@@ -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 + +endifpackage/sdl2/sdl2.mk new file +62
-0@@ -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))
PATCH v2 1/1 sdl2: new package
In reply to Guillaume GARDETHi 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 unconditionnalSDL2_CONF_OPTSupper
* 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.mkpackage/Config.in +1
-0@@ -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@@ -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+ +endifpackage/sdl2/sdl2.mk new file +1
-0@@ -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
PATCH v3 1/1 sdl2: new package
---
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
@@ -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
@@ -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
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@@ -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
[ ] barwhich 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. | '------------------------------^-------^------------------^--------------------'
PATCH v4 1/1 sdl2: new package
---
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
@@ -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
@@ -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
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.inChanges 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 fixChanges 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 unconditionnalSDL2_CONF_OPTSupper
* Remove host buildpackage/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
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.inChanges 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 fixChanges 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 unconditionnalSDL2_CONF_OPTSupper
* Remove host buildpackage/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 100644package/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
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
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
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
PATCH v4 1/1 sdl2: new package
In reply to Guillaume GARDET - OliséoHi 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
PATCH v4 1/1 sdl2: new package
In reply to Guillaume GARDET - OliséoHi 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.inChanges 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 fixChanges 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 unconditionnalSDL2_CONF_OPTSupper
* Remove host buildpackage/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 100644package/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_libXcursorSDL2_CONF_OPTS += --enable-video-x11-xcursor
elseSDL2_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 supportNo, 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
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.
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-configWithout this line, the include path returned by
sdl2-config --cflagswill 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.
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-configWithout this line, the include path returned by
sdl2-config --cflagswill 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
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_REENTRANTsdl2-config --libs
-L/usr/lib -lSDL2 -lpthreadAnd 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_REENTRANTsdl2-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
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_REENTRANTsdl2-config --libs
-L/usr/lib -lSDL2 -lpthreadAnd 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 -lpthreadI'm finally looking at sdl2, and added the
SDL2_CONFIG_SCRIPTSline 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
PATCH v4 1/1 sdl2: new package
In reply to Guillaume GARDETQuoted 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@@ -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
-0source "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
PATCH v2 1/1 sdl2: new package
In reply to Guillaume GARDETGuillaume 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