Spec URL: http://bochecha.fedorapeople.org/packages/gnome-music.spec SRPM URL: http://bochecha.fedorapeople.org/packages/gnome-music-3.9.90-1.fc21.src.rpm Description: Music player and management application for GNOME. Fedora Account System Username: bochecha
Building/compiling a package in Fedora is required to be verbose/non-silent. Please append --disable-silent-rules to %configure to make it verbose.
(In reply to Ralf Corsepius from comment #1) > Building/compiling a package in Fedora is required to be verbose/non-silent. I can't find anything about that in the guidelines, do you have a link?
(In reply to Mathieu Bridon from comment #2) > (In reply to Ralf Corsepius from comment #1) > > Building/compiling a package in Fedora is required to be verbose/non-silent. > > I can't find anything about that in the guidelines, do you have a link? No I don't have a link at hand, but verbosity has been a requirement in Fedora rpm-specs ever since Fedora exits, because packages without are unmaintainable and maintains not doing so are working irresponsible. How do you want to check your build.log for correctness if it doesn't contain the necessary informantion (Like your build.logs) E.g. how do you prove/check your CC calls receive the correct defines and includes? You're in error to believe a CC call not erroring out means "clean compilation".
Hi, I'll review this one. Thank you for your initial comments Ralf. Warm regards, Ankur
I see that your spec talks about "bundled libgd". Do we have any proper reason to bundle it?
(In reply to Ralf Corsepius from comment #3) > (In reply to Mathieu Bridon from comment #2) > > (In reply to Ralf Corsepius from comment #1) > > > Building/compiling a package in Fedora is required to be verbose/non-silent. > > > > I can't find anything about that in the guidelines, do you have a link? > > No I don't have a link at hand, but verbosity has been a requirement in > Fedora rpm-specs ever since Fedora exits, because packages without are > unmaintainable and maintains not doing so are working irresponsible. You know, nobody forces you to maintain packages if you find them unmaintainable... > How do you want to check your build.log for correctness if it doesn't > contain the necessary informantion (Like your build.logs) E.g. how do you > prove/check your CC calls receive the correct defines and includes? > > You're in error to believe a CC call not erroring out means "clean > compilation". What makes you think that I believe this from the one line I wrote above? I asked you for a link because I'm actually interested in learning if I missed something in the guidelines. No need to call me irresponsible, or to try and guess what people think... When you assert that something is required, please add a link to the relevant part of the guidelines, to educate your fellow packagers. I did search for it before asking, and could not find anything about it. Now, as it happens, I do disagree with you: if it's not required, then I'd rather keep silent rules. Verbose build just spews way too much stuff to be useful in the general case, I prefer enabling it when I really want it (e.g when a build fails weirdly, or before submitting this review request ;)
(In reply to Elad Alfassa from comment #5) > I see that your spec talks about "bundled libgd". Do we have any proper > reason to bundle it? libgd is meant to be used that way. It's maintained upstream in its own git module, then bundled to a few gnome-* projects as a git submodule. So the library is in fact maintained only in one place, but because it changes rapidly and is meant to be temporary (the stuff it contains slowly moves to Gtk once it matures) then it gets bundled into the source tarballs. So yes, it's entirely expected to be bundled this way, it really wouldn't make any sense to package it on its own. Note that it gets installed in a private directory so it isn't picked up automatically by the linker, and (unless I made a mistake) rpmbuild doesn't add any Provides on it.
Review: [+] OK [-] NA [?] Issue ** Mandatory review guidelines: ** [?] rpmlint output: [asinha@ankur result]$ rpmlint *.rpm ~/rpmbuild/SPECS/gnome-music.spec ~/rpmbuild/SRPMS/gnome-music-3.9.90-1.fc21.src.rpm gnome-music.src: W: invalid-url URL: http://wiki.gnome.org/Music HTTP Error 404: NOT FOUND gnome-music.src:39: W: unversioned-explicit-provides bundled(libgd) gnome-music.x86_64: W: invalid-url URL: http://wiki.gnome.org/Music HTTP Error 404: NOT FOUND gnome-music.x86_64: W: no-manual-page-for-binary gnome-music gnome-music-debuginfo.x86_64: W: invalid-url URL: http://wiki.gnome.org/Music HTTP Error 404: NOT FOUND /home/asinha/rpmbuild/SPECS/gnome-music.spec:39: W: unversioned-explicit-provides bundled(libgd) gnome-music.src: W: invalid-url URL: http://wiki.gnome.org/Music HTTP Error 404: NOT FOUND gnome-music.src:39: W: unversioned-explicit-provides bundled(libgd) 4 packages and 1 specfiles checked; 0 errors, 8 warnings. [asinha@ankur result]$ The URL needs to be updated. [+] License is acceptable (...) [?] License field in spec is correct Needs to add "with exceptions" in the GPLv2+ part [?] License files included in package %docs if included in source package Not all the licenses are included. Not a blocker though. [-] License files installed when any subpackage combination is installed [+] Spec written in American English [+] Spec is legible [ ] Sources match upstream unless altered to fix permissibility issues [asinha@ankur SPECS]$ review-md5check.sh gnome-music.spec Getting http://ftp.gnome.org/pub/GNOME/sources/gnome-music/3.9/gnome-music-3.9.90.tar.xz to /tmp/review/gnome-music-3.9.90.tar.xz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 1305k 100 1305k 0 0 41626 0 0:00:32 0:00:32 --:--:-- 27715 7aca2f5922a12dd341f50414354f984f /tmp/review/gnome-music-3.9.90.tar.xz 7aca2f5922a12dd341f50414354f984f /home/asinha/rpmbuild/SOURCES/gnome-music-3.9.90.tar.xz removed ‘/tmp/review/gnome-music-3.9.90.tar.xz’ removed directory: ‘/tmp/review’ [asinha@ankur SPECS]$ [+] Build succeeds on at least one primary arch [-] Build succeeds on all primary arches or has ExcludeArch + bugs filed [+] BuildRequires correct, justified where necessary [?] Locales handled with %find_lang, not %_datadir/locale/* find_lang has a --with-gnome flag. Worth checking if that should be used here. https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Handling_Locale_Files [-] %post, %postun call ldconfig if package contains shared .so files [?] No bundled libs Internal library, so it should be fine. The library cannot be used by all apps since they currently use their own versions temporarily. [-] Relocatability is justified [+] Package owns all directories it creates [+] Package requires others for directories it uses but does not own [+] No duplication in %files unless necessary for license files [+] File permissions are sane [+] Package contains permissible code or content [-] Large docs go in -doc subpackage [+] %doc files not required at runtime [-] Static libs go in -static package/virtual Provides [-] Development files go in -devel package [-] -devel packages Require base with fully-versioned dependency, %_isa [+] No .la files [?] GUI app uses .desktop file, installs it with desktop-file-install Desktop file included but not installed using desktop-file-install or validate https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#desktop-file-install_usage [+] File list does not conflict with other packages' without justification [+] File names are valid UTF-8 ** Optional review guidelines: ** [?] Query upstream about including license files Would be good to include the other license files also [-] Translations of description, summary [+] Builds in mock [+] Builds on all arches [-] Functions as described (e.g. no crashes) NOT TESTED [+] Scriptlets are sane [-] Subpackages require base with fully-versioned dependency if sensible [-] .pc file subpackage placement is sensible [+] No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin [-] Include man pages if available Naming guidelines: [+] Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+ [+] Package names are sane [+] No naming conflicts [+] Spec file name matches base package name [+] Version is sane [+] Version does not contain ~ [+] Release is sane [+] %dist tag [+] Case used only when necessary [+] Renaming handled correctly Packaging guidelines: [+] Useful without external bits [+] No kmods [-] Pre-built binaries, libs removed in %prep [+] Sources contain only redistributable code or content [+] Spec format is sane [+] Package obeys FHS, except libexecdir, /run, /usr/target [+] No files in /bin, /sbin, /lib* on >= F17 [-] Programs run before FS mounting use /run instead of /var/run [-] Binaries in /bin, /sbin do not depend on files in /usr on < F17 [-] No files under /srv, /opt, /usr/local [+] Changelog in prescribed format [+] No Packager, Vendor, Copyright, PreReq tags [+] Summary does not end in a period [-] Correct BuildRoot tag on < EL6 [-] Correct %clean section on < EL6 Do you plan to package this for EL? [+] Requires correct, justified where necessary Comments with the Requires would be good to have. [+] Summary, description do not use trademarks incorrectly [+] All relevant documentation is packaged, appropriately marked with %doc [+] Doc files do not drag in extra dependencies (e.g. due to +x) [+] Code compilable with gcc is compiled with gcc [+] Build honors applicable compiler flags or justifies otherwise Looks like, but a non verbose build would be more helpful [-] PIE used for long-running/root daemons, setuid/filecap programs [+] Useful -debuginfo package or disabled and justified [-] Package with .pc files Requires pkgconfig on < EL6 [+] No static executables [+] Rpath absent or only used for internal libs [-] Config files marked with %config(noreplace) or justified %config [-] No config files under /usr [-] Third party package manager configs acceptable, in %_docdir [?] .desktop files are sane Needs to be checked. [+] Spec uses macros consistently [+] Spec uses macros instead of hard-coded names where appropriate [+] Spec uses macros for executables only when configurability is needed [-] %makeinstall used only when alternatives don't work [-] Macros in Summary, description are expandable at srpm build time [+] Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir [+] No software collections (scl) [-] Macro files named /etc/rpm/macros.%name [+] Build uses only python/perl/shell+coreutils/lua/BuildRequired langs [-] %global, not %define [?] Package translating with gettext BuildRequires it Is this required? [-] Package translating with Linguist BuildRequires qt-devel [+] File ops preserve timestamps [+] Parallel make [+] No Requires(pre,post) notation [-] User, group creation handled correctly (See Packaging:UsersAndGroups) [-] Web apps go in /usr/share/%name, not /var/www [-] Conflicts are justified [+] One project per package [+] No bundled fonts [-] Patches have appropriate commentary [-] Available test suites executed in %check [-] tmpfiles.d used for /run, /run/lock on >= F15 ** Python guidelines: ** [+] Runtime Requires correct Look OK [-] Python macros declared on < EL6 [+] All .py files packaged with .pyc, .pyo counterparts [+] Includes .egg-info files/directories when generated [+] Provides/Requires properly filtered [-] Code that invokes gtk.gdk.get_pixels_array() Requires numpy A few issues. Some are blockers, but they aren't too complicated to fix.
* a verbose build would be more helpful.
Thanks for the feedback Ankur. Here is a new package, fixing the issues you mentioned. Note that I didn't add the --with-gnome option to the %find_lang macro because the package doesn't contain GNOME help files, and as a result the build fails with that option. Also, due to popular request, I made the build verbose. One thing I hadn't thought about but you mentioned on IRC is that it makes the reviewer's job much easier when it comes to ensuring that the build flags are respected. Spec URL: http://bochecha.fedorapeople.org/packages/gnome-music.spec SRPM URL: http://bochecha.fedorapeople.org/packages/gnome-music-3.9.90-2.fc21.src.rpm
(In reply to Mathieu Bridon from comment #10) > Thanks for the feedback Ankur. > > Here is a new package, fixing the issues you mentioned. > > Note that I didn't add the --with-gnome option to the %find_lang macro > because the package doesn't contain GNOME help files, and as a result the > build fails with that option. I wasn't sure of this either. If it doesn't provide GNOME help files, the --with-gnome flag will fail. No worries. > > Also, due to popular request, I made the build verbose. One thing I hadn't > thought about but you mentioned on IRC is that it makes the reviewer's job > much easier when it comes to ensuring that the build flags are respected. Thanks. The flags are indeed being respected. > > Spec URL: http://bochecha.fedorapeople.org/packages/gnome-music.spec > SRPM URL: > http://bochecha.fedorapeople.org/packages/gnome-music-3.9.90-2.fc21.src.rpm Looks good to me. New rpmlint output: [asinha@ankur SRPMS]$ rpmlint ./gnome-music-3.9.90-2.fc21.src.rpm ../SPECS/gnome-music.spec /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm gnome-music.src:40: W: unversioned-explicit-provides bundled(libgd) ../SPECS/gnome-music.spec:40: W: unversioned-explicit-provides bundled(libgd) gnome-music.src:40: W: unversioned-explicit-provides bundled(libgd) gnome-music.x86_64: W: no-manual-page-for-binary gnome-music 4 packages and 1 specfiles checked; 0 errors, 4 warnings. XXXXX APPROVED XXXX Thanks, Warm regards, Ankur
Thanks Ankur! New Package SCM Request ======================= Package Name: gnome-music Short Description: Music player and management application for GNOME Owners: bochecha Branches: f20 devel
Git done (by process-git-requests).
Thanks for the Git process Jon! Package built in Rawhide and F20, closing.