Spec URL: https://gitorious.org/valastuff/presence/blobs/spec/presence.spec SRPM URL: http://fabiand.fedorapeople.org/presence/presence-0.4.1-1.fc16.src.rpm Description: A simple tool to establish a bi-directional audio/video connection in simple networks (aka LAN). $ rpmlint presence.spec presence.spec:9: W: macro-in-comment %{version} presence.spec:10: W: macro-in-comment %{version} presence.spec:10: W: macro-in-comment %{version} presence.spec:11: W: macro-in-comment %{version} presence.spec:11: W: macro-in-comment %{version} presence.spec: W: invalid-url Source0: presence-0.4.1.tar.xz 0 packages and 1 specfiles checked; 0 errors, 6 warnings. $ rpmlint ~/rpmbuild/SRPMS/presence-0.4.1-1.fc16.src.rpm presence.src:9: W: macro-in-comment %{version} presence.src:10: W: macro-in-comment %{version} presence.src:10: W: macro-in-comment %{version} presence.src:11: W: macro-in-comment %{version} presence.src:11: W: macro-in-comment %{version} presence.src: W: invalid-url Source0: presence-0.4.1.tar.xz 1 packages and 0 specfiles checked; 0 errors, 6 warnings. $ rpmlint ~/rpmbuild/SRPMS/presence-0.4.1-1.fc16.src.rpm ~/rpmbuild/RPMS/x86_64/presence-0.4.1-1.fc16.x86_64.rpm ~/rpmbuild/RPMS/x86_64/presence-debuginfo-0.4.1-1.fc16.x86_64.rpm presence.src:9: W: macro-in-comment %{version} presence.src:10: W: macro-in-comment %{version} presence.src:10: W: macro-in-comment %{version} presence.src:11: W: macro-in-comment %{version} presence.src:11: W: macro-in-comment %{version} presence.src: W: invalid-url Source0: presence-0.4.1.tar.xz presence.x86_64: W: no-manual-page-for-binary presence 3 packages and 0 specfiles checked; 0 errors, 7 warnings.
More about the tool at http://dummdida.blogspot.com/p/presence.html
If you want to create tarball from git it's better to use "git archive" to do it. Your "tar -cJvf presence-%{version}.tar.xz presence-%{version}" packs .git directory as well. Another thing: Upstream version is 0.4 not 0.4.1. Version numbers in Fedora should match versions from upstream tarball. I know you are upstream, but it's still better to be consistent. License is also incorrect, COPYING file is LGPL 2.1, yet spec states GPLv2+.
Thanks for looking at it. (In reply to comment #2) > If you want to create tarball from git it's better to use "git archive" to do > it. Your "tar -cJvf presence-%{version}.tar.xz presence-%{version}" packs .git > directory as well. True. I wonder why I used tar in the first place. > Another thing: Upstream version is 0.4 not 0.4.1. Version numbers in Fedora > should match versions from upstream tarball. I know you are upstream, but it's > still better to be consistent. I actually pushed the tag for presence-0.4.1, this should match upstream. Or am I missing something? > License is also incorrect, COPYING file is LGPL 2.1, yet spec states GPLv2+. Ouch, will be fixed.
(In reply to comment #3) > > Another thing: Upstream version is 0.4 not 0.4.1. Version numbers in Fedora > > should match versions from upstream tarball. I know you are upstream, but it's > > still better to be consistent. > > I actually pushed the tag for presence-0.4.1, this should match upstream. Or am > I missing something? Well, configure.ac in tarball states it's version 0.4 so I assumed that. But you're right that it's created from 0.4.1 tag so I guess it's more of a configure.ac problem.
(In reply to comment #4) > (In reply to comment #3) > > > Another thing: Upstream version is 0.4 not 0.4.1. Version numbers in Fedora > > > should match versions from upstream tarball. I know you are upstream, but it's > > > still better to be consistent. > > > > I actually pushed the tag for presence-0.4.1, this should match upstream. Or am > > I missing something? > > Well, configure.ac in tarball states it's version 0.4 so I assumed that. But > you're right that it's created from 0.4.1 tag so I guess it's more of a > configure.ac problem. Ah yes, I need to get used to update all those relevant places ... One question remains for me: How do I "mark" the software as GPLv2+ (and not just GPLv2)? I've updated the spec at https://gitorious.org/valastuff/presence/blobs/spec/presence.spec The new srpm is here http://fabiand.fedorapeople.org/presence/presence-0.4.2-1.fc15.src.rpm and the corresponding koji task is http://koji.fedoraproject.org/koji/taskinfo?taskID=3522354
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > > Another thing: Upstream version is 0.4 not 0.4.1. Version numbers in Fedora > > > > should match versions from upstream tarball. I know you are upstream, but it's > > > > still better to be consistent. > > > > > > I actually pushed the tag for presence-0.4.1, this should match upstream. Or am > > > I missing something? > > > > Well, configure.ac in tarball states it's version 0.4 so I assumed that. But > > you're right that it's created from 0.4.1 tag so I guess it's more of a > > configure.ac problem. > > Ah yes, I need to get used to update all those relevant places ... > One question remains for me: How do I "mark" the software as GPLv2+ (and not > just GPLv2)? This is usually done by adding "or (at your option) any later version" to licensing headers of source files. Look for "How to Apply These Terms to Your New Programs" in your COPYING file.
I've added the appropriate headers to all source files and a small script to generate to archive. specfile: https://gitorious.org/valastuff/presence/blobs/spec/presence.spec srpm: http://fabiand.fedorapeople.org/presence/presence-0.4.4-1.fc16.src.rpm koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3523291
OK, since I've already had a look, I might just as well do the official review :-)
That's "hammer "(de_DE, variant of awesome) :) Thanks for the guidance and patience up to now.
Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated ==== C/C++ ==== [x]: MUST Header files in -devel subpackage, if present. [x]: MUST Package does not contain any libtool archives (.la) [x]: MUST Package contains no static executables. [x]: MUST Rpath absent or only used for internal libs. [x]: MUST Package is not relocatable. ==== Generic ==== [x]: MUST Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: MUST Package successfully compiles and builds into binary rpms on at least one supported architecture. [x]: MUST All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: MUST Package contains no bundled libraries. [x]: MUST Changelog in prescribed format. [x]: MUST Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).(EPEL6 & Fedora < 13) [x]: MUST Sources contain only permissible code or content. [x]: MUST Each %files section contains %defattr if rpm < 4.4 [x]: MUST Macros in Summary, %description expandable at SRPM build time. [x]: MUST Package contains a properly installed %{name}.desktop using desktop-file-install file if it is a GUI application. [x]: MUST Package requires other packages for directories it uses. [x]: MUST Package uses nothing in %doc for runtime. [x]: MUST Package is not known to require ExcludeArch. [x]: MUST Permissions on files are set properly. [x]: MUST Package does not contain duplicates in %files. [x]: MUST Spec file lacks Packager, Vendor, PreReq tags. [!]: MUST Package run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) and the beginning of %install. (EPEL5) [x]: MUST 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. [x]: MUST License field in the package spec file matches the actual license. [x]: MUST Package consistently uses macros (instead of hard-coded directory names). [x]: MUST Package meets the Packaging Guidelines. [x]: MUST Package is named according to the Package Naming Guidelines. [x]: MUST Package does not generates any conflict. [x]: MUST Package obeys FHS, except libexecdir and /usr/target. [!]: MUST Package must own all directories that it creates. /usr/share/presence is unowned [x]: MUST Package does not own files or directories owned by other packages. [x]: MUST Package installs properly. [x]: MUST Requires correct, justified where necessary. [!]: MUST Rpmlint output is silent. rpmlint presence-0.4.4-1.fc17.src.rpm ================================================================================ presence.src:9: W: macro-in-comment %{version} presence.src:10: W: macro-in-comment %{version} presence.src:10: W: macro-in-comment %{version} presence.src: W: invalid-url Source0: presence-0.4.4.tar.xz 1 packages and 0 specfiles checked; 0 errors, 4 warnings. ================================================================================ rpmlint presence-0.4.4-1.fc17.i686.rpm ================================================================================ presence.i686: W: no-manual-page-for-binary presence 1 packages and 0 specfiles checked; 0 errors, 1 warnings. ================================================================================ rpmlint presence-debuginfo-0.4.4-1.fc17.i686.rpm ================================================================================ 1 packages and 0 specfiles checked; 0 errors, 0 warnings. ================================================================================ Well now that you generate tarball with a script, it would be best to get rid of those macros in comments [!]: MUST Sources used to build the package matches the upstream source, as provided in the spec URL. presence-0.4.4.tar.xz : MD5SUM this package : 7d3b9be502690752ee443e4434b657ee MD5SUM upstream package : upstream source not found verified they match when regenerated [x]: MUST Spec file is legible and written in American English. [x]: MUST Spec file name must match the spec package %{name}, in the format %{name}.spec. [-]: MUST Package contains a SysV-style init script if in need of one. [x]: MUST File names are valid UTF-8. [x]: SHOULD Reviewer should test that the package builds in mock. [x]: SHOULD If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: SHOULD Dist tag is present. [x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q --requires). [!]: SHOULD Package functions as described. While I don't have a webcam in my pc, I'd assume the application shows icons in bottom screen area. Those buttons work, but they have no icons so it's impossible to navigate [x]: SHOULD Package does not include license text files separate from upstream. [x]: SHOULD SourceX is a working URL. [-]: SHOULD Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: SHOULD Package should compile and build into binary rpms on all supported architectures. [-]: SHOULD %check is present and all tests pass. [x]: SHOULD Packages should try to preserve timestamps of original installed files. [x]: SHOULD Spec use %global instead of %define. Issues: [!]: MUST Package run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) and the beginning of %install. (EPEL5) - just in case you want EPEL5 branch, then you'd need defattr back as well and buildroot def. I guess not... [!]: MUST Package must own all directories that it creates. /usr/share/presence is unowned [!]: SHOULD Package functions as described. While I don't have a webcam in my pc, I'd assume the application shows icons in bottom screen area. Those buttons work, but they have no icons so it's impossible to navigate. Perhaps some missing requires?
(In reply to comment #10) > Issues: > [!]: MUST Package run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) and the > beginning of %install. (EPEL5) Fixed. > - just in case you want EPEL5 branch, then you'd need defattr back as well and > buildroot def. I guess not... I am not yet interested in getting this into EPEL :) > [!]: MUST Package must own all directories that it creates. > /usr/share/presence is unowned I suppose this was because of globbing, I added the dir explicitly. > [!]: SHOULD Package functions as described. > > While I don't have a webcam in my pc, I'd assume the application shows icons in > bottom screen area. Those buttons work, but they have no icons so it's > impossible to navigate. Perhaps some missing requires? Yes, I suppose you ain't running GNOME? :) I added a dependency on gnome-icon-theme and -symbolic this should solve this. And you'll need to be creative to test the software on a machine without a webcam :) specfile: https://gitorious.org/valastuff/presence/blobs/spec/presence.spec srpm: http://fabiand.fedorapeople.org/presence/presence-0.4.4-2.fc16.src.rpm koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3523562
(In reply to comment #11) > (In reply to comment #10) > > > Issues: > > [!]: MUST Package run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) and the > > beginning of %install. (EPEL5) > > Fixed. Well this was really needed only for EPEL5, but it doesn't hurt :-) > I suppose this was because of globbing, I added the dir explicitly. OK now. Though the "%{_datadir}/presence/*" entry is now redundant :-) > > [!]: SHOULD Package functions as described. > > > > While I don't have a webcam in my pc, I'd assume the application shows icons in > > bottom screen area. Those buttons work, but they have no icons so it's > > impossible to navigate. Perhaps some missing requires? > > Yes, I suppose you ain't running GNOME? :) I added a dependency on > gnome-icon-theme and -symbolic this should solve this. > And you'll need to be creative to test the software on a machine without a > webcam :) Well I can't seem to make it work in Xnest (segfaults), Xephyr (works, but it has the same problem with icons) even though I already had those icons installed. You are right I am not running Gnome, so my environment is a little non-standard. It shouldn't matter though. I should be able to run any application under any WM/DE. There was another weird thing when I tried to resize the window. It jumped to maximum width (as in maxiumu allowed by my graphics gard - wider than my monitor). Normally I'd probably tell the packager to sort it out with upstream first, but since you are the upstream I think you are in a perfect position to fix this later on...That said: APPROVED
Unbelievable :) Thanks for this quick review, Stanislav! (In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > > > Issues: > > > [!]: MUST Package run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) and the > > > beginning of %install. (EPEL5) > > > > Fixed. > > Well this was really needed only for EPEL5, but it doesn't hurt :-) I'll take this into account and will have a look at other packages what is needed. > > I suppose this was because of globbing, I added the dir explicitly. > > OK now. Though the "%{_datadir}/presence/*" entry is now redundant :-) Hum, I thought that this just pulls the path, but not it's children. Will be removed though. > > > [!]: SHOULD Package functions as described. > > > > > > While I don't have a webcam in my pc, I'd assume the application shows icons in > > > bottom screen area. Those buttons work, but they have no icons so it's > > > impossible to navigate. Perhaps some missing requires? > > > > Yes, I suppose you ain't running GNOME? :) I added a dependency on > > gnome-icon-theme and -symbolic this should solve this. > > And you'll need to be creative to test the software on a machine without a > > webcam :) > > Well I can't seem to make it work in Xnest (segfaults), Xephyr (works, but it > has the same problem with icons) even though I already had those icons > installed. You are right I am not running Gnome, so my environment is a little > non-standard. It shouldn't matter though. I should be able to run any > application under any WM/DE. Crashes in Xnest as expected as it requires GL, which ain't supported by Xnest (afaik). I am also seeing the icon problems on Fedora 15, but they are not present on Fedora 16. Are you running F15? > There was another weird thing when I tried to resize the window. It jumped to > maximum width (as in maxiumu allowed by my graphics gard - wider than my > monitor). This is weird. I actually don't do much with the window size, just restricting the aspect ratio and switching to fullscreen on demand. Maybe a result of the WM and aspect-ratio-based-resizing. > Normally I'd probably tell the packager to sort it out with upstream first, but > since you are the upstream I think you are in a perfect position to fix this > later on...That said: > > APPROVED What a friday :) Thanks, fabian.
New Package SCM Request ======================= Package Name: presence Short Description: Bi-directional audio/video connections Owners: fabiand Branches: f16 InitialCC:
Git done (by process-git-requests).
presence-0.4.4-2.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/presence-0.4.4-2.fc16
presence-0.4.4-2.fc16 has been pushed to the Fedora 16 testing repository.
presence-0.4.4-2.fc16 has been pushed to the Fedora 16 stable repository.