[Bug 848144] Review Request: SDL2 A cross-platform multimedia library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=848144

Paulo Andrade <paulo.cesar.pereira.de.andrade@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |paulo.cesar.pereira.de.andr
                   |                            |ade@xxxxxxxxx

--- Comment #9 from Paulo Andrade <paulo.cesar.pereira.de.andrade@xxxxxxxxx> ---
(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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=OpAzBOOAaU&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]