Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 848144 - Review Request: SDL2 A cross-platform multimedia library
Summary: Review Request: SDL2 A cross-platform multimedia library
Keywords:
Status: CLOSED DUPLICATE of bug 989752
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 767528 (view as bug list)
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2012-08-14 18:24 UTC by MERCIER Jonathan
Modified: 2013-07-30 08:42 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-06-26 23:35:01 UTC


Attachments (Terms of Use)
spec cleanup (deleted)
2013-01-03 20:28 UTC, Michael Schwendt
no flags Details | Diff
SDL2.spec (deleted)
2013-01-18 01:44 UTC, Paulo Andrade
no flags Details


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 767528 None None None Never

Internal Links: 767528

Description MERCIER Jonathan 2012-08-14 18:24:29 UTC
spec: http://bioinfornatics.fedorapeople.org/SDL2.spec
srpms: http://bioinfornatics.fedorapeople.org/SDL2-2-1.20120812hg9612bcd79130.fc17.3.src.rpm
koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=4390642

$ rpmlint $(find ~/rpmbuild/RPMS ~/rpmbuild/SRPMS -iname "*.rpm")
SDL2-devel.x86_64: E: rpath-in-buildconfig /usr/lib64/pkgconfig/sdl2.pc lines ['13']
SDL2-devel.x86_64: E: rpath-in-buildconfig /usr/bin/sdl2-config lines ['48']
SDL2-devel.x86_64: W: no-manual-page-for-binary sdl2-config
SDL2.x86_64: W: spelling-error Summary(fr) multi -> mufti, multiple
SDL2.x86_64: W: spelling-error %description -l fr multi -> mufti, multiple
SDL2.src: W: spelling-error Summary(fr) multi -> mufti, multiple
SDL2.src: W: spelling-error %description -l fr multi -> mufti, multiple
SDL2.src: W: invalid-url Source0: SDL2-20120812hg9612bcd79130.tar.xz
4 packages and 0 specfiles checked; 2 errors, 6 warnings.

Comment 1 Christophe Fergeau 2012-08-14 22:16:42 UTC
Release:        1.%{alphatag}%{?dist}.3

The .3 should not be here, and I'm unsure about the 1. prefix (looking at http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages )

Why this specific revision (9612bcd79130)? Is it recommended by upstream? Used by other distros? or just random?

The BuildRequires: geany and generation of the geany tags are unneeded and don't belong there imo.

chmod 644 $(find src \( -name "*.c" -or -name "*.h" \) )
Do you get any issues if you don't change the file permissions? If yes, this should be mentioned in the comment above, if not, I think you can tell rpm to adjust the file permissions for you in %file

Comment 2 MERCIER Jonathan 2012-08-14 23:03:44 UTC
(In reply to comment #1)
> Release:        1.%{alphatag}%{?dist}.3
> 
> The .3 should not be here, and I'm unsure about the 1. prefix (looking at
> http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages )

I agree .3 is an artifact

> Why this specific revision (9612bcd79130)? Is it recommended by upstream?
> Used by other distros? or just random?

Not recommended by upstream i taken the last revision from mercurial repo. I will work with upstream. As achlinux has already SDL2 into their repository they are no reason to do same. In more SDL do not override SDL 1.2 .

> The BuildRequires: geany and generation of the geany tags are unneeded and
> don't belong there imo.

Ok it was to help developer.

> chmod 644 $(find src \( -name "*.c" -or -name "*.h" \) )
> Do you get any issues if you don't change the file permissions? If yes, this
> should be mentioned in the comment above, if not, I think you can tell rpm
> to adjust the file permissions for you in %file

debuginfo take this sources files then %attr is not useful here

spec: http://bioinfornatics.fedorapeople.org/SDL2.spec
srpms: http://bioinfornatics.fedorapeople.org/SDL2-2-2.20120812hg9612bcd79130.fc17.src.rpm

thanks

Comment 3 Petr Pisar 2012-09-05 08:31:41 UTC
Upstream provides snapshots <http://www.libsdl.org/tmp/>.

Comment 4 Petr Pisar 2012-09-05 08:40:57 UTC
*** Bug 767528 has been marked as a duplicate of this bug. ***

