This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 848144 - Review Request: SDL2 A cross-platform multimedia library
Review Request: SDL2 A cross-platform multimedia library
Status: CLOSED DUPLICATE of bug 989752
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
: 767528 (view as bug list)
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2012-08-14 14:24 EDT by MERCIER Jonathan
Modified: 2013-07-30 04:42 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-06-26 19:35:01 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
spec cleanup (2.16 KB, patch)
2013-01-03 15:28 EST, Michael Schwendt
no flags Details | Diff
SDL2.spec (4.15 KB, text/plain)
2013-01-17 20:44 EST, Paulo Andrade
no flags Details

  None (edit)
Description MERCIER Jonathan 2012-08-14 14:24:29 EDT
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 18:16:42 EDT
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 19:03:44 EDT
(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 04:31:41 EDT
Upstream provides snapshots <http://www.libsdl.org/tmp/>.
Comment 4 Petr Pisar 2012-09-05 04:40:57 EDT
*** Bug 767528 has been marked as a duplicate of this bug. ***
Comment 5 MERCIER Jonathan 2012-09-21 16:46:00 EDT
$ 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-08 19:13:13 EST
404 not found for both downloads
Comment 7 MERCIER Jonathan 2012-12-09 12:33:07 EST
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 15:28:22 EST
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 10:24:36 EST
(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-17 20:44:02 EST
Created attachment 681954 [details]
SDL2.spec

Sample spec with my suggestions and patch in previous attachment.
Comment 11 Paulo Andrade 2013-01-21 10:57:43 EST
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 17:24:11 EDT
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 19:35:01 EDT
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 04:40:27 EDT
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.