Spec URL: http://rmattes.fedorapeople.org/rospackages/std-msgs/ros-std_msgs.spec SRPM URL: http://rmattes.fedorapeople.org/rospackages/std-msgs/ros-std_msgs-0.4.8-1.gitef63d31.fc18.src.rpm Description: Standard ROS Messages including common message types representing primitive data types and other basic message constructs, such as multi-arrays. Fedora Account System Username: rmattes rpmlint: $ rpmlint ros-std_msgs.spec ../../RPMS/noarch/ros-std_msgs-devel-0.4.8-1.gitef63d31.fc18.noarch.rpm ros-std_msgs.spec: W: invalid-url Source0: wg-debs-std_msgs-release-upstream-0.4.8-0-gef63d31.tar.gz ros-std_msgs-devel.noarch: W: spelling-error %description -l en_US multi -> mulch, mufti ros-std_msgs-devel.noarch: W: no-documentation ros-std_msgs-devel.noarch: E: zero-length /usr/share/std_msgs/msg/Empty.msg 1 packages and 1 specfiles checked; 1 errors, 3 warnings. The spelling error is part of a hyphenated "multi-arrays" which is OK. I think the empty message needs to be empty to work properly, even if rpmlint throws an error.
Hi Rich, Are the BuildRequires for this package correct? BuildRequires: ros-catkin-devel BuildRequires: ros-genmsg BuildRequires: ros-gencpp BuildRequires: ros-genlisp BuildRequires: ros-genpy We don't have a ros-catkin package, do we? We have a "catkin" package. Similarly, we don't have ros-genmsg, we have a python-genmsg etc. I'll begin the review after you've updated the spec. Thanks, Warm regards, Ankur
They're correct, the "catkin" package has a virtual provides for "ros-catkin" to make installation easier via yum (e.g. yum install ros-*) All of the other packages have similar provides, and I've added them as dependencies to this package (I forgot to when I submitted the review.)
The other packages seem to be in testing. Rich, can you see if they can be pushed to stable so I can use mock/koji to test builds without using a local repo please? I'll review this package later today.
So I removed the ros-* virtual provides from most of the packages; I'll update this spec later tonight with the correct provides. The gen{cpp,lisp,py} builds were only pushed a day or two ago, so they probably won't make it into stable until next week. You should be able to do rawhide mock builds now (once I fix the BuildRequires)
Updated packages Spec URL: http://rmattes.fedorapeople.org/rospackages/std-msgs/ros-std_msgs.spec SRPM URL: http://rmattes.fedorapeople.org/rospackages/std-msgs/ros-std_msgs-0.4.11-2.20130605gitde0dcf1.fc19.src.rpm $ rpmlint ros-std_msgs.spec ../../RPMS/noarch/ros*0.4.11-2* ros-std_msgs.noarch: W: spelling-error %description -l en_US msgs -> mags, megs, mugs ros-std_msgs.noarch: W: spelling-error %description -l en_US multiarrays -> multiracial ros-std_msgs.noarch: W: no-documentation ros-std_msgs.noarch: E: zero-length /usr/share/std_msgs/msg/Empty.msg ros-std_msgs-devel.noarch: W: spelling-error Summary(en_US) msgs -> mags, megs, mugs ros-std_msgs-devel.noarch: W: spelling-error %description -l en_US msgs -> mags, megs, mugs ros-std_msgs-devel.noarch: W: no-documentation 2 packages and 1 specfiles checked; 1 errors, 6 warnings.
[+] OK [-] NA [?] Issue ** Mandatory review guidelines: ** [+] rpmlint output: [asinha@localhost SRPMS]$ rpmlint ../SPECS/ros-std_msgs.spec ./ros-std_msgs-0.4.11-2.20130605gitde0dcf1.fc19.src.rpm /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm ros-std_msgs.src: W: spelling-error %description -l en_US msgs -> mags, megs, mugs ros-std_msgs.src: W: spelling-error %description -l en_US multiarrays -> multiracial ros-std_msgs.noarch: W: spelling-error %description -l en_US msgs -> mags, megs, mugs ros-std_msgs.noarch: W: spelling-error %description -l en_US multiarrays -> multiracial ros-std_msgs.noarch: W: no-documentation ros-std_msgs.noarch: E: zero-length /usr/share/std_msgs/msg/Empty.msg ros-std_msgs.src: W: spelling-error %description -l en_US msgs -> mags, megs, mugs ros-std_msgs.src: W: spelling-error %description -l en_US multiarrays -> multiracial ros-std_msgs-devel.noarch: W: spelling-error Summary(en_US) msgs -> mags, megs, mugs ros-std_msgs-devel.noarch: W: spelling-error %description -l en_US msgs -> mags, megs, mugs ros-std_msgs-devel.noarch: W: no-documentation 4 packages and 1 specfiles checked; 1 errors, 10 warnings. [asinha@localhost SRPMS]$ [+] License is acceptable (...) [+] License field in spec is correct [+] License files included in package %docs if included in source package [+] License files installed when any subpackage combination is installed [+] Spec written in American English [+] Spec is legible [+] Sources match upstream unless altered to fix permissibility issues [asinha@localhost SRPMS]$ review-md5check.sh ../SPECS/ros-std_msgs.spec Getting https://github.com/ros/std_msgs/archive/de0dcf16baaee40f756b9e55656fe2e744bc8fc3/std_msgs-0.4.11-de0dcf1.tar.gz to /tmp/review/std_msgs-0.4.11-de0dcf1.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 154 100 154 0 0 122 0 0:00:01 0:00:01 --:--:-- 123 100 6947 100 6947 0 0 3041 0 0:00:02 0:00:02 --:--:-- 19569 67999742fb86f0ed7b2880e5917ddf5a /tmp/review/std_msgs-0.4.11-de0dcf1.tar.gz 67999742fb86f0ed7b2880e5917ddf5a /home/asinha/rpmbuild/SOURCES/std_msgs-0.4.11-de0dcf1.tar.gz removed ‘/tmp/review/std_msgs-0.4.11-de0dcf1.tar.gz’ removed directory: ‘/tmp/review’ [asinha@localhost SRPMS]$ [+] Build succeeds on at least one primary arch [+] Build succeeds on all primary arches or has ExcludeArch + bugs filed [+] BuildRequires correct, justified where necessary [-] Locales handled with %find_lang, not %_datadir/locale/* [+] %post, %postun call ldconfig if package contains shared .so files [+] No bundled libs [-] Relocatability is justified [+] Package owns all directories it creates [?] Package requires others for directories it uses but does not own [asinha@localhost result]$ review-req-check == ros-std_msgs-0.4.11-2.20130605gitde0dcf1.fc20.noarch.rpm == Provides: ros-std_msgs = 0.4.11-2.20130605gitde0dcf1.fc20 Requires: python(abi) = 2.7 ros-release == ros-std_msgs-0.4.11-2.20130605gitde0dcf1.fc20.src.rpm == Provides: Requires: cmake python-setuptools-devel catkin-devel python-genmsg-devel python-gencpp-devel python-genlisp-devel python-genpy-devel == ros-std_msgs-devel-0.4.11-2.20130605gitde0dcf1.fc20.noarch.rpm == Provides: pkgconfig(std_msgs) = 0.4.11 ros-std_msgs = 0.4.11-2.20130605gitde0dcf1.fc20 ros-std_msgs-devel = 0.4.11-2.20130605gitde0dcf1.fc20 Requires: /usr/bin/pkg-config [asinha@localhost result]$ ^^ 1.Just confirming: Which package that is Required by this one is the %{_datadir}/common-lisp/ros/ directory owned by? 2. Shouldn't the package require the non devel versions of the python BRs to function? [+] No duplication in %files unless necessary for license files [+] File permissions are sane [+] Package contains permissible code or content [-] Large docs go in -doc subpackage [?] %doc files not required at runtime There is no documentation at all. No licence or even a README :/ [-] Static libs go in -static package/virtual Provides [+] Development files go in -devel package [+] -devel packages Require base with fully-versioned dependency, %_isa Noarch so isa isn't needed [+] No .la files [-] GUI app uses .desktop file, installs it with desktop-file-install [-] File list does not conflict with other packages' without justification [+] File names are valid UTF-8 ** Optional review guidelines: ** [?] Query upstream about including license files We can, but I don't think ROS intends to include licence files in any of it's packages. Should we make ros-release include a license file if it doesn't already, since all these packages will be expected to Require it? [-] Translations of description, summary [+] Builds in mock [+] Builds on all arches [-] Functions as described (e.g. no crashes) [-] Scriptlets are sane [+] Subpackages require base with fully-versioned dependency if sensible [+] .pc file subpackage placement is sensible [+] No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin [-] Include man pages if available Naming guidelines: [+] Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+ [+] Package names are sane [+] No naming conflicts [+] Spec file name matches base package name [+] Version is sane [+] Version does not contain ~ [+] Release is sane [+] %dist tag [+] Case used only when necessary [-] Renaming handled correctly Packaging guidelines: [+] Useful without external bits [+] No kmods [-] Pre-built binaries, libs removed in %prep [+] Sources contain only redistributable code or content [+] Spec format is sane [+] Package obeys FHS, except libexecdir, /run, /usr/target [+] No files in /bin, /sbin, /lib* on >= F17 [-] Programs run before FS mounting use /run instead of /var/run [-] Binaries in /bin, /sbin do not depend on files in /usr on < F17 [-] No files under /srv, /opt, /usr/local [+] Changelog in prescribed format [+] No Packager, Vendor, Copyright, PreReq tags [+] Summary does not end in a period [-] Correct BuildRoot tag on < EL6 [-] Correct %clean section on < EL6 [?] Requires correct, justified where necessary The directory owning the list sub dir needs clarification [+] Summary, description do not use trademarks incorrectly [-] All relevant documentation is packaged, appropriately marked with %doc [-] Doc files do not drag in extra dependencies (e.g. due to +x) [-] Code compilable with gcc is compiled with gcc [+] Build honors applicable compiler flags or justifies otherwise [-] PIE used for long-running/root daemons, setuid/filecap programs [+] Useful -debuginfo package or disabled and justified [-] Package with .pc files Requires pkgconfig on < EL6 [+] No static executables [+] Rpath absent or only used for internal libs [+] Config files marked with %config(noreplace) or justified %config [+] No config files under /usr [-] Third party package manager configs acceptable, in %_docdir [-] .desktop files are sane [+] Spec uses macros consistently [+] Spec uses macros instead of hard-coded names where appropriate [+] Spec uses macros for executables only when configurability is needed [+] %makeinstall used only when alternatives don't work [+] Macros in Summary, description are expandable at srpm build time [+] Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir [+] No software collections (scl) [+] Macro files named /etc/rpm/macros.%name [+] Build uses only python/perl/shell+coreutils/lua/BuildRequired langs [+] %global, not %define [-] Package translating with gettext BuildRequires it [-] Package translating with Linguist BuildRequires qt-devel [+] File ops preserve timestamps [+] Parallel make [+] No Requires(pre,post) notation [-] User, group creation handled correctly (See Packaging:UsersAndGroups) [-] Web apps go in /usr/share/%name, not /var/www [-] Conflicts are justified [+] One project per package [+] No bundled fonts [-] Patches have appropriate commentary [-] Available test suites executed in %check [-] tmpfiles.d used for /run, /run/lock on >= F15 ** Python guidelines: ** [?] Runtime Requires correct Should this package require other ros python packages, the non devel versions of ones required as BRs? [-] Python macros declared on < EL6 [+] All .py files packaged with .pyc, .pyo counterparts [?] Includes .egg-info files/directories when generated Should egg info be generated for this package? [-] Provides/Requires properly filtered [-] Code that invokes gtk.gdk.get_pixels_array() Requires numpy Are all the files in the non devel package files that are required at runtime? I think they are. Do you think you need to split the package further in to a python and a lisp subpackage? Then we'd have: a main package a python bindings package a list package a c++ headers package The package is mostly OK. Some tiny issues that need to be handled.
Basically, the python and lisp compontents are the generated messages. They are required during runtime for any python or lisp program that needs the messages, though not every ROS program will need one or both of them. I think it would be possible to split them out into something like package-python-msgs and package-lisp-msgs, but I'm not convinced it's worth the effort. As far as /usr/share/common-lisp/ros goes, I'm not sure we can use that folder as per [1]. I think I may have to change the way that catkin is installing the lisp messages so that it lands in a packagename subfolder (or maybe we can just move the ros/ folder into common-lisp/source and have ros-release own it) You're right about requiring the runtime genpy and genlisp libraries: all the python getnpy messages import genpy. I'm not sure about lisp, and it looks like the c++ generated headers require headers from roscpp_core-devel. I'll update the Requires accordingly. I don't think egg-info needs to be generated; that guideline is only for when upstream does generate it, which isn't the case here. [1] http://fedoraproject.org/wiki/Packaging:Lisp#Install_location_and_hooking_into_the_common-lisp-controller
Update: Spec URL: http://rmattes.fedorapeople.org/rospackages/std-msgs/ros-std_msgs.spec SRPM URL: http://rmattes.fedorapeople.org/rospackages/std-msgs/ros-std_msgs-0.4.11-3.20130605gitde0dcf1.fc19.src.rpm I tried to address the concerns you mentioned above, and I also tried to conform to the common-lisp packaging guidelines $ rpmlint ros-std_msgs.spec ../../RPMS/noarch/ros-std_msgs*-0.4.11-3* ros-std_msgs.noarch: W: spelling-error %description -l en_US msgs -> mags, megs, mugs ros-std_msgs.noarch: W: spelling-error %description -l en_US multiarrays -> multiracial ros-std_msgs.noarch: W: no-documentation ros-std_msgs.noarch: E: zero-length /usr/share/std_msgs/msg/Empty.msg ros-std_msgs-devel.noarch: W: spelling-error Summary(en_US) msgs -> mags, megs, mugs ros-std_msgs-devel.noarch: W: spelling-error %description -l en_US msgs -> mags, megs, mugs ros-std_msgs-devel.noarch: W: no-documentation 2 packages and 1 specfiles checked; 1 errors, 6 warnings.
This ticket has blocked another review for a long time.
You're welcome to take over and continue the review.
0.5.8 is its latest version, right? https://github.com/ros/std_msgs/releases
Yes and no. The releases of the ROS packages correspond with different ROS distributions. The packages all in Fedora right now are for the Fuerte ROS distribution, so the package needs to be kept in line with the latest compatible fuerte release[1] which is 0.4.11 [1] http://ros.org/rosinstalls/fuerte-ros-base.rosinstall
0. Ankur wanted you to split out many subpkgs, well I don't have any ideas, is it worth to be considered? I think it's not. You ca split out any stuffs later but now I think we don't have that need. 1. BuildRequires: python-setuptools-devel --> BuildRequires: python-setuptools Also missing python2-devel 2. /usr/sbin/register-common-lisp-source std_msgs why not /usr/sbin/register-common-lisp-source %{stackname} ? ;) 3. %{python_sitelib} --> %{python2_sitelib} 4. Installation errors ------------------- INFO: mock.py version 1.1.33 starting... Start: init plugins INFO: selinux enabled Finish: init plugins Start: run Mock Version: 1.1.33 INFO: Mock Version: 1.1.33 Start: lock buildroot INFO: installing package(s): /home/rpmaker/Desktop/ros-std_msgs/results/ros-std_msgs-0.4.11-3.20130605gitde0dcf1.fc21.noarch.rpm /home/rpmaker/Desktop/ros-std_msgs/results/ros-std_msgs-devel-0.4.11-3.20130605gitde0dcf1.fc21.noarch.rpm ERROR: Command failed: # ['/usr/bin/yum', '--installroot', '/var/lib/mock/fedora-rawhide-i386/root/', 'install', '/home/rpmaker/Desktop/ros-std_msgs/results/ros-std_msgs-0.4.11-3.20130605gitde0dcf1.fc21.noarch.rpm', '/home/rpmaker/Desktop/ros-std_msgs/results/ros-std_msgs-devel-0.4.11-3.20130605gitde0dcf1.fc21.noarch.rpm', '--setopt=tsflags=nocontexts'] Error: Package: ros-std_msgs-devel-0.4.11-3.20130605gitde0dcf1.fc21.noarch (/ros-std_msgs-devel-0.4.11-3.20130605gitde0dcf1.fc21.noarch) Requires: roscpp_core-devel I just added a new depends. 5. Any explanation of: ros-std_msgs.noarch: E: zero-length /usr/share/std_msgs/msg/Empty.msg ?