Comment 5 MERCIER Jonathan 2012-09-21 20:46:00 UTC
$ rpmlint /home/builder/rpmbuild/SRPMS/SDL2-2.0.0-3.fc17.src.rpm /home/builder/rpmbuild/RPMS/x86_64/SDL2-2.0.0-3.fc17.x86_64.rpm /home/builder/rpmbuild/RPMS/x86_64/SDL2-devel-2.0.0-3.fc17.x86_64.rpm /home/builder/rpmbuild/RPMS/x86_64/SDL2-debuginfo-2.0.0-3.fc17.x86_64.rpm
SDL2.src: W: spelling-error Summary(fr) multi -> mufti, multiple
SDL2.src: W: spelling-error %description -l fr multi -> mufti, multiple
SDL2.x86_64: W: spelling-error Summary(fr) multi -> mufti, multiple
SDL2.x86_64: W: spelling-error %description -l fr multi -> mufti, multiple
SDL2-devel.x86_64: W: no-manual-page-for-binary sdl2-config
4 packages and 0 specfiles checked; 0 errors, 5 warnings

spec: http://bioinfornatics.fedorapeople.org/SDL2.spec
srpms: http://bioinfornatics.fedorapeople.org/SDL2-2.0.0-3.fc17.src.rpm

Comment 6 Michael Schwendt 2012-12-09 00:13:13 UTC
404 not found for both downloads

Comment 7 MERCIER Jonathan 2012-12-09 17:33:07 UTC
yes that is true i update the organization. Now i put all my pending package into http://bioinfornatics.fedorapeople.org/packages/

srpms: http://bioinfornatics.fedorapeople.org/packages/SDL2-2-2.20120812hg9612bcd79130.fc17.src.rpm

spec: http://bioinfornatics.fedorapeople.org/packages/SDL2.spec

Comment 8 Michael Schwendt 2013-01-03 20:28:22 UTC
Created attachment 672205 [details]
spec cleanup

* The linked spec file is an older one. The src.rpm is much newer.

* As I've noticed lots of  "no"  results for the various checks during the configure step, I skimmed over the spec file and fixed several minor issues. The diff should be self-explaining.

* SDL2-devel.x86_64 will conflict with SDL2-devel.i686 due to the sdl2-config script

* .pc file:

  $ pkg-config sdl2 --libs
  -Wl,-rpath,/usr/lib64 -lSDL2 -lpthread 

It includes duplicated -lpthread options and another -lSDL2 in the .private section:

  $ grep pth /usr/lib64/pkgconfig/sdl2.pc 
  Libs: -L${libdir} -Wl,-rpath,${libdir} -lSDL2  -lpthread
  Libs.private: -lSDL2  -lpthread  -lm -ldl -lpthread


* Several build requirements seem to be missing. The test programs in the "test" subdirectory fail to build due to that.

* If this SDL2 is rebuilt with added BuildRequires, the tests can be built, too.

The resulting binary rpm is missing shared library dependencies. Oh, the libs are loaded dynamically by SDL - run-time RPM dependencies will be needed for them, too, however, probably not limited to these:

$ grep DYNA config.status
D["SDL_AUDIO_DRIVER_ALSA_DYNAMIC"]=" \"libasound.so.2\""
D["SDL_AUDIO_DRIVER_PULSEAUDIO_DYNAMIC"]=" \"libpulse-simple.so.0\""
D["SDL_VIDEO_DRIVER_X11_DYNAMIC"]=" \"libX11.so.6\""
D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XEXT"]=" \"libXext.so.6\""
D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XCURSOR"]=" \"libXcursor.so.1\""
D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XINERAMA"]=" \"libXinerama.so.1\""
D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XINPUT2"]=" \"libXi.so.6\""
D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XRANDR"]=" \"libXrandr.so.2\""
D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XVIDMODE"]=" \"libXxf86vm.so.1\""

Comment 9 Paulo Andrade 2013-01-14 15:24:36 UTC
(In reply to comment #8)

  I would like to see a SDL2 package as I almost finished a sample
package for http://te4.org but it needs SDL2 (the compatibility layer
for SDL 1.2 is broken in the final release).

> Created attachment 672205 [details]
> spec cleanup

  My comments after applying this patch.

> * The linked spec file is an older one. The src.rpm is much newer.
> 
> * As I've noticed lots of  "no"  results for the various checks during the
> configure step, I skimmed over the spec file and fixed several minor issues.

What I did not have installed in a "standard" rawhide:

# fatal error: audio/audiolib.h: No such file or directory
BuildRequires: nas-devel
# fatal error: X11/extensions/scrnsaver.h: No such file or directory
BuildRequires: libXScrnSaver-devel
# fatal error: GLES/gl.h: No such file or directory
BuildRequires: mesa-libGLES-devel
# fatal error: tslib.h: No such file or directory
BuildRequires: tslib-devel
# fatal error: usb.h: No such file or directory
BuildRequires: libusb-devel

