Bug 871197 (catkin) - Review Request: catkin - Collection of CMake macros for ROS
Summary: Review Request: catkin - Collection of CMake macros for ROS
Keywords:
Status: CLOSED ERRATA
Alias: catkin
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ankur Sinha (FranciscoD)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: ros-release
Blocks: rospack roscpp_core 927470
TreeView+ depends on / blocked
 
Reported: 2012-10-29 21:45 UTC by Rich Mattes
Modified: 2013-05-02 03:50 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-04-20 19:44:32 UTC
sanjay.ankur: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 926034 None None None Never

Internal Links: 926034

Description Rich Mattes 2012-10-29 21:45:34 UTC
Spec URL: http://rmattes.fedorapeople.org/rospackages/catkin/python-catkin.spec
SRPM URL: http://rmattes.fedorapeople.org/rospackages/catkin/python-catkin-0.4.4-2.git3c6beb1.fc17.src.rpm
Description: 
Catkin is the official build system of ROS and the successor to the original
ROS build system, rosbuild.  catkin uses the cmake build system for the 
actual build, and adds some features to cmake using cmake macros and python 
scripts.

Fedora Account System Username: rmattes

Comment 1 Rich Mattes 2013-01-29 04:23:28 UTC
Update to use new ros-release package:

Spec URL: http://rmattes.fedorapeople.org/rospackages/catkin/python-catkin.spec
SRPM URL: http://rmattes.fedorapeople.org/rospackages/catkin/python-catkin-0.4.5-3.gitd4f1f24.fc18.src.rpm

rpmlint still needs some attention, I'll clean it up in a day or two:

python-catkin.spec: W: invalid-url Source0: ros-catkin-0.4.5-0-gd4f1f24.tar.gz
python-catkin.noarch: E: devel-dependency python-setuptools-devel
python-catkin.noarch: W: no-documentation
python-catkin.noarch: W: non-conffile-in-etc /etc/catkin/profile.d/00.catkin.sh
python-catkin.noarch: E: script-without-shebang /usr/bin/catkin_util.sh
python-catkin.noarch: W: non-conffile-in-etc /etc/catkin/profile.d/00.catkin.all
python-catkin.noarch: W: non-conffile-in-etc /etc/catkin/profile.d/00.catkin.tcsh
python-catkin.noarch: W: non-conffile-in-etc /etc/catkin/profile.d/00.catkin.zsh
python-catkin.noarch: W: non-conffile-in-etc /etc/catkin/profile.d/00.catkin.bash
python-catkin.noarch: E: non-executable-script /usr/share/catkin/setup.bash 0644L /bin/bash
python-catkin.noarch: W: non-conffile-in-etc /etc/catkin/profile.d/00.catkin.csh
python-catkin.noarch: W: hidden-file-or-dir /usr/share/catkin/.rosinstall
python-catkin.noarch: E: non-executable-script /usr/share/catkin/setup.zsh 0644L /bin/zsh
python-catkin.noarch: E: non-executable-script /usr/share/catkin/setup.sh 0644L /bin/sh
python-catkin.noarch: W: no-manual-page-for-binary catkin_install_parse
python-catkin.noarch: W: no-manual-page-for-binary catkin-version
python-catkin.noarch: W: no-manual-page-for-binary git-catkin-track-all
python-catkin.noarch: W: no-manual-page-for-binary catkin_util.sh
python-catkin.noarch: W: no-manual-page-for-binary git-catkin
python-catkin.noarch: W: no-manual-page-for-binary catkin-parse-stack
python-catkin.noarch: W: no-manual-page-for-binary catkin-bump-version
python-catkin.noarch: W: no-manual-page-for-binary catkin-build-debs-of-workspace
python-catkin.noarch: W: no-manual-page-for-binary catkin-topological-order
python-catkin-devel.noarch: W: no-documentation
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.sh.installable.in 0644L /bin/sh
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.sh.installable.in.etc 0644L /bin/sh
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.zsh.installable.in 0644L /bin/zsh
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.bash.installable.in 0644L /bin/bash
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.bash.buildspace.in 0644L /bin/bash
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/update_project_index.py.in 0644L /usr/bin/env
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.sh.buildspace.in 0644L /bin/sh
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.zsh.buildspace.in 0644L /bin/zsh
python-catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/interrogate_setup_dot_py.py 0644L /usr/bin/env
2 packages and 1 specfiles checked; 14 errors, 19 warnings.

