Spec URL: http://jwilson.fedorapeople.org/packaging/rinputd/rinputd.spec SRPM URL: http://jwilson.fedorapeople.org/packaging/rinputd/rinputd-1.0.1-2.fc12.src.rpm Description: The Remote Input server allows clients to send input events, such as mouse movements or keyboard presses, over a secure, authenticated communication channel. Remote devices, be they separate computers or mobile devices, can provide mouse, keyboard, multi-touch, switches and other input to the Remote Input server, and the server will create virtual keyboards and mice for each connection.
Taking this for review, I'll probably review this tomorrow
Just a few notes before doing the official review: - Why are you not using the %cmake macro? - rpmlint is not silent: [makerpm@laptop-stefan SPECS]$ rpmlint /home/makerpm/rpmbuild/RPMS/i686/rinputd*.rpm rinputd.spec rinputd.i686: E: postin-without-chkconfig /etc/init.d/rinputd rinputd.i686: E: init-script-without-chkconfig-preun /etc/init.d/rinputd rinputd.i686: W: missing-lsb-keyword Required-Start in /etc/init.d/rinputd rinputd.i686: W: missing-lsb-keyword Required-Stop in /etc/init.d/rinputd rinputd.i686: W: missing-lsb-keyword Default-Stop in /etc/init.d/rinputd rinputd.i686: W: service-default-enabled /etc/init.d/rinputd rinputd.i686: E: subsys-not-used /etc/init.d/rinputd rinputd-debuginfo.i686: E: debuginfo-without-sources rinputd-devel.i686: W: non-standard-group Libraries/Development rinputd-devel.i686: W: no-documentation rinputd.spec:23: W: non-standard-group Libraries/Development rinputd.spec: W: no-buildroot-tag 3 packages and 1 specfiles checked; 4 errors, 8 warnings. I'm not saying everything can or should be fixed, but a couple of obvious ones shouldn't be a problem.
Bah. I suck. I usually run rpmlint over everything, but its been a while since I've done a new package... Yeah, I can fix most of that.
And by the way, thanks much for picking up this review, its is indeed very cool stuff... :)
Updated build: http://jwilson.fedorapeople.org/packaging/rinputd/rinputd-1.0.1-3.fc12.src.rpm All rpmlint errors fixed. I was blissfully unaware of the %cmake macro, first package I've ever dealt with that uses cmake. Unfortunately, using %cmake adds a library dependency on libcommon.so.()(<arch>) that doesn't crop up otherwise. The build normally makes a static common/libcommon.a (its own private common lib) that is statically linked into the daemon. The use of %cmake leads to it being built as a shared object, so rpm adds a dependency on libcommon.so(), but the lib isn't in the package, and other packages in fedora already provide a lib of the same name... So it'll require some source hacking to remedy this, I think... I'll ping upstream about it.
Talking with upstream about the cmake issue. We'll switch to this shortly: %cmake -DBUILD_SHARED_LIBS:BOOL=OFF So we're using the macro, just overriding the shared lib behavior. Still working on talking upstream into making libcommon.a into librinput.so... :)
New package ready to go: http://jwilson.fedorapeople.org/packaging/rinputd/rinputd-1.0.2-1.fc12.src.rpm Uses the %cmake macro now, with the shared libs bit off. libcommon.a is an internal-use only thing, its not actually related to the rinput.h header. The 1.0.2 update was put out just for us at my request (thank you, Chase), it adds true daemonization support to rinputd, so the initscript and whatnot are cleaner now.
Ok, great! rpmlint output: [makerpm@laptop-stefan SPECS]$ rpmlint rinputd.spec /home/makerpm/rpmbuild/SRPMS/rinputd-1.0.2-1.fc12.src.rpm /home/makerpm/rpmbuild/RPMS/i686/rinputd-1.0.2-1.fc12.i686.rpm /home/makerpm/rpmbuild/RPMS/i686/rinputd-devel-1.0.2-1.fc12.i686.rpm /home/makerpm/rpmbuild/RPMS/i686/rinputd-debuginfo-1.0.2-1.fc12.i686.rpm rinputd.spec: W: no-buildroot-tag rinputd.src: W: no-buildroot-tag rinputd-devel.i686: W: no-documentation 4 packages and 1 specfiles checked; 0 errors, 3 warnings. I'll do the official review probably tomorrow... Stefan
Stefan, I'm the upstream for this package. Do you know when you will get a chance to review this? I look forward to it being available more easily to fedora users. Thanks
I can review this if Stefan is too busy. BTW, I'm not able to use the app in F-12 with Remotux: even if I disabled the firewall it seems I'm not able to discover my laptop. rinputd --verbose does not show anything other than Listening on port 8771 Avahi service _rinput._tcp with name 'bingo Remote Input' established any idea?
Additionally (not sure this is a bug) hitting CTRL-C gives back the shell but does not actually terminate the daemon
Gianluca, First, thanks for reviewing this. As rinputd logged, it is trying to broadcast using multicast dns. I am not sure exactly why it's not working for you, but it's most likely a network configration issue. I'm using the standard Avahi library for mDNS, so I doubt it's an issue with rinputd. You can also specify the ip address in Remotux instead of using mDNS. As for rinputd running after ctrl-c, it runs as a daemon by default now. This means it forks to the background when it starts, relinquishing foreground control back to the shell. I'm guessing it went back to the shell when you ran it, and you hit ctrl-c, which is like a noop in the shell. You can run rinputd in the foreground using the -f switch if that's what you prefer. Thanks, Chase
(In reply to comment #12) > First, thanks for reviewing this. As rinputd logged, it is trying to broadcast > using multicast dns. I am not sure exactly why it's not working for you, but > it's most likely a network configration issue. I'm using the standard Avahi > library for mDNS, so I doubt it's an issue with rinputd. You can also specify > the ip address in Remotux instead of using mDNS. Also tried to enter the IP, no luck. There should be something really wrong in my setup, I will try to diagnose the issue and let you know. Jarod, how are you using this package right now? > > As for rinputd running after ctrl-c, it runs as a daemon by default now. This > means it forks to the background when it starts, relinquishing foreground > control back to the shell. I'm guessing it went back to the shell when you ran > it, and you hit ctrl-c, which is like a noop in the shell. You can run rinputd > in the foreground using the -f switch if that's what you prefer. Ah, you are right. Maybe --verbose should imply -f but anyway, it was a clear PEBCAK case :)
Ok, some progress here. It seems I can't discover nor access the laptop when it's connected using wifi (wlan0 interface); again, not sure if this is a bug. As soon as I attach it to a cable on the same network, I can definitely discover the service with avahi. However, I don't know which user/password pair to use on the iphone side upon connection; my own user's data does not work there.
One thing that might not be clearly spelled out by simply installing the package is that you need saslauthd up and running to get properly authenticated. Can't recall if I that Just Worked after it was started or not... I'm connecting to a wired zotac ion system running x86_64 Fedora 12.
Gianluca, On Linux there is an extra step required for configuration. Details can be found at: https://wiki.ubuntu.com/RemoteInput/rinputd Essentially, you have to set up Remote Input user accounts through the SASL authentication mechanism.
OK: The package must be named according to the Package Naming Guideline. OK: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. [2] . OK: The package must meet the Packaging Guidelines . OK: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . OK: The License field in the package spec file must match the actual license. OK: 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 must be included in %doc. OK: The spec file must be written in American English. OK: The spec file for the package MUST be legible. NOK: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this. $ md5sum rin* 2d3266b6c3b3865edda9e5d8dd8a1b9c rinputd_1.0.2.tar.gz 2d3266b6c3b3865edda9e5d8dd8a1b9c rinputd_1.0.2.tar.gz.rpm NOTE: The Source0: line is wrong, it is now: http://launchpad.net/rinput/trunk/1.0.1/+download/%{name}_%{version}.tar.gz I suggest: http://launchpad.net/rinput/trunk/%{version}/+download/%{name}_%{version}.tar.gz OK: The package MUST successfully compile and build into binary rpms on at least one primary architecture. Koji scratch build: OK: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line. OK: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines; inclusion of those as BuildRequires is optional. Apply common sense. OK: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.[9] not applicable OK: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. not applicable OK: Packages must NOT bundle copies of system libraries. OK: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. not applicable OK: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. OK: A Fedora package must not list a file more than once in the spec file's %files listings. OK: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. OK: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). OK: Each package must consistently use macros. OK: The package must contain code, or permissable content. OK: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). OK: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. OK: Header files must be in a -devel package. OK: Static libraries must be in a -static package. No static libs are packaged OK: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. OK: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} OK: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. OK: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. not applicable OK: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. not applicable OK: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). OK: All filenames in rpm packages must be valid UTF-8. OK: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. not applicable Ok: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. not available OK: The reviewer should test that the package builds in mock. it builds OK: The package should compile and build into binary rpms on all supported architectures. not applicable, this is a noarch package OK: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example. OK: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity. OK: Usually, subpackages other than devel should require the base package using a fully versioned dependency. OK: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. not applicable, there are no pkgconfig files OK: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. not appplicable So, please fix the Source line, looks good otherwise! Stefan PS, sorry that it took so long, I more or less forgot about this pkg, sorry for that!
And the link to the scratch build I forgot to include http://koji.fedoraproject.org/koji/taskinfo?taskID=1963319
Bah, bz mail got buried... Just pushed out an updated SRPM and spec that include the repaired Source0 URL. http://jwilson.fedorapeople.org/packaging/rinputd/rinputd-1.0.2-2.fc12.src.rpm
Ok great! That means this package is APPROVED
Thanks much, Stefan. Setting review + flag and cvs ? flag to get the cvs bits done... New Package CVS Request ======================= Package Name: rinputd Short Description: A server for receiving input events over the network Owners: jwilson Branches: F-12 InitialCC:
CVS done (by process-cvs-requests.py).
Builds working their way through koji right now.