Spec URL: http://people.fedoraproject.org/~jwrdegoede/spice-vdagent.spec SRPM URL: http://people.fedoraproject.org/~jwrdegoede/spice-vdagent-0.6.3-1.fc14.src.rpm Description: Spice agent for Linux guests offering the following features: Features: * Client mouse mode (no need to grab mouse by client, no mouse lag) this is handled by the daemon by feeding mouse events into the kernel via uinput. This will only work if the active X-session is running a spice-vdagent process so that its resolution can be determined. * Automatic adjustment of the X-session resolution to the client resolution * Support of copy and paste (text and images) between the active X-session and the client
Hi Hans: While trying to compile I got the following error: + make 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic' cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -I/usr/include/spice-1 -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -D_GNU_SOURCE -c -o vdagentd.o vdagentd.c vdagentd.c: In function 'send_capabilities': vdagentd.c:87:21: error: 'VD_AGENT_CAP_CLIPBOARD_BY_DEMAND' undeclared (first use in this function) vdagentd.c:87:21: note: each undeclared identifier is reported only once for each function it appears in vdagentd.c: In function 'do_client_clipboard': vdagentd.c:167:10: error: 'VD_AGENT_CLIPBOARD_GRAB' undeclared (first use in this function) vdagentd.c:173:10: error: 'VD_AGENT_CLIPBOARD_REQUEST' undeclared (first use in this function) vdagentd.c:174:9: error: 'VDAgentClipboardRequest' undeclared (first use in this function) vdagentd.c:174:34: error: 'req' undeclared (first use in this function) vdagentd.c:174:66: error: expected expression before ')' token vdagentd.c:187:10: error: 'VD_AGENT_CLIPBOARD_RELEASE' undeclared (first use in this function) vdagentd.c: In function 'virtio_port_read_complete': vdagentd.c:241:10: error: 'VD_AGENT_CLIPBOARD_GRAB' undeclared (first use in this function) vdagentd.c:242:10: error: 'VD_AGENT_CLIPBOARD_REQUEST' undeclared (first use in this function) vdagentd.c:244:10: error: 'VD_AGENT_CLIPBOARD_RELEASE' undeclared (first use in this function) vdagentd.c:247:31: error: 'VDAgentClipboardGrab' undeclared (first use in this function) vdagentd.c:249:31: error: 'VDAgentClipboardRequest' undeclared (first use in this function) vdagentd.c: In function 'do_agent_clipboard': vdagentd.c:275:12: error: 'VD_AGENT_CAP_CLIPBOARD_BY_DEMAND' undeclared (first use in this function) vdagentd.c:295:35: error: 'VD_AGENT_CLIPBOARD_GRAB' undeclared (first use in this function) vdagentd.c:300:9: error: 'VDAgentClipboardRequest' undeclared (first use in this function) vdagentd.c:300:33: error: expected ';' before 'req' vdagentd.c:302:35: error: 'VD_AGENT_CLIPBOARD_REQUEST' undeclared (first use in this function) vdagentd.c:303:47: error: 'req' undeclared (first use in this function) vdagentd.c:327:35: error: 'VD_AGENT_CLIPBOARD_RELEASE' undeclared (first use in this function) vdagentd.c:338:21: error: 'VD_AGENT_CLIPBOARD_NONE' undeclared (first use in this function) vdagentd.c: In function 'update_active_session_connection': vdagentd.c:427:35: error: 'VD_AGENT_CLIPBOARD_RELEASE' undeclared (first use in this function) vdagentd.c: In function 'main': vdagentd.c:684:35: error: 'VD_AGENT_CLIPBOARD_RELEASE' undeclared (first use in this function) make: *** [vdagentd.o] Error 1 Please give me some hints so I can successfully build the package and start the review :) Regards, Germán.
Hi, Oops I forgot about that. You need the spice-protocol from updates-testing (which moved to regular updates earlier today, so should show up their soon). It might be easiest to get it directly from koji: http://koji.fedoraproject.org/koji/buildinfo?buildID=200958 Regards, Hans
Hi Hans, here follows the review: Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. Tested on: x86_64 [!] Rpmlint output: spice-vdagent.src: W: spelling-error %description -l en_US uinput -> input, u input, sinciput spice-vdagent.x86_64: W: spelling-error %description -l en_US uinput -> input, u input, sinciput spice-vdagent.x86_64: W: non-conffile-in-etc /etc/xdg/autostart/spice-vdagent.desktop spice-vdagent.x86_64: W: no-manual-page-for-binary spice-vdagentd spice-vdagent.x86_64: W: no-manual-page-for-binary spice-vdagent spice-vdagent.x86_64: W: service-default-enabled /etc/rc.d/init.d/spice-vdagentd spice-vdagent.x86_64: E: malformed-line-in-lsb-comment-block # daemon enhances the spice guest user experience with client spice-vdagent.x86_64: E: malformed-line-in-lsb-comment-block # mouse mode, guest <-> client copy and paste support and more. spice-vdagent.x86_64: W: incoherent-subsys /etc/rc.d/init.d/spice-vdagentd $prog spice-vdagent.x86_64: W: service-default-enabled /etc/rc.d/init.d/spice-vdagentd [x] Package is not relocatable. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)) [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x] License field in the package spec file matches the actual license. License type: GPLv3+ [x] 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] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. MD5SUM this package : dca976a6a92744462e1aed101f4ea467 MD5SUM upstream package: dca976a6a92744462e1aed101f4ea467 [x] Package is not known to require ExcludeArch, OR: Arches excluded: Why: [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [x] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [-] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [x] Package consistently uses macros. [-] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. [-] Package does not own files or directories owned by other packages. === SUGGESTED ITEMS === [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. [!] Reviewer should test that the package builds in mock. Tested on: x86_64 [?] Package should compile and build into binary rpms on all supported architectures. Tested on: [?] Package functions as described. [x] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files are correct. [-] File based requires are sane. === Issues === 1. Most important for me in this review is the rpmlint output, as it has some errors and warnings. The first 4 warnings are acceptable for me. However, as you know this is my first review, and I've never seen these errors/warnings before. If you didn't see them before, please try to correct them. If they are acceptable, please explain why. 2. This is a should, but I was not able to successfully compile the package in mock because of the updates-testing requirement for one of the BRs. Package is nice, but please clarify the marked issues. Regards, Germán.
Hi, Thanks for the review! > spice-vdagent.src: W: spelling-error %description -l en_US uinput -> > input, u input, sinciput > spice-vdagent.x86_64: W: spelling-error %description -l en_US uinput -> > input, u input, sinciput <sigh>, rpmlint is stupid wrt spelling errors, uinput is the correct term these can safely be ignored. > spice-vdagent.x86_64: W: non-conffile-in-etc > /etc/xdg/autostart/spice-vdagent.desktop And another favorite useless rpmlint warnings some files under /etc are not meant for user modification and thus not marked %config, files under /etc/xdg is one of the groups of files which should not be %config even though being under /etc. > spice-vdagent.x86_64: W: no-manual-page-for-binary spice-vdagentd > spice-vdagent.x86_64: W: no-manual-page-for-binary spice-vdagent Not really an issue as they are not meant to be directly started from the cmdline, let alone started with options on the cmdline. They are automatically started as service, resp. by .desktop files -> no need for manpage. > spice-vdagent.x86_64: W: service-default-enabled > /etc/rc.d/init.d/spice-vdagentd Yes, and intentionally so. The initscript is a no-op (exits immediately) when not running under spice. > spice-vdagent.x86_64: E: malformed-line-in-lsb-comment-block # daemon > enhances the spice guest user experience with client > spice-vdagent.x86_64: E: malformed-line-in-lsb-comment-block # mouse mode, > guest <-> client copy and paste support and more. Oh rpmlint found a real issue. I'll upload a fixed package. > spice-vdagent.x86_64: W: incoherent-subsys /etc/rc.d/init.d/spice-vdagentd <sigh> and another useless rpmlint check > === Issues === > 1. Most important for me in this review is the rpmlint output, as it has some > errors and warnings. The first 4 warnings are acceptable for me. However, as > you know this is my first review, and I've never seen these errors/warnings > before. If you didn't see them before, please try to correct them. If they are > acceptable, please explain why. See above, please don't take it personal if I sound a bit grumpy above. Although rpmlint is a very useful tool (as shown by the one real issue did find) sometimes it makes me <sigh> a lot because some of its checks are useless (like the terrible newly added spelling checking failure). But still it did find one real issue I'll upload a new version with this fixed soon. > 2. This is a should, but I was not able to successfully compile the package in > mock because of the updates-testing requirement for one of the BRs. The needed spice-protocol package is in stable updates now :)
Here is a new version with the lsb header issue fixed: Spec URL: http://people.fedoraproject.org/~jwrdegoede/spice-vdagent.spec SRPM URL: http://people.fedoraproject.org/~jwrdegoede/spice-vdagent-0.6.3-2.fc14.src.rpm Once more thanks for the review!
Hi Hans! Thanks very much for your explanations about rpmlint output. As I said above, there were some warnings/errors I didn't see before so I needed to see your comments about them. Now it is all clear for me...and you didn't sound grumpy, so don't worry about that :) Some items that remains to check now that you, as upstream, released a modified version. Some things like checksums didn't change because you only patched the original code. === REQUIRED ITEMS === [x] Package successfully compiles and builds into binary rpms on at least one supported architec ture. Tested on: x86_64 [x] Rpmlint output: spice-vdagent.src: W: spelling-error %description -l en_US uinput -> input, u input, sinciput spice-vdagent.x86_64: W: spelling-error %description -l en_US uinput -> input, u input, sinciput spice-vdagent.x86_64: W: non-conffile-in-etc /etc/xdg/autostart/spice-vdagent.desktop spice-vdagent.x86_64: W: no-manual-page-for-binary spice-vdagentd spice-vdagent.x86_64: W: no-manual-page-for-binary spice-vdagent spice-vdagent.x86_64: W: service-default-enabled /etc/rc.d/init.d/spice-vdagentd spice-vdagent.x86_64: W: incoherent-subsys /etc/rc.d/init.d/spice-vdagentd $prog spice-vdagent.x86_64: W: service-default-enabled /etc/rc.d/init.d/spice-vdagentd === SUGGESTED ITEMS === [x] Reviewer should test that the package builds in mock. Tested on: x86_64 === Final Notes === 1. Now the real problem rpmlint detected is fixed and all those warnings were explained by you in a previous comment. 2. Now I was able to successfully build the package in mock. Therefore, your package is... ================ *** APPROVED *** ================ I have tried to be as much clear as I could, but my English is poor, so if something remained obscure please let me know. Also, I would like to hear from you how I have been doing in my first review, so I can continue reviewing other packages also. All the best, Germán.
Hi, Thanks for the review. You forgot to set the fedora-review flag to +, can you please do that? Regards, Hans
I'm sorry Hans. Now it's ok.
New Package SCM Request ======================= Package Name: spice-vdagent Short Description: Agent for Spice guests Owners: jwrdegoede Branches: f14 InitialCC:
Git done (by process-git-requests).
spice-vdagent-0.6.3-2.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/spice-vdagent-0.6.3-2.fc14
Hi, (In reply to comment #8) > I'm sorry Hans. Now it's ok. No problem. Doing things and making mistakes is the only way to learn :) Let me know if you want to swap another review. Regards, Hans
spice-vdagent-0.6.3-2.fc14 has been pushed to the Fedora 14 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update spice-vdagent'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/spice-vdagent-0.6.3-2.fc14
*** Bug 613654 has been marked as a duplicate of this bug. ***
spice-vdagent-0.6.3-2.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ====================== Package Name: spice-vdagent New Branches: el5 Owners: jwrdegoede I would like to introduce spice-vdagent as an EPEL-package for EL5.