Comment 2 Rich Mattes 2013-03-21 02:12:49 UTC
Alright I think the package is in good enough shape to be reviewed now.  After a day or two that turned into a month or two...

Spec URL: http://rmattes.fedorapeople.org/rospackages/catkin/python-catkin.spec
SRPM URL: http://rmattes.fedorapeople.org/rospackages/catkin/python-catkin-0.4.5-4.gitd4f1f24.fc18.src.rpm

rpmlint output:
$ rpmlint python-catkin.spec ../RPMS/noarch/python-catkin-0.4.5-4.gitd4f1f24.fc18.noarch.rpm 
python-catkin.spec: W: invalid-url Source0: ros-catkin-0.4.5-0-gd4f1f24.tar.gz
python-catkin.noarch: W: no-manual-page-for-binary catkin_install_parse
python-catkin.noarch: W: no-manual-page-for-binary git-catkin
python-catkin.noarch: W: no-manual-page-for-binary git-catkin-track-all
python-catkin.noarch: W: no-manual-page-for-binary catkin-version
python-catkin.noarch: W: no-manual-page-for-binary catkin-parse-stack
python-catkin.noarch: W: no-manual-page-for-binary catkin-bump-version
python-catkin.noarch: W: no-manual-page-for-binary catkin-build-debs-of-workspace
python-catkin.noarch: W: no-manual-page-for-binary catkin-topological-order
1 packages and 1 specfiles checked; 0 errors, 9 warnings.

The package is a checkout from github, and the checkout command is included in the spec comments.  The package also doesn't have manpages, so the warnings are all OK.

Comment 3 Ankur Sinha (FranciscoD) 2013-03-21 03:06:14 UTC
Hi Rich,

I'll try and review this by the weekend. 

Thanks,
Ankur

Comment 4 Ankur Sinha (FranciscoD) 2013-03-23 10:49:31 UTC
REVIEW:

[+] OK
[-] NA
[?] Issue

** Mandatory review guidelines: **
 [?] rpmlint output:
[ankur@dhcppc1  SRPMS]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm ../SPECS/python-catkin.spec ./python-catkin-0.4.5-4.gitd4f1f24.fc18.src.rpm

python-catkin.noarch: W: no-manual-page-for-binary catkin_install_parse
python-catkin.noarch: W: no-manual-page-for-binary git-catkin
python-catkin.noarch: W: no-manual-page-for-binary git-catkin-track-all
python-catkin.noarch: W: no-manual-page-for-binary catkin-version
python-catkin.noarch: W: no-manual-page-for-binary catkin-parse-stack
python-catkin.noarch: W: no-manual-page-for-binary catkin-bump-version
python-catkin.noarch: W: no-manual-page-for-binary
catkin-build-debs-of-workspace
python-catkin.noarch: W: no-manual-page-for-binary catkin-topological-order
python-catkin.src: W: invalid-url Source0: ros-catkin-0.4.5-0-gd4f1f24.tar.gz
python-catkin-devel.noarch: W: no-documentation
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/templates/setup.sh.installable.in 0644L /bin/sh
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/templates/setup.sh.installable.in.etc 0644L /bin/sh
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/templates/setup.zsh.installable.in 0644L /bin/zsh
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/templates/setup.bash.installable.in 0644L /bin/bash
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/templates/setup.bash.buildspace.in 0644L /bin/bash
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/templates/update_project_index.py.in 0644L /usr/bin/env
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/templates/setup.sh.buildspace.in 0644L /bin/sh
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/templates/setup.zsh.buildspace.in 0644L /bin/zsh
python-catkin-devel.noarch: E: non-executable-script
/usr/share/catkin/cmake/interrogate_setup_dot_py.py 0644L /usr/bin/env
../SPECS/python-catkin.spec: W: invalid-url Source0:
ros-catkin-0.4.5-0-gd4f1f24.tar.gz
python-catkin.src: W: invalid-url Source0: ros-catkin-0.4.5-0-gd4f1f24.tar.gz
4 packages and 1 specfiles checked; 9 errors, 12 warnings.
[ankur@dhcppc1  SRPMS]$

^^
Couple of rpmlint errors. Can be easily corrected.

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

^^ Couldn't find a license file. Please request upstream to include one.

 [+] Spec written in American English
 [+] Spec is legible
 [-] Sources match upstream unless altered to fix permissibility issues
