Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=923163 Mathieu Bridon <bochecha@xxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@xxxxxxxxxxxxxxxxx |bochecha@xxxxxxxxxxxxxxxxx --- Comment #5 from Mathieu Bridon <bochecha@xxxxxxxxxxxxxxxxx> --- Here's the full review, very few is missing for the package to be approved. :) Summary ======= [!]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!]: License field in the package spec file matches the actual license. => Content is under several licenses: - GPLv2+ the actual source code of gnome-weather - LGPLv2+ the bundled libgd - MIT src/package.js - BSD src/util.js - CC-BY weather-clear weather-clear-night weather-few-clouds weather-fog weather-overcast weather-showers weather-showers-scattered weather-snow weather-storm - CC-BY-SA weather-few-clouds-night => The License tag should be: License: GPLv2+ and LGPLv2+ and MIT and CC-BY and CC-BY-SA => Add the data/CREDITS file as %doc, to respect the CC-BY and CC-BY-SA licenses. => Note that src/params.js doesn't have any license header, so you could ask upstream to add one. This seems to be some kind of helper for GNOME apps written in JS, so I have no doubt that the intention of the author was to have it under a Free Software license. Not blocking the review on this. [!]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. => Please have upstream add the license file (not a blocker for the review though, it can easily come in a later update) [!]: Package contains no bundled libraries without FPC exception. => Package bundles libgd, which is fine given the nature of libgd. However: - You should make that explicit, by adding: Provides: bundled(libgd) - libgd.so should not get installed, but be linked statically, as explained by Matthias: https://bugzilla.redhat.com/show_bug.cgi?id=919265#c6 [!]: Final provides and requires are sane (see attachments). => As already mentioned, please add: Provides: bundled(libgd) => As already mentioned, don't install libgd, which would remove the automatically generated: Provides: libgd.so()(64bit) [!]: Packages should try to preserve timestamps of original installed files. => You can pass "INSTALL=/usr/bin/install -p" to the "make install" line (not a blocker for the review) [!]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 10373120 bytes in /usr/share 10373120 gnome-weather-3.7.92-1.fc20.x86_64.rpm => You **could** eventually make that a noarch subpackage. However, once you stop installing the bundled libgd, the only arch-specific thing which remains in the package is the Gd-1.0.typelib file, so having a noarch subpackage might be a bit overkill. Longterm, I believe libgd will eventually get merged into Gtk, right? So at that point, the typelib won't get installed by gnome-weather any more, and the whole package could be made noarch. With all the above in mind, I won't block the review on this, just try to remember to make the whole package noarch when that becomes possible. [!]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). Package Review ============== Key: [x] = Pass [!] = Fail [-] = Not applicable [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [-]: Development (unversioned) .so files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [!]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. => See details below. [x]: %build honors applicable compiler flags or justifies otherwise. [!]: Package contains no bundled libraries without FPC exception. => Package bundles libgd, which is fine given the nature of libgd. However: - You should make that explicit, by adding: Provides: bundled(libgd) - libgd.so should not get installed, but be linked statically, as explained by Matthias: https://bugzilla.redhat.com/show_bug.cgi?id=919265#c6 [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Development files must be in a -devel package [x]: Package requires other packages for directories it uses. [x]: Package uses nothing in %doc for runtime. [x]: Package is not known to require ExcludeArch. [x]: glib-compile-schemas is run if required [-]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [!]: License field in the package spec file matches the actual license. => Content is under several licenses: - GPLv2+ the actual source code of gnome-weather - LGPLv2+ the bundled libgd - MIT src/package.js - BSD src/util.js - CC-BY weather-clear weather-clear-night weather-few-clouds weather-fog weather-overcast weather-showers weather-showers-scattered weather-snow weather-storm - CC-BY-SA weather-few-clouds-night => The License tag should read: License: GPLv2+ and LGPLv2+ and MIT and CC-BY and CC-BY-SA => Please add the data/CREDITS file as %doc, to respect the CC-BY and CC-BY-SA licenses. => Note that src/params.js doesn't have any license header, so you could ask upstream to add one. This seems to be some kind of helper for GNOME apps written in JS, so I have no doubt that the intention of the author was to have it under a Free Software license. Not blocking the review on this. [x]: The spec file handles locales properly. [x]: Package consistently uses macro is (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [-]: update-desktop-database is invoked when required [x]: Useful -debuginfo package or justification otherwise. [-]: Large documentation must go in a -doc subpackage. Note: Documentation size is 10240 bytes in 1 files. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Each %files section contains %defattr if rpm < 4.4 [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install if there is such a file. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [-]: Fully versioned dependency in subpackages, if present. [x]: Spec file lacks Packager, Vendor, PreReq tags. [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package do not use a name that already exist [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). ===== SHOULD items ===== Generic: [!]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. => Please have upstream add the license file (not a blocker for the review though, it can easily come in a later update) [!]: Final provides and requires are sane (see attachments). => As already mentioned, please add: Provides: bundled(libgd) => As already mentioned, don't install libgd, which would remove the automatically generated: Provides: libgd.so()(64bit) [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [ ]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [!]: Packages should try to preserve timestamps of original installed files. => You can pass "INSTALL=/usr/bin/install -p" to the "make install" line [x]: Sources can be downloaded from URI in Source: tag [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: Dist tag is present. [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Uses parallel make. [x]: SourceX tarball generation or download is documented. [x]: SourceX is a working URL. [-]: Spec use %global instead of %define. ===== EXTRA items ===== Generic: [!]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 10373120 bytes in /usr/share 10373120 gnome-weather-3.7.92-1.fc20.x86_64.rpm => You **could** eventually make that a noarch subpackage. However, once you stop installing the bundled libgd, the only arch-specific thing which remains in the package is the Gd-1.0.typelib file, so having a noarch subpackage might be a bit overkill. Longterm, I believe libgd will eventually get merged into Gtk, right? So at that point, the typelib won't get installed by gnome-weather any more, and the whole package could be made noarch. With all the above in mind, I won't block the review on this, just try to remember to make the whole package noarch when that becomes possible. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: gnome-weather-3.7.92-1.fc20.x86_64.rpm gnome-weather.x86_64: W: no-manual-page-for-binary gnome-weather => Feel free to ignore this. gnome-weather.x86_64: W: one-line-command-in-%post /sbin/ldconfig => The call to ldconfig should be removed completely, as libgd.so should not be installed. Requires -------- gnome-weather (rpmlib, GLIBC filtered): /bin/sh /usr/bin/gjs libatk-1.0.so.0()(64bit) libc.so.6()(64bit) libcairo-gobject.so.2()(64bit) libcairo.so.2()(64bit) libgdk-3.so.0()(64bit) libgdk_pixbuf-2.0.so.0()(64bit) libgio-2.0.so.0()(64bit) libglib-2.0.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgtk-3.so.0()(64bit) libm.so.6()(64bit) libpango-1.0.so.0()(64bit) libpangocairo-1.0.so.0()(64bit) rtld(GNU_HASH) Provides -------- gnome-weather: gnome-weather gnome-weather(x86-64) libgd.so()(64bit) Unversioned so-files -------------------- gnome-weather: /usr/lib64/gnome-weather/libgd.so MD5-sum check ------------- http://ftp.acc.umu.se/pub/GNOME/sources/gnome-weather/3.7/gnome-weather-3.7.92.tar.xz : CHECKSUM(SHA256) this package : de1f67ef240a65cff3a787c21c0541869358f6e478ee77562d89103d3fe61347 CHECKSUM(SHA256) upstream package : de1f67ef240a65cff3a787c21c0541869358f6e478ee77562d89103d3fe61347 Generated by fedora-review 0.4.0 (660ce56) last change: 2013-01-29 Buildroot used: fedora-rawhide-x86_64 Command line :/usr/bin/fedora-review -b 923163 -m fedora-rawhide-x86_64 -- 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=Kd3zbMsBhd&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review