Spec URL: http://ecik.zspswidwin.pl/mutagen/mutagen.spec SRPM URL: http://ecik.zspswidwin.pl/mutagen/mutagen-1.5.1-1.src.rpm Description: Hi! Recently, I was writing an application which uses mutagen to handle metadata. So I think that would be good if all packages needed by my app was in Extras! Mutagen supports reading ID3 (all versions), APEv2, FLAC, and Ogg Vorbis/FLAC/Theora. It can write ID3v1.1, ID3v2.4, APEv2, FLAC, and Ogg Vorbis/FLAC/Theora comments. It can also read MPEG audio and Xing headers, FLAC stream info blocks, and Ogg Vorbis/FLAC/Theora stream headers.
I forgot to write that this is my first package and I need a sponsor :)
== Not an official review as I'm not yet sponsored == Mock build for rawhide i386 is sucessfull * MUST Items: - rpmlint shows errors as E: mutagen no-binary W: mutagen wrong-file-end-of-line-encoding /usr/share/doc/mutagen-1.5.1/TUTORIAL E: mutagen non-executable-script /usr/lib/python2.4/site-packages/mutagen/__init__.py 0644 - dist tag is present. - The package is named according to the Package Naming Guidelines. - The spec file name matching the base package mutagen, in the format mutagen.spec. - This package meets the Packaging Guidelines. - The spec file for the package MUST be legible. - The package is licensed with an open-source compatible license GPL. - This package includes License file COPYING. - This source package includes the text of the license in its own file,and that file, containing the text of the license for the package is included in %doc. - The sources used to build the package matches the upstream source, as provided in the spec URL. md5sum is correct (9ce5d5f14e02f2eabd919d6bdaebadbc) - This package successfully compiled and built into binary rpms for i386 architecture. - This package did not containd any ExcludeArch. - This package owns all directories that it creates. - This package did not contain any duplicate files in the %files listing. - This package have a %clean section, which contains rm -rf $RPM_BUILD_ROOT. - This package used macros. - Document files are included like COPYING, NEWS, README, TUTORIAL. - Package did NOT contained any .la libtool archives. Also, * Source URL is present and working. * BuildRoot is correct BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) What you Need to Do:- I think you dont need following line if you are building for only fc5 Requires: python-abi = %(%{__python} -c "import sys ; print sys.version[:3]") BuildRequires: python-devel also try to clean %files section of SPEC as %files %defattr(-,root,root,-) %doc COPYING NEWS README TUTORIAL %{_bindir}/* %{_mandir}/man*/*gz %dir %{python_sitelib}/mutagen %{python_sitelib}/mutagen/*.py %{python_sitelib}/mutagen/*.pyc %ghost %{python_sitelib}/mutagen/*.pyo I removes those above Requires and modified %files as shown above and package built without any problem. For rpmlint errors, E: mutagen no-binary There is no mutagen named binary when i did python setup.py build in Source W: mutagen wrong-file-end-of-line-encoding /usr/share/doc/mutagen-1.5.1/TUTORIAL I check TUTORIAL file but did not understood what is the problem. E: mutagen non-executable-script /usr/lib/python2.4/site-packages/mutagen/__init__.py 0644 I found other python packages are also having same __init__.py 0644 permissions. so why rpmlint report problem i confuesd? can anyone comment on this issues?
Created attachment 132479 [details] mutagen-1.5.1-2.spec New SPEC file version (1.5.1-2)
Thanks for review this package. The line: Requires: python-abi = %(%{__python} -c "import sys ; print sys.version[:3]" is from fedora-rpmdevtools template so I didn't change it and, as I see, other SPEC files also has this line so I don't delete it. W: mutagen wrong-file-end-of-line-encoding /usr/share/doc/mutagen-1.5.1/TUTORIAL I checked it and I noticed that TUTORIAL file has Windows-style line endings, so there is \r\n instead of \n. In new SPEC this is fixed. E: mutagen non-executable-script ... The first line of file is #! /usr/bin/env python and for this reason rpmlint treat this file as executable but it shouldn't be executable. I think we can leave it alone. I've done new SPEC with your and my fixes.
(In reply to comment #4) > Thanks for review this package. > > The line: > Requires: python-abi = %(%{__python} -c "import sys ; print sys.version[:3]" > is from fedora-rpmdevtools template so I didn't change it and, as I see, other > SPEC files also has this line so I don't delete it. The python spec template will soon be fixed to get rid of this: http://www.redhat.com/archives/fedora-extras-list/2006-July/msg00557.html If you check the "requires" of the built package on FC4 or later, you should find a dependency on "python(abi) = 2.4" automatically added by RPM, making the python-abi dependency in the spec file redundant. > E: mutagen non-executable-script ... > The first line of file is > #! /usr/bin/env python > and for this reason rpmlint treat this file as executable but it shouldn't be > executable. I think we can leave it alone. rpmlint knows it's not executable but thinks that it's a script (which it isn't) because of the "#! /usr/bin/env python" as you say. You can shut rpmlint up by editing the first line out of the affected files in the %prep stage of the spec.
Created attachment 132496 [details] mutagen-1.5.1-3.spec Okay, I've made new SPEC file with deleted python-abi require and fix in %prep section.
MichaÅ, I'll sponsor you. A quick question before I start the review: why is this package arch-specific? It doesn't seem to contain any compiled code. This results in the following rpmlint warnings: E: mutagen no-binary E: mutagen-debuginfo empty-debuginfo-package which are correct since there are no binary executables and the debuginfo package ends up empty. Adding "BuildArch: noarch" seems to result in a proper package.
Created attachment 132774 [details] mutagen-1.5.1-4.spec Thanks for advice and sponsorship. I noticed that python packages are mostly noarch and I forgot to add it to spec file. So, here is new spec file with no rpmlint errors.
Note: it's nice to the reviewers if you generate a new src.rpm with each change you make to your spec. That way it's simple to just pull down the new package and build it. You seem to have tickled a new rpmlint warning: W: mutagen mixed-use-of-spaces-and-tabs This happened because you indented "noarch" with a tab. Not a big deal but that means it's easy to fix. More serious is the name of the package: according to the naming guidelines this package should be named python-mutagen. See http://fedoraproject.org/wiki/Packaging/NamingGuidelines#AddonPython There's no need to pass CFLAGS to setyp.py since this is a noarch package. This package seems to have a test suite, but you don't call it. You should consider adding a section like: %check %{__python} setup.py coverage Finally, note that if you disagree with me about the necessity of any of these issues I've raised, let me know why you think my reasoning is bogus and we'll discuss it. Review: * source files match upstream: 9ce5d5f14e02f2eabd919d6bdaebadbc mutagen-1.5.1.tar.gz X package meets naming and packaging guidelines (should be called python-mutagen). X specfile is properly named, is cleanly written and uses macros consistently. (looks good but should be named python-mutagen.spec). * dist tag is present. * build root is correct. * license field matches the actual license. * license is open source-compatible. License text included in package. * latest version is being packaged. * BuildRequires are proper. X No need to pass compiler flags for noarch packages. * %clean is present. * package builds in mock (development, x86_64). X rpmlint is silent (spaces and tabs thing) * noarch package; no debuginfo. * final provides and requires are sane: mutagen = 1.5.1-4.fc6 = /usr/bin/python python(abi) = 2.4 X %check is not present but there is a test suite. * no shared libraries are present. * package is not relocatable. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no scriptlets present. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no libtool .la droppings. * not a GUI app.
Created attachment 132784 [details] python-mutagen-1.5.1-5.spec (In reply to comment #9) > Note: it's nice to the reviewers if you generate a new src.rpm with each change > you make to your spec. That way it's simple to just pull down the new package > and build it. Yes, I know, but I have a slow connection shared on 5 computers in home, so sending even 296 kB file blocks it completely and I send such files as rarely as it possible... :/ > You seem to have tickled a new rpmlint warning: > W: mutagen mixed-use-of-spaces-and-tabs > This happened because you indented "noarch" with a tab. Not a big deal but that > means it's easy to fix. This is odd, because when I checked it in my rpmlint, it didn't show any errors, but I fixed it in new spec. > More serious is the name of the package: according to the naming guidelines this > package should be named python-mutagen. See > http://fedoraproject.org/wiki/Packaging/NamingGuidelines#AddonPython I don't know how can I overlooked that :/ You're right and I fixed it. > There's no need to pass CFLAGS to setyp.py since this is a noarch package. Fixed. > This package seems to have a test suite, but you don't call it. You should > consider adding a section like: > > %check > %{__python} setup.py coverage I know, but check procedure looks broken. It shows errors that look like dependent to errors in check procedure, not in program.
Huh, I found some time to send new SPEC and SRPM so it's here: Spec URL: http://ecik.zspswidwin.pl/mutagen/python-mutagen.spec SRPM URL: http://ecik.zspswidwin.pl/mutagen/python-mutagen-1.5.1-5.src.rpm
Nice, thanks. Let's see: Package and specfile name fixed. Compiler flags not passed needlessly. rpmlint placated. Absence of %check section reasonably explained. I did run the checks and it seemed to work but I admit to not understanding the output, and in the end there wasn't anything that told me whether or not it actually found any problems. It would still be nice to work with upstream and get it enabled eventually; we want to have automated tests for as many packages as possible. So everything is fixed, and this package is APPROVED. Now you can deal with the CLA if you haven't already done so, and then request your cvsextras account. I'll sponsor you. You can also request fedorabugs access so that you can do your own reviews; I can approve you for that as well. Info for doing that is at http://fedoraproject.org/wiki/Extras/Contributors#GetAFedoraAccount Once that's all done, you can check in your code, request your branches, start your builds and close this ticket. Just email me or catch me on IRC if you have questions. (My nick is "tibbs".)
Package built successfully, so I can close this ticket