Bug 927462 (roscpp_core)

Summary: Review Request: roscpp_core - The ROS C++ API
Product: [Fedora] Fedora Reporter: Rich Mattes <richmattes>
Component: Package ReviewAssignee: Christopher Meng <i>
Status: CLOSED WONTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: i, jwakely, package-review, richmattes, sanjay.ankur, tim
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-02-08 00:46:29 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 871197    
Bug Blocks: 928584    

Description Rich Mattes 2013-03-26 01:30:40 UTC
Spec URL: http://rmattes.fedorapeople.org/rospackages/roscpp-core/roscpp_core.spec
SRPM URL: http://rmattes.fedorapeople.org/rospackages/roscpp-core/roscpp_core-0.2.5-2.gite92f9eb.fc18.src.rpm
Description:
roscpp-core is an underlying library for support roscpp message data
types. It is a lightweight/minimal library that can easily be used
in non-ROS-based projects.

Fedora Account System Username: rmattes

rpmlint:
$ rpmlint roscpp_core.spec ../../RPMS/x86_64/roscpp_core*
roscpp_core.spec: W: invalid-url Source0: wg-debs-roscpp_core-release-upstream-0.2.5-0-ge92f9eb.tar.gz
roscpp_core.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core.x86_64: E: invalid-soname /usr/lib64/libcpp_common.so libcpp_common.so
roscpp_core.x86_64: E: invalid-soname /usr/lib64/libroscpp_serialization.so libroscpp_serialization.so
roscpp_core.x86_64: E: invalid-soname /usr/lib64/librostime.so librostime.so
roscpp_core.x86_64: W: no-documentation
roscpp_core-debuginfo.x86_64: W: spelling-error Summary(en_US) roscpp -> Roscoe
roscpp_core-debuginfo.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core-devel.x86_64: W: spelling-error Summary(en_US) roscpp -> Roscoe
roscpp_core-devel.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
3 packages and 1 specfiles checked; 3 errors, 7 warnings.

Comment 1 Rich Mattes 2013-06-06 01:55:27 UTC
Update:
Spec URL: http://rmattes.fedorapeople.org/rospackages/roscpp-core/roscpp_core.spec
SRPM URL: http://rmattes.fedorapeople.org/rospackages/roscpp-core/roscpp_core-0.2.6-1.20130605gitd0b5ce1.fc19.src.rpm

$ rpmlint roscpp_core.spec ../../RPMS/x86_64/roscpp_core-*0.2.6*
roscpp_core.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core.x86_64: E: invalid-soname /usr/lib64/libroscpp_serialization.so libroscpp_serialization.so
roscpp_core.x86_64: E: invalid-soname /usr/lib64/libcpp_common.so libcpp_common.so
roscpp_core.x86_64: E: invalid-soname /usr/lib64/librostime.so librostime.so
roscpp_core.x86_64: W: no-documentation
roscpp_core-debuginfo.x86_64: W: spelling-error Summary(en_US) roscpp -> Roscoe
roscpp_core-debuginfo.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core-devel.x86_64: W: spelling-error Summary(en_US) roscpp -> Roscoe
roscpp_core-devel.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
3 packages and 1 specfiles checked; 3 errors, 6 warnings.

Comment 2 Ankur Sinha (FranciscoD) 2013-07-24 09:58:37 UTC
[+] OK
[-] NA
[?] Issue

** Mandatory review guidelines: **
 [?] rpmlint output:
[asinha@localhost  SRPMS]$ rpmlint ../SPECS/roscpp_core.spec ./roscpp_core-0.2.6-1.20130605gitd0b5ce1.fc19.src.rpm /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
roscpp_core.src: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core.src: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core.x86_64: E: invalid-soname /usr/lib64/libroscpp_serialization.so libroscpp_serialization.so
roscpp_core.x86_64: E: invalid-soname /usr/lib64/libcpp_common.so libcpp_common.so
roscpp_core.x86_64: E: invalid-soname /usr/lib64/librostime.so librostime.so
roscpp_core.x86_64: W: no-documentation
roscpp_core-debuginfo.x86_64: W: spelling-error Summary(en_US) roscpp -> Roscoe
roscpp_core-debuginfo.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core-devel.x86_64: W: spelling-error Summary(en_US) roscpp -> Roscoe
roscpp_core-devel.x86_64: W: spelling-error %description -l en_US roscpp ->
Roscoe
5 packages and 1 specfiles checked; 3 errors, 8 warnings.
[asinha@localhost  SRPMS]$

