Spec URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather.spec SRPM URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather-3.7.92-1.fc19.src.rpm Description: gnome-weather is a weather application for GNOME. Like e.g. Documents Photos and Clocks, Weather is supposed to be a core GNOME utility integrated with the rest of the desktop. Fedora Account System Username: cosimoc
I just tried reviewing this package, and it doesn't build: + ./configure --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --disable-static /var/tmp/rpm-tmp.4dZ8zy: line 34: ./configure: No such file or directory The problem is your invocation of the %setup macro: %setup -q -c gnome-weather -n gnome-weather This means that rpmbuild will first create a "gnome-weather" folder, then extract the tarball inside. So you end up with the path: gnome-weather/gnome-weather-3.7.92/ The contents of the archive (including the configure file) are in that last subfolder. But rpmbuild will only enter the gnome-weather folder, and as such not find the contents. You could just use the simplest form of the setup macro: %setup -q And that fixes the issue.
Yeah, that's a leftover from a previous version of the spec where I was building from a git snapshot, hence the more complicated setup. I'll update shortly.
Updated: Spec URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather.spec SRPM URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather-3.7.92-1.fc19.src.rpm Unfortunately, this still doesn't build, as it requires a non-released version of gobject-introspection. I'll ping Colin and try to get one soon.
gobject-introspection 1.35.9 is now available in F19/Rawhide and I successfully made a scratch build [1] of gnome-weather 3.7.92 using the spec from the previous comment. [1] http://koji.fedoraproject.org/koji/taskinfo?taskID=5154118
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
Sorry, I hit the submit button a bit too quickly. This passes: [x]: Package should compile and build into binary rpms on all supported architectures.
(In reply to comment #5) > Here's the full review, very few is missing for the package to be approved. > :) Thanks Mathieu, I am a bit unclear on some points. > => The License tag should be: > License: GPLv2+ and LGPLv2+ and MIT and CC-BY and CC-BY-SA I always assumed the License: tag was about the overall license of the shipped binary, and not the individual licenses of source files inside the package. For example, GPLv2+, LGLPv2+ and MIT can be used together in a GPLv2-licensed binary (not sure the same applies to CC licenses though). I changed the spec file to your suggestion anyway. > [!]: 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 I did the former; the latter is not possible for this package, since there's no C binary to statically link the library to. > => 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. Yeah; right now I don't think it's useful to split any noarch part out of the package. Updated spec/SRPM for review at the same location. Spec URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather.spec SRPM URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather-3.7.92-1.fc19.src.rpm
(In reply to comment #7) > (In reply to comment #5) > > Here's the full review, very few is missing for the package to be approved. > > :) > > Thanks Mathieu, I am a bit unclear on some points. > > > => The License tag should be: > > License: GPLv2+ and LGPLv2+ and MIT and CC-BY and CC-BY-SA > > I always assumed the License: tag was about the overall license of the > shipped binary, and not the individual licenses of source files inside the > package. > For example, GPLv2+, LGLPv2+ and MIT can be used together in a > GPLv2-licensed binary (not sure the same applies to CC licenses though). > I changed the spec file to your suggestion anyway. Yes, linking for example GPL and MIT together might make the result GPL, but I can't say that for sure to be honest. Detailing it all explicitly doesn't hurt, leads to the same effective result, and can actually help finding conflicts of licenses in some cases, so that's why I prefer always being explicit on licenses. (Note that in the value I suggested, I only counted the source files which actually end up being built in the binary RPM, the license of things like the autotools boilerplate normally doesn't count) > > [!]: 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 > > I did the former; the latter is not possible for this package, since there's > no C binary to statically link the library to. Oh, I thought the typelib could have been statically linked (granted, I don't really know how the typelib works) In this case, you should at least filter the provides on libgd.so out. > > => 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. > > Yeah; right now I don't think it's useful to split any noarch part out of > the package. Works for me. > Updated spec/SRPM for review at the same location. > Spec URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather.spec > SRPM URL: > http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather-3.7.92-1.fc19. > src.rpm --- gnome-weather.spec.bak 2013-03-22 09:45:04.516932207 +0800 +++ gnome-weather.spec 2013-03-26 04:25:49.000000000 +0800 @@ -3,7 +3,7 @@ Release: 1%{?dist} Summary: A weather application for GNOME -License: GPLv2+ +License: GPLv2+ and LGPLv2+ and MIT and CC-BY and CC-BY-SA => Fixes my concern with the license tag. URL: https://live.gnome.org/Design/Apps/Weather Source0: http://ftp.acc.umu.se/pub/GNOME/sources/%{name}/3.7/%{name}-%{version}.tar.xz @@ -15,6 +15,11 @@ BuildRequires: gtk3-devel BuildRequires: gobject-introspection >= 1.35.9 +# libgd is not meant to be installed as a system-wide shared library. +# It is just a way for GNOME applications to share widgets and other common +# code on an ad-hoc basis. +Provides: bundled(libgd) => Good. %description gnome-weather is a weather application for GNOME @@ -31,11 +36,7 @@ desktop-file-validate $RPM_BUILD_ROOT/%{_datadir}/applications/%{name}.desktop %find_lang %{name} -%post -/sbin/ldconfig - => Good. %postun -/sbin/ldconfig => Good. if [ $1 -eq 0 ] ; then /usr/bin/glib-compile-schemas %{_datadir}/glib-2.0/schemas &> /dev/null || : fi @@ -44,7 +45,7 @@ /usr/bin/glib-compile-schemas %{_datadir}/glib-2.0/schemas &> /dev/null || : %files -f %{name}.lang -%doc NEWS +%doc NEWS data/CREDITS => Fixes my concern with the CC licenses %{_datadir}/%{name} %{_bindir}/%{name} %{_datadir}/glib-2.0/schemas/* ----- So, right now the only remaining problem that I can see is the provides on libgd.so: libgd.so()(64bit) As it is, if another package requires libgd.so (either because it was erroneously dynamically linked, or because a requires had not been properly filtered out), then gnome-weather would satisfy as a dependency of that package as far as RPM is concerned, which would be wrong. Adding the following two lines before %description does the trick: %filter_provides_in %{_libdir}/%{name}/.*\.so$ %filter_setup
(In reply to comment #8) > So, right now the only remaining problem that I can see is the provides on > libgd.so: > libgd.so()(64bit) > > As it is, if another package requires libgd.so (either because it was > erroneously dynamically linked, or because a requires had not been properly > filtered out), then gnome-weather would satisfy as a dependency of that > package as far as RPM is concerned, which would be wrong. > > Adding the following two lines before %description does the trick: > %filter_provides_in %{_libdir}/%{name}/.*\.so$ > %filter_setup Thanks, makes sense; I updated the spec and SRPM to include this. Updated files at the same location. Spec URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather.spec SRPM URL: http://people.gnome.org/~cosimoc/gnome-weather/gnome-weather-3.7.92-1.fc19.src.rpm
--- gnome-weather.spec.bak 2013-03-27 10:37:35.135489201 +0800 +++ gnome-weather.spec 2013-03-26 22:40:29.000000000 +0800 @@ -20,6 +20,11 @@ # code on an ad-hoc basis. Provides: bundled(libgd) +# Filter out the libgd.so() provide, since we don't want to possibly +# satisfy any erroneous dependency on it. +%filter_provides_in %{_libdir}/%{name}/.*\.so$ +%filter_setup + %description gnome-weather is a weather application for GNOME That fixes my only issue with the package, so I'm approving it. Just a note for the future: you should bump the Release tag of your spec file every time you change it, even for the review. It makes it easier for the reviewer to check changes. (if I hadn't paid attention, I could have overwritten your previous srpm on my hard drive by downloading the new one, therefore not being able to compare)
New Package SCM Request ======================= Package Name: gnome-weather Short Description: A weather application for GNOME Owners: cosimoc Branches: f19
(In reply to comment #10) > Just a note for the future: you should bump the Release tag of your spec > file every time you change it, even for the review. It makes it easier for > the reviewer to check changes. (if I hadn't paid attention, I could have > overwritten your previous srpm on my hard drive by downloading the new one, > therefore not being able to compare) Noted! Thanks for the review!
Git done (by process-git-requests).
Thanks, closing this now.