^^ NA: generated from git tag

 [+] 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
 [+] 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 isa, but it's a noarch

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

 [-] Translations of description, summary
 [+] Builds in mock
 [+] Builds on all arches
 [?] Functions as described (e.g. no crashes)
^^
Need to check this

 [-] 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
^^ 
Do we intend to support EPEL for ROS? Could be a good thing.

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


 ** Python guidelines: **
 [+] Runtime Requires correct
 [+] Python macros declared on < EL6
 [+] All .py files packaged with .pyc, .pyo counterparts
 [+] Includes .egg-info files/directories when generated
 [+] Provides/Requires properly filtered
 [-] Code that invokes gtk.gdk.get_pixels_array() Requires numpy


Typo in the -devel package's Requires:

The docs say that it requires python-catkin_pkg to build, but we haven't packaged it yet. http://www.ros.org/wiki/catkin

Could this be because the current package version is 0.5.65?

https://github.com/ros/catkin/tags

While this package can be approved after a few fixes, I think the newest version should be packaged and reviewed.

Thanks,
Warm regards,
Ankur

Comment 5 Ankur Sinha (FranciscoD) 2013-03-23 10:51:06 UTC
I was also wondering if the package needs to be named python-catkin? Wouldn't just "catkin" be okay? It isnt' a python module and the python prefix is therefore not necessary IMO.

Comment 6 Ankur Sinha (FranciscoD) 2013-03-23 11:02:22 UTC
Hi Rich,

I've submitted python-catkin_pkg for review here: https://bugzilla.redhat.com/show_bug.cgi?id=926034

It's a really simple package. Please check if this one requires it. If it does, we can review it quickly and push it to the repos.

Thanks,
Warm regards,
Ankur

Comment 7 Rich Mattes 2013-03-25 23:33:28 UTC
The 4.x versions of catkin don't require catkin-pkg yet, only the groovy versions.  Since I'm targeting Fuerte first, I'm trying to stick to the package versions listed in http://ros.org/rosinstalls/fuerte-ros-base.rosinstall This includes catkin 4.5.  Once we upgrade to groovy, catkin-pkg will be needed.

I named it python-catkin because i thought it was installed by pypi (though I think I was just confused, I was doing all of the other python-ros* utilities at the same time.)  I'll rename the package to catkin.

I think most of the rpmlint errors are OK (the ones with the .in files).  The .in files are templates that catkin uses when it generates build files for other packages, so they never get executed directly.  That's why they're in share and don't have an executable perm.

