Spec URL: http://mmornati.fedorapeople.org/wbfs-manager/wbfs-manager.spec SRPM URL: http://mmornati.fedorapeople.org/wbfs-manager/wbfs-manager-0.1.11-1.fc12.src.rpm Description: WBFS management is a GTK manager to copy file into Nintendo File System. The official project page is locate to: http://code.google.com/p/linux-wbfs-manager/
*** Bug 554659 has been marked as a duplicate of this bug. ***
Just some comments after a quick look at your spec file. - The summary needs some love https://fedoraproject.org/wiki/Examples_of_good_package_summaries - Lines in the %description seems to be to long - Remove %pre, %post, and %postun they are useless - gcc is not needed https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2 - All GUI applications need a .desktop file https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files - Please preserve the time stamps https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps - Isn't parallel building working? https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make
Thanks a lot for your suggestions Fabian. I've uploaded the new spec file (http://mmornati.fedorapeople.org/wbfs-manager/wbfs-manager.spec) following your comments. I'm not sure where can I add the png icon to use with .desktop file. Is it correct to add a resources as a "SourceX" in the spec file? (The .destop is created directly inside spec) Thanks a lot Marco
I've just been through learning where to put icons for one of my own packages. Apparently the preferred location is /usr/share/pixmaps. I can only offer an informal review, someone else will have to do an official one. I'm not sure whether removing the "linux-" prefix from the upstream naming is a good idea. I can see why it would be desirable, and I'd personally prefer it, but I can't find anything in the Fedora naming guidelines to support it: "When naming a package, the name should match the upstream tarball or project name from which this software came." http://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming Becuase of this, the Source0 tag and setup macro aren't working as written. The Source0 tag should apparently use "linux-%{name}-%{version}.tar.gz", and the corresponding setup macro should be "%setup -q -n linux-%{name}" (no -%{version} because the tarball doesn't use that in the path). rpmlint reports: rpmlint wbfs-manager.spec ../RPMS/x86_64/wbfs-manager-* wbfs-manager.spec:65: W: macro-in-%changelog %pre wbfs-manager.x86_64: W: no-documentation wbfs-manager.x86_64: E: script-without-shebang /usr/share/applications/wbfsmanager.desktop wbfs-manager-debuginfo.x86_64: E: empty-debuginfo-package defattr should be "%defattr(-,root,root,-)"; if you need 0755 permissions on some files, you should arrange for that in the build or install sections. That should fix the "script-without-shebang" error. Should probably use install rather than cp to install the executable, e.g. install -D -p -m 755 lmdemo %{buildroot}%{_bindir}/lmdemo The make invocation should include CFLAGS="%{optflags}". I'd thought that might fix the empty-debuginfo-package warning, but it doesn't, possibly because the Makefile isn't using CFLAGS for the link. MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK MUST: The spec file for the package is legible and macros are used consistently. NEEDSWORK -- spec uses both $RPM_BUILD_ROOT and %{buildroot} MUST: The package must be named according to the Package Naming Guidelines. probably OK? see what an experienced reviewer says MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK MUST: The License field in the package spec file must match the actual license. OK MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. NEEDSWORK - just fix the Source0 tag as described above MUST: The package MUST successfully compile and build into binary rpms. NEEDSWORK - just fix the setup macro as described above MUST: The spec file MUST handle locales properly. N/A MUST: Optflags are used and time stamps preserved. NEEDSWORK MUST: Packages containing shared library files must call ldconfig. N/A MUST: A package must own all directories that it creates or require the package that owns the directory. OK MUST: Files only listed once in %files listings. OK MUST: Debuginfo package is complete. NEEDSWORK MUST: Permissions on files must be set properly. OK MUST: Clean section exists. OK MUST: Large documentation files must go in a -doc subpackage. N/A MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK MUST: Header files must be in a -devel package. N/A MUST: Static libraries must be in a -static package. N/A MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A MUST: Packages does not contain any .la libtool archives. OK MUST: Desktop files are installed properly. NEEDSWORK add icon as described above, and need to run desktop-file-install or desktop-file-validate http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files MUST: No file conflicts with other packages and no general names. OK MUST: Buildroot cleaned before install. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. NEEDSWORK SHOULD: The package builds in mock. NEEDSWORK - if the Source0 tag and setup macro are fixes as described above, that will probably take care of mock.
Assigning.
MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK MUST: The spec file for the package is legible and macros are used consistently. NEEDSWORK - Create desktop file in %prep, not in %install. - IMHO too much empty lines within sections. - Use of macros for standard commands is OK, although unnecessary. MUST: The package must be named according to the Package Naming Guidelines. NEEDSWORK - The naming guidelines clearly indicate that the package name should be linux-wbfs-manager. http://fedoraproject.org/wiki/Packaging/NamingGuidelines MUST: The spec file name must match the base package %{name}. OK - For now. MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK MUST: The License field in the package spec file must match the actual license. NEEDSWORK - Not all files are GPLv2: /* Rijndael Block Cipher - rijndael.c Written by Mike Scott 21st April 1999 mike.ie Permission for free direct or derivative use is granted subject to compliance with any conditions that the originators of the algorithm place on its exploitation. */ MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. NEEDSWORK - URL not functioning. MUST: The package MUST successfully compile and build into binary rpms. NEEDSWORK - Package does not compile. MUST: The spec file MUST handle locales properly. N/A MUST: Optflags are used and time stamps preserved. NEEDSWORK - Optimization flags are not used. Using make CFLAGS="%{optflags}" %{?_smp_mflags} does the trick. MUST: Packages containing shared library files must call ldconfig. N/A MUST: A package must own all directories that it creates or require the package that owns the directory. N/A MUST: Files only listed once in %files listings. ~OK - Remove the empty %dir line. MUST: Debuginfo package is complete. NEEDSWORK - Debuginfo is empty. MUST: Permissions on files must be set properly. NEEDSWORK - Use %defattr(-,root,root,-). MUST: Clean section exists. OK MUST: Large documentation files must go in a -doc subpackage. N/A MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. NEEDSWORK - Add README to %doc. MUST: Header files must be in a -devel package. N/A MUST: Static libraries must be in a -static package. N/A MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A MUST: Packages does not contain any .la libtool archives. N/A MUST: Desktop files are installed properly. NEEDSWORK - Use desktop-file-install to install the desktop file. MUST: No file conflicts with other packages and no general names. OK MUST: Buildroot cleaned before install. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. NEEDSWORK - License not included in tarball. SHOULD: The package builds in mock. NEEDSWORK - Does not build at all, when it is fixed it builds in mock.
Oh, and as Eric pointed out you are also mixing macros, which is not allowed.
As was quite clear from the quality of the spec file, Marco is not sponsored and is probably looking for one. Added FE-NEEDSPONSOR.
More notes: GenericName=WBFS Manager GenericName[it]=WBFS Manager is redundant, drop the Italian name. ** Drop the .png suffix in Icon=wbfs_gtk.png the system will automatically determine the best file to use.
The rijndael.c code license is non-free because it does not give permission to freely redistribute. We either need to get that permission from the copyright holder, or we need to replace that code with something under a Free and GPLv2 compatible license. There is a list of known implementations here: http://en.wikipedia.org/wiki/AES_implementations
No activity in 5 months, closing review. Removing FE-LEGAL and FE-NEEDSPONSOR, adding FE-DEADREVIEW.