^^ The invalid soname errors need to be corrected. Generally, the versioned
soname should be present in the package, and the unversioned soname in the
devel package as a symlink of the versioned soname. At the present time, the
devel package here doesn't even have a shared object in it to link against.


 [+] 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  SPECS]$ review-md5check.sh roscpp_core.spec
mkdir: cannot create directory ‘/tmp/review’: File exists
Getting https://github.com/ros/roscpp_core/archive/d0b5ce1d8f42050a5674875b866a7dda6383a75b/roscpp_core-0.2.6-d0b5ce1.tar.gz
to /tmp/review/roscpp_core-0.2.6-d0b5ce1.tar.gz % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   157  100   157    0     0    119      0  0:00:01  0:00:01 --:--:--   119
  0     0    0 46845    0     0  14953      0 --:--:--  0:00:03 --:--:-- 44444
58af78843ead517ef8be46df06f90c0b  /tmp/review/roscpp_core-0.2.6-d0b5ce1.tar.gz
58af78843ead517ef8be46df06f90c0b /home/asinha/rpmbuild/SOURCES/roscpp_core-0.2.6-d0b5ce1.tar.gz
removed ‘/tmp/review/roscpp_core-0.2.6-d0b5ce1.tar.gz’
removed directory: ‘/tmp/review’
[asinha@localhost  SPECS]$
 [+] 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
^ 
Please use the correct scriptlets

 [+] No bundled libs
 [-] Relocatability is justified
 [+] Package owns all directories it creates
 [-] Package requires others for directories it uses but does not own
 [+] 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
 [-] Static libs go in -static package/virtual Provides
 [+] Development files go in -devel package
 [+] -devel packages Require base with fully-versioned dependency, %_isa
 [+] 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
 [-] Translations of description, summary
 [+] Builds in mock
 [+] Builds on all arches
 [-] Functions as described (e.g. no crashes)
 [?] Scriptlets are sane
^
Missing ldconfig scriptlets

 [+] 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
 [+] 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

[asinha@localhost  result]$ review-req-check
== roscpp_core-0.2.6-1.20130605gitd0b5ce1.fc20.src.rpm ==
Provides:

Requires:
cmake
gtest
boost-devel
python-sphinx
ros-catkin-devel

== roscpp_core-0.2.6-1.20130605gitd0b5ce1.fc20.x86_64.rpm ==
Provides:
libcpp_common.so()(64bit)
libroscpp_serialization.so()(64bit)
librostime.so()(64bit)
roscpp_core = 0.2.6-1.20130605gitd0b5ce1.fc20
roscpp_core(x86-64) = 0.2.6-1.20130605gitd0b5ce1.fc20

Requires:
libboost_date_time-mt.so.1.53.0()(64bit)
libboost_system-mt.so.1.53.0()(64bit)
libboost_thread-mt.so.1.53.0()(64bit)
libc.so.6()(64bit)
libc.so.6(GLIBC_2.14)(64bit)
libc.so.6(GLIBC_2.2.5)(64bit)
libc.so.6(GLIBC_2.4)(64bit)
libgcc_s.so.1()(64bit)
libgcc_s.so.1(GCC_3.0)(64bit)
libm.so.6()(64bit)
libm.so.6(GLIBC_2.2.5)(64bit)
librt.so.1()(64bit)
librt.so.1(GLIBC_2.2.5)(64bit)
libstdc++.so.6()(64bit)
libstdc++.so.6(CXXABI_1.3)(64bit)
libstdc++.so.6(GLIBCXX_3.4)(64bit)
libstdc++.so.6(GLIBCXX_3.4.11)(64bit)
libstdc++.so.6(GLIBCXX_3.4.15)(64bit)
libstdc++.so.6(GLIBCXX_3.4.9)(64bit)
rtld(GNU_HASH)

