Spec URL: http://people.atrpms.net/~hdegoede/arm-gp2x-linux-SDL.spec SRPM URL: http://people.atrpms.net/~hdegoede/arm-gp2x-linux-SDL-1.2.9-2.fc8.src.rpm Description: This is a Cross Compiled version of the SDL Library, which can be used to compile and link binaries for the arm-gp2x-linux platform, instead of for the native i386 platform.
Reviewing this one.
This one is also missing the %defattr in the %files section.
Can you please submit an updated specfile for this with the missing %defattr added and the License tag updated to the new standard (specifying LGPL version) so we don't waste time on these during the review?
New version with these changes: * Sat Aug 4 2007 Hans de Goede <j.w.r.degoede> 1.2.9-3 - Add missing %%defattr to %%files - Strip some unneeded file removals / additions from Patch0, dramatically reducing it in size - Update License tag for new Licensing Guidelines compliance - Do NOT provide and require native .so files <sigh> here: Spec URL: http://people.atrpms.net/~hdegoede/arm-gp2x-linux-SDL.spec SRPM URL: http://people.atrpms.net/~hdegoede/arm-gp2x-linux-SDL-1.2.9-3.fc8.src.rpm
Currently going through the review checklist. Any particular reason you aren't using %{?_smp_mflags}? The native SDL package is built with it (and has been even back when it was at 1.2.9).
(In reply to comment #5) > Currently going through the review checklist. Any particular reason you aren't > using %{?_smp_mflags}? The native SDL package is built with it (and has been > even back when it was at 1.2.9). No reason, probably a cut and paste error.
MUST Items: + rpmlint output: + SRPM: W: arm-gp2x-linux-SDL invalid-license LGPLv2+ This is the F7 rpmlint being out of date. :-) E: arm-gp2x-linux-SDL configure-without-libdir-spec This one's OK for a cross library. (There's no 64-bit GP2X ;-).) + noarch RPM: W: arm-gp2x-linux-SDL devel-file-in-non-devel-package /usr/arm-gp2x-linux/include/SDL/SDL_cpuinfo.h (and more like this). OK because this is a cross-development package, and these are all target development files. It would make no sense to make a separate devel vs. runtime part because we aren't going to run ARM GP2X binaries on i386/x86_64/ppc/... Fedora anyway. E: arm-gp2x-linux-SDL library-without-ldconfig-postin /usr/arm-gp2x-linux/lib/libSDL-1.2.so.0.7.2 E: arm-gp2x-linux-SDL library-without-ldconfig-postun /usr/arm-gp2x-linux/lib/libSDL-1.2.so.0.7.2 OK, as these are target libraries which aren't even in the ldconfig search path. The required symlinks are already there anyway. W: arm-gp2x-linux-SDL invalid-license LGPLv2+ Again, the F7 rpmlint being out of date. E: arm-gp2x-linux-SDL arch-independent-package-contains-binary-or-object /usr/arm-gp2x-linux/lib/libSDLmain.a E: arm-gp2x-linux-SDL arch-independent-package-contains-binary-or-object /usr/arm-gp2x-linux/lib/libSDL.a E: arm-gp2x-linux-SDL arch-independent-package-contains-binary-or-object /usr/arm-gp2x-linux/lib/libSDL-1.2.so.0.7.2 Again, this is OK because those are target files. W: arm-gp2x-linux-SDL non-standard-dir-in-usr arm-gp2x-linux This one's OK too for a cross-library package. + named and versioned according to the Package Naming Guidelines + spec file name matches base package name + Packaging Guidelines: + License LGPLv2+ OK, matches actual license + No known patent problems + No emulator, no firmware, no binary-only or prebuilt components + Complies with the FHS (with the cross-toolchain exception for %{_prefix}/%{target}) + proper changelog, tags, BuildRoot, Requires, BuildRequires, Summary, Description + no non-UTF-8 characters + relevant documentation is included + It would make no sense to use RPM_OPT_FLAGS here because this is a target package, which is built using a cross GCC which won't understand stuff like -fstack-protector, and for which x86 -march and -mtune switches definitely don't make sense. Thus the omission of RPM_OPT_FLAGS is correct. + no -debuginfo package because this is noarch + no host static libraries nor .la files (I think we can give the target static libraries a pass. This isn't a Fedora target, so trying to apply our static library policies to the target wouldn't make much sense.) + no duplicated system libraries + no rpaths (no host executables or libraries at all, I also ran readelf -d on the target shared library to make sure and there's no rpath there either) + no configuration files, so %config guideline doesn't apply + no init scripts, so init script guideline doesn't apply + no GUI programs, so no .desktop file present or needed + no timestamp-clobbering file commands ! _smp_mflags not used + scriptlets are valid + not a web application, so web application guideline doesn't apply + no conflicts + complies with all the legal guidelines + COPYING (yes, it's the LGPL, even if it's not named COPYING.LIB) included as %doc + spec file written in American English + spec file is legible + source matches upstream: MD5SUM: 80919ef556425ff82a8555ff40a579a0 SHA1SUM: 8140de00e73ccdbdee196fa8fd9952ddb3cc75f1 (This one looks like a pretty pointless exercise given the 380 KB patch applied to it, but... ;-) ) + builds on at least one arch (F7 i386 live system) + no known non-working arches, so no ExcludeArch needed + no missing BR + no translations, so translation/locale guidelines don't apply + no host shared libraries, so no ldconfig calls needed + package not relocatable + ownership correct (owns package-specific directories, doesn't own directories owned by another package) + no duplicate files in %files + permissions set properly (%defattr present) + %clean section present and correct + macros used where possible (%configure not used for several reasons, including it playing jokes with --target and using host-specific RPM_OPT_FLAGS) + no non-code content + no large documentation files, so no -doc package needed + %doc files not required at runtime + no host headers, target headers are OK in this cross-development package + no host static libraries, so no -static package needed + no .pc files, so no Requires: pkgconfig needed + no host shared libraries, so .so symlink guidelines don't apply + no -devel package, so the guideline to require the main package in it doesn't apply + no .la files + no GUI programs, so no .desktop file needed + buildroot is deleted at the beginning of %install (same nitpick about mkdir $RPM_BUILD_ROOT as for arm-gp2x-linux-binutils) + all filenames are valid UTF-8 SHOULD Items: + license already included upstream + no translations for description and summary provided by upstream * Skipping mock test. * Skipping the "all architectures" test, I only have i386. + package functions as described: The following test program: #include <stdlib.h> #include <SDL.h> int main(void) { if (SDL_Init(SDL_INIT_AUDIO|SDL_INIT_VIDEO) < 0) { fprintf(stderr, "Unable to init SDL: %s\n", SDL_GetError()); return 1; } atexit(SDL_Quit); return 0; } compiles and links fine using: arm-gp2x-linux-gcc -Wall -Wextra -Os gp2x-sdl-test.c `arm-gp2x-linux-sdl-config --cflags --libs` -o gp2x-sdl-test + scriptlets are sane + no subpackages other than -devel, so "Usually, subpackages other than devel should require the base package using a fully versioned dependency." is irrelevant + no .pc files, so "placement of .pc files" is irrelevant + no file dependencies MUST FIX: * please use _smp_mflags as there's no good reason not to
Many thanks for the review! (In reply to comment #7) > MUST FIX: > * please use _smp_mflags as there's no good reason not to Fixed, new version here: Spec URL: http://people.atrpms.net/~hdegoede/arm-gp2x-linux-SDL.spec SRPM URL: http://people.atrpms.net/~hdegoede/arm-gp2x-linux-SDL-1.2.9-4.fc8.src.rpm p.s. I don't want to discourage you / abuse your kindness, but SDL and zlib are just the top of the mountain, to be able to provide a full SDK on which most gp2x software can be found, the following are needed to: tremor (integer vorbis decoder) libogg SDL_mixer libpng libjpg libtiff SDL_image And I might be forgetting some. I haven't submitted these yet as I still need to clean them up, and I first wanted to get some more experience, but I will be slowly submitting them in the near future, now that the base is there. Once I've submitted a few I'll drop you a note, if you don't feel like reviewing these too thats 100% understandable, no hard feelings.
Verified fixed. => APPROVED For the other libraries needed for a complete SDK: We'll see about these once you submit them. We can still hope that the presence of the base packages (especially the really hard ones like GCC) will motivate more people to help out. :-) Otherwise, I will, it would be sad to see half a SDK in and the other half stuck in the review queue forever. It's too bad there aren't more people interested in cross-toolchains.
New Package CVS Request ======================= Package Name: arm-gp2x-linux-SDL Short Description: Cross Compiled SDL Library targeted at arm-gp2x-linux Owners: j.w.r.degoede Branches: FC-6 F-7 devel InitialCC: <empty>
Cvs done.
imported and build for rawhide (rest is slow due to new F-7 procedures delaying needed other bits)
Package Change Request ====================== Package Name: arm-gp2x-linux-SDL New Branches: el6 Owners: jwrdegoede
Git done (by process-git-requests).