These I presume are missing (cannot fool proof test it right
now because mock is broken in rawhide #894623):
BuildRequires: alsa-lib-devel
BuildRequires: mesa-libGL-devel
BuildRequires: libXrandr-devel
BuildRequires: libXi-devel
BuildRequires: libXinerama-devel
BuildRequires: libXcursor-devel

> The diff should be self-explaining.
> 
> * SDL2-devel.x86_64 will conflict with SDL2-devel.i686 due to the
> sdl2-config script

  I think this is common practice, just do "repoquery -f"
in a few /usr/bin/*-config to verify

> * .pc file:
> 
>   $ pkg-config sdl2 --libs
>   -Wl,-rpath,/usr/lib64 -lSDL2 -lpthread 
> 
> It includes duplicated -lpthread options and another -lSDL2 in the .private
> section:
> 
>   $ grep pth /usr/lib64/pkgconfig/sdl2.pc 
>   Libs: -L${libdir} -Wl,-rpath,${libdir} -lSDL2  -lpthread
>   Libs.private: -lSDL2  -lpthread  -lm -ldl -lpthread

should also remove the -rpath

> * Several build requirements seem to be missing. The test programs in the
> "test" subdirectory fail to build due to that.
> 
> * If this SDL2 is rebuilt with added BuildRequires, the tests can be built,
> too.
> 
> The resulting binary rpm is missing shared library dependencies. Oh, the
> libs are loaded dynamically by SDL - run-time RPM dependencies will be
> needed for them, too, however, probably not limited to these:
> 
> $ grep DYNA config.status
> D["SDL_AUDIO_DRIVER_ALSA_DYNAMIC"]=" \"libasound.so.2\""
> D["SDL_AUDIO_DRIVER_PULSEAUDIO_DYNAMIC"]=" \"libpulse-simple.so.0\""
> D["SDL_VIDEO_DRIVER_X11_DYNAMIC"]=" \"libX11.so.6\""
> D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XEXT"]=" \"libXext.so.6\""
> D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XCURSOR"]=" \"libXcursor.so.1\""
> D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XINERAMA"]=" \"libXinerama.so.1\""
> D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XINPUT2"]=" \"libXi.so.6\""
> D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XRANDR"]=" \"libXrandr.so.2\""
> D["SDL_VIDEO_DRIVER_X11_DYNAMIC_XVIDMODE"]=" \"libXxf86vm.so.1\""

These should be added as Requires, e.g.:

Requires: libasound
Requires: pulseaudio-libs
Requires: libXcursor
Requires: libXinerama
Requires: libXi
Requires: libXrandr

others should be automatically required by mesa-libGL, and a few
of the above already required by any desktop environment, but
better to have proper requires.


Extra suggestions I have:
* Optionally use only %{snapdate} in the release, that is, instead
of SDL2-2-2.20120812hg9612bcd79130 call it SDL2-2-2.20120812,
but keep metainformation in the spec about proper commit.

* Move some README* to the main package, and do not install
others. At least README and README-SDL.txt should be in the
main package:
[...]
Please distribute this file with the SDL runtime environment:
[...]
.android, .iOS, .MacOSX, .WinCE should not be installed.

* Instead of removing the .a libraries, maybe create a
-static package. Not something to encourage, but static
linking would be a way to have some package not breaking
in the near future.

(In reply to comment #3)
> Upstream provides snapshots <http://www.libsdl.org/tmp/>.

Probably better to use the upstream snapshots also. The
oldest snapshots appear to be one year old.

Comment 10 Paulo Andrade 2013-01-18 01:44:02 UTC
Created attachment 681954 [details]
SDL2.spec

Sample spec with my suggestions and patch in previous attachment.

Comment 11 Paulo Andrade 2013-01-21 15:57:43 UTC
Note that only a SDL2 package would not be enough.
I did some experiments with SDL_ttf built on top
of SDL2. Needs a lot of patching, and the "trivial"
patch would just create a SDL_ttf that conflicts
with the one based on SDL 1.2, so, needs massive
patching to call it SDL2_ttf, that is, basically
a s/SDL/SDL2/ s/sdl/sdl2/ everywhere but a few
places, e.g. need to still call the header SDL.h,
what breaks auto{conf,make} implicit rules in
configure.* and Makefile.*

Comment 12 MERCIER Jonathan 2013-06-26 21:24:11 UTC
sorry but finally i do not use enough SDL2 to be a good package maintener.
if someone could take SDL2 package …

Comment 13 Jason Tibbitts 2013-06-26 23:35:01 UTC
Guess I'll close this, then.  If someone else wants to submit this package, please feel free to open a new ticket.

Comment 14 Björn 'besser82' Esser 2013-07-30 08:40:27 UTC
Has been resubmitted by ignatenkobrain.

*** This bug has been marked as a duplicate of bug 989752 ***


Note You need to log in before you can comment on or make changes to this bug.