== roscpp_core-debuginfo-0.2.6-1.20130605gitd0b5ce1.fc20.x86_64.rpm ==
Provides:
roscpp_core-debuginfo = 0.2.6-1.20130605gitd0b5ce1.fc20
roscpp_core-debuginfo(x86-64) = 0.2.6-1.20130605gitd0b5ce1.fc20

Requires:

== roscpp_core-devel-0.2.6-1.20130605gitd0b5ce1.fc20.x86_64.rpm ==
Provides:
pkgconfig(cpp_common) = 0.2.6
pkgconfig(roscpp_serialization) = 0.2.6
pkgconfig(roscpp_traits) = 0.2.6
pkgconfig(rostime) = 0.2.6
roscpp_core-devel = 0.2.6-1.20130605gitd0b5ce1.fc20
roscpp_core-devel(x86-64) = 0.2.6-1.20130605gitd0b5ce1.fc20

Requires:
/usr/bin/pkg-config
roscpp_core(x86-64) = 0.2.6-1.20130605gitd0b5ce1.fc20

[asinha@localhost  result]$


A few issues. The blocker is the missing sonames in the devel subpackage.
Rich, will this package own %{_includedir}/ros or will one of the other
packages, such as ros-release own it? Other ros stacks also might put stuff in
there, right?


Additionally, you can break the docs into a separate sub package if you wish.
Not a blocker.

Comment 3 Rich Mattes 2013-07-25 01:17:51 UTC
Update:

Spec URL: http://rmattes.fedorapeople.org/rospackages/roscpp-core/roscpp_core.spec
SRPM URL: http://rmattes.fedorapeople.org/rospackages/roscpp-core/roscpp_core-0.2.6-2.20130605gitd0b5ce1.fc19.src.rpm

$ rpmlint roscpp_core.spec ../../RPMS/x86_64/roscpp_core*
roscpp_core.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core.x86_64: W: no-documentation
roscpp_core-debuginfo.x86_64: W: spelling-error Summary(en_US) roscpp -> Roscoe
roscpp_core-debuginfo.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe
roscpp_core-devel.x86_64: W: spelling-error Summary(en_US) roscpp -> Roscoe
roscpp_core-devel.x86_64: W: spelling-error %description -l en_US roscpp -> Roscoe


The docs are pretty small, I don't think it's worth it to separate them into a separate subpackage.

roslib-devel from the "ros" and roscpp-devel from the "ros_comm" package also put headers in this folder, so I'll make sure that roslib-devel and roscpp-devel depend on roscpp-core_devel, and I'll make sure this package owns includedir/ros.

Comment 4 Christopher Meng 2013-10-26 16:51:56 UTC
ping Ankur.

Comment 5 Christopher Meng 2013-12-05 10:39:02 UTC
ping *2.

Comment 6 Rich Mattes 2014-04-15 00:21:19 UTC
Ankur, are you still able to do this review?

Comment 7 Christopher Meng 2014-07-09 16:22:13 UTC
(In reply to Rich Mattes from comment #6)
> Ankur, are you still able to do this review?

I will take this, I spoke with Ankur on FUDCon and he agreed to change.

Comment 8 Christopher Meng 2014-07-29 02:18:03 UTC
Is there any new snapshot or version after a year before my review?

Comment 9 Rich Mattes 2014-07-29 23:25:09 UTC
Not for ros-fuerte.  0.2.6 is the latest.

Comment 10 Christopher Meng 2014-07-31 00:24:19 UTC
Is the patch merged upstream?

And, I'd like to know when will Fedora users get the latest ROS? I checked the release page on github, it seems that 0.2.6 is pretty old.

Comment 11 Jonathan Wakely 2015-03-06 15:03:45 UTC
Would it make sense for the %description to explain what ROS stands for, or is everyone looking at Fedora packages assumed to know that?