The isa stuff is OK since catkin is a noarch package (it doesn't have an isa)

Thus far I have been putting all the ROS packages I've gotten reviewed into EPEL6.  This won't be an exception, so I added a %clean section.

Functionally it should work fine.  I used it for the rest of the ROS chain which I will be submitting for review soon.

Ticket asking for license file at:
https://github.com/ros/catkin/issues/398

So new packages are at:Spec URL: http://rmattes.fedorapeople.org/rospackages/catkin/catkin.spec
SRPM URL: http://rmattes.fedorapeople.org/rospackages/catkin/catkin-0.4.5-5.gitd4f1f24.fc18.src.rpm

Comment 8 Ankur Sinha (FranciscoD) 2013-03-26 13:29:56 UTC
Errors have been corrected.

rpmlint:

[ankur@dhcppc1  SRPMS]$ rpmlint ../SPECS/catkin.spec ./catkin-0.4.5-5.gitd4f1f24.fc18.src.rpm /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
../SPECS/catkin.spec: W: invalid-url Source0: ros-catkin-0.4.5-0-gd4f1f24.tar.gz
catkin.src: W: invalid-url Source0: ros-catkin-0.4.5-0-gd4f1f24.tar.gz
catkin.noarch: W: no-manual-page-for-binary catkin_install_parse
catkin.noarch: W: no-manual-page-for-binary git-catkin
catkin.noarch: W: no-manual-page-for-binary git-catkin-track-all
catkin.noarch: W: no-manual-page-for-binary catkin-version
catkin.noarch: W: no-manual-page-for-binary catkin-parse-stack
catkin.noarch: W: no-manual-page-for-binary catkin-bump-version
catkin.noarch: W: no-manual-page-for-binary catkin-build-debs-of-workspace
catkin.noarch: W: no-manual-page-for-binary catkin-topological-order
catkin.src: W: invalid-url Source0: ros-catkin-0.4.5-0-gd4f1f24.tar.gz
catkin-devel.noarch: W: no-documentation
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.sh.installable.in 0644L /bin/sh
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.sh.installable.in.etc 0644L /bin/sh
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.zsh.installable.in 0644L /bin/zsh
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.bash.installable.in 0644L /bin/bash
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.bash.buildspace.in 0644L /bin/bash
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/update_project_index.py.in 0644L /usr/bin/env
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.sh.buildspace.in 0644L /bin/sh
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/templates/setup.zsh.buildspace.in 0644L /bin/zsh
catkin-devel.noarch: E: non-executable-script /usr/share/catkin/cmake/interrogate_setup_dot_py.py 0644L /usr/bin/env
4 packages and 1 specfiles checked; 9 errors, 12 warnings.
[ankur@dhcppc1  SRPMS]$

The *.in files are okay I guess. Although, the templates would've been better without shebangs. What about the one py file? :/

That's all I can find. The rest looks okay. I can't quite figure out what this one py file does. Does it need to have the shebang? I checked up. Py files in /usr/share have their shebangs removed.

Thanks,
Warm regards,
Ankur

Comment 9 Rich Mattes 2013-03-28 01:51:07 UTC
Updated packages:

Spec URL: http://rmattes.fedorapeople.org/rospackages/catkin/catkin.spec
SRPM URL: http://rmattes.fedorapeople.org/rospackages/catkin/catkin-0.4.5-6.gitd4f1f24.fc18.src.rpm

rpmlint:
$ rpmlint catkin.spec ../RPMS/noarch/catkin-*
catkin.spec: W: invalid-url Source0: ros-catkin-0.4.5-0-gd4f1f24.tar.gz
catkin.noarch: W: no-manual-page-for-binary catkin_install_parse
catkin.noarch: W: no-manual-page-for-binary git-catkin
catkin.noarch: W: no-manual-page-for-binary git-catkin-track-all
catkin.noarch: W: no-manual-page-for-binary catkin-version
catkin.noarch: W: no-manual-page-for-binary catkin-parse-stack
catkin.noarch: W: no-manual-page-for-binary catkin-bump-version
catkin.noarch: W: no-manual-page-for-binary catkin-build-debs-of-workspace
catkin.noarch: W: no-manual-page-for-binary catkin-topological-order
catkin-devel.noarch: W: no-documentation
2 packages and 1 specfiles checked; 0 errors, 10 warnings.


I checked on the usage of the python helpers.  It looks like they're being invoked with python instead of executed directly, so they don't need the shebangs.  I also got rid of the shebangs in the templates; they're meant to be sourced by other scripts and don't really need shebangs either.

Comment 10 Ankur Sinha (FranciscoD) 2013-04-09 12:52:27 UTC
Hi Rich,

Yep. The scripts are invoked with python, and not invoked directly as far as I can tell too. The scripts being sourced don't need shebangs either. 

The rpmlint errors have been corrected. The package now looks good to me.

XXX APPROVED XXX

Thanks,
Warm regards,
Ankur

Comment 11 Rich Mattes 2013-04-09 22:54:26 UTC
New Package SCM Request
=======================
Package Name: catkin
Short Description: Collection of CMake macros for ROS
Owners: rmattes
Branches: f18 f19 el6
InitialCC:

Comment 12 Gwyn Ciesla 2013-04-10 10:41:06 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2013-04-12 13:06:15 UTC
catkin-0.4.5-6.gitd4f1f24.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/catkin-0.4.5-6.gitd4f1f24.fc19

Comment 14 Fedora Update System 2013-04-12 13:06:29 UTC
catkin-0.4.5-6.gitd4f1f24.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/catkin-0.4.5-6.gitd4f1f24.fc18

Comment 15 Fedora Update System 2013-04-12 15:17:09 UTC
catkin-0.4.5-6.gitd4f1f24.fc19 has been pushed to the Fedora 19 testing repository.

Comment 16 Fedora Update System 2013-04-20 19:44:35 UTC
catkin-0.4.5-6.gitd4f1f24.fc19 has been pushed to the Fedora 19 stable repository.

Comment 17 Fedora Update System 2013-05-02 03:50:17 UTC
catkin-0.4.5-6.gitd4f1f24.fc18 has been pushed to the Fedora 18 stable repository.


Note You need to log in before you can comment on or make changes to this bug.