Bug 538820 - Review Request: roboptim-core - Numerical optimization for robotics
Summary: Review Request: roboptim-core - Numerical optimization for robotics
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-11-19 14:02 UTC by Thomas Moulard
Modified: 2010-02-23 05:24 UTC (History)
3 users (show)

Fixed In Version: roboptim-core-0.5-2.fc12
Clone Of:
Environment:
Last Closed: 2010-02-02 17:04:27 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Thomas Moulard 2009-11-19 14:02:26 UTC
Spec URL: http://homepages.laas.fr/~tmoulard/fedora/roboptim-core-0.4-1.spec
SRPM URL: http://homepages.laas.fr/~tmoulard/fedora/roboptim-core-0.4-1.fc11.src.rpm

Description:

Hi! Here is a robotics package I have developed and packaged.
It is the first software I package for Fedora so please forgive the
errors I may have done.

RobOptim provide C++ numerical optimization libraries for robotics.
RobOptim core focuses on providing abstractions for optimization problem
definition and infrastructure to allow the use of existing solvers as   
a back-end.

RobOptim is composed of several packages: core, trajectory (for trajectory optimization) and plug-ins (two packages have been developed but only one is acceptable by Fedora). I will request reviews for the two others if this one gets accepted.

A successful Koji build (i386 / f12) of this package is available here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1816570

Thanks you by advance.

Comment 1 Thomas Moulard 2009-11-19 14:11:59 UTC
rpmlint output:
$ rpmlint -i ~/rpmbuild/RPMS/i586/roboptim-core-0.4-1.fc11.i586.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint -i ~/rpmbuild/RPMS/i586/roboptim-core-devel-0.4-1.fc11.i586.rpm
roboptim-core-devel.i586: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

roboptim-core-devel.i586: W: dangling-relative-symlink /usr/lib/roboptim-core-dummy-plugin.so roboptim-core-dummy-plugin.so.1.0.1
The target of the symbolic link does not exist within this package or its file
based dependencies.  Verify spelling of the link target and that the target is
included in a package in this package's dependency chain.

1 packages and 0 specfiles checked; 0 errors, 2 warnings.

$ rpmlint -i ~/rpmbuild/RPMS/i586/roboptim-core-doc-0.4-1.fc11.i586.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

comments:

roboptim-core-devel.i586: W: no-documentation
There is a separate documentation package so I do not have anything to put in this one.

roboptim-core-devel.i586: W: dangling-relative-symlink
As stated in [1] in ``Devel Packages'' section, the unversioned shared library /only/ is in the devel package, the versioned one is in the ``roboptim-core'' package. So package dependency provides the symlink target.

[1] https://fedoraproject.org/w/index.php?title=Packaging:Guidelines&oldid=133795

Comment 2 Mamoru TASAKA 2010-01-08 09:09:38 UTC
Some notes:

* SourceURL
  - For sourceforge hosted tarball, please use:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

    By the way while I could download a tarball from
    http://downloads.sourceforge.net/roboptim/%{name}-%{version}.tar.gz ,
                                      ^^^^^^^^^^^^^^^
    the one I could download differs from the tarball in your srpm:
-----------------------------------------------------------------
498194 2009-11-17 21:53 original/roboptim-core-0.4.tar.gz
498224 2009-11-17 02:21 roboptim-core-0.4-1.fc11.src/roboptim-core-0.4.tar.gz
-----------------------------------------------------------------

* Requires
  - "Requires: pkgconfig" on -devel package is no longer needed (on Fedora,
    not on EPEL) because Fedora rpmbuild automatically detects this
    dependency.

  - Requires: roboptim-core = %{version}-%{release}
    can be replaced with
    Requires: %{name} = %{version}-%{release}

  - As -devel subpackage contains the following files (for example)
    /usr/include/roboptim/core/function.hh:
----------------------------------------------------------------
    28  # include <boost/numeric/ublas/matrix.hpp>
    29  # include <boost/numeric/ublas/vector.hpp>
    30  # include <boost/tuple/tuple.hpp>
----------------------------------------------------------------
    /usr/include/roboptim/core/solver-factory.hh
----------------------------------------------------------------
    26  # include <ltdl.h>
----------------------------------------------------------------
    -devel subpackage should have "Requires: boost-devel"
    and "Requires: libtool-ltdl-devel" (here I am not saying about
    BuildRequires).

? pkgconfig file
  - By the way you specify configure option:
----------------------------------------------------------------
--docdir=%{_datadir}/doc/%{name}-%{version}
----------------------------------------------------------------
    Does this mean that you have to modify docdir & doxygendocdir
    in build-aux/pkg-config.pc.in (and the installed roboptim-core.pc) ?

* Non-library in %_libdir
  - What is the file "roboptim-core-dummy-plugin.so.1" in %_libdir?

    This file does not have the name "libXXXX.so.Y", so this
    does not seem to be a system-wide library, and this should
    be moved to some subdirectory under %_libdir (like %_libdir/%name).

    ! Note
      the reason rpmlint warns about "dangling-relative-symlink" on
      -devel package is because "roboptim-core-dummy-plugin.so.1" is
      not regarded as a library (because this file does not have
      the name libXXXX.so.Y)
      ( you can see that actually rpmlint does not show this waring
        on libroboptim-core.so )

* Timestamps
  - Please consider to use
-----------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
-----------------------------------------------------------------
    to keep timestamps on installed files as much as possible.
    This method usually works for Makefiles generated by recent
    autotools.

* About -doc subpackage
  - Currently only 5 files (AUTHORS COPYING THANKS README NEWS) are
    included in the -doc subpackage and I guess this is not
    what you expect.

    ! Note
      With the following lines
----------------------------------------------------------------
    64  %files
    65  %defattr(-,root,root,-)
    66  %doc AUTHORS COPYING THANKS README NEWS
----------------------------------------------------------------
      rpmbuild
      - Firstly clean up the directory %buildroot%_docdir/%name-%version
      - Then again create the directory %buildroot%_docdir/%name-%version
      - and installs the files specified in %doc to this directory
      (please check build.log what is actually done by rpmbuild)

    Also please check if needed BuildRequies are correctly written.
    build.log says:
-----------------------------------------------------------------
   336  make[3]: Nothing to be done for `install-exec-am'.
   337  ../config.status --file="Doxyfile.extra":"Doxyfile.extra.in"
   338  config.status: creating Doxyfile.extra
   339  config.status: creating Doxyfile
   340  sh: latex: command not found
   341  sh: dvips: command not found
   342  make[3]: Leaving directory `/builddir/build/BUILD/roboptim-core-0.4/doc'
-----------------------------------------------------------------

Comment 3 Thomas Moulard 2010-01-13 14:19:12 UTC
Here is my new try:
Spec URL: http://homepages.laas.fr/~tmoulard/fedora/roboptim-core-0.5-1.spec
SRPM URL:
http://homepages.laas.fr/~tmoulard/fedora/roboptim-core-0.5-1.fc11.src.rpm

* SourceURL
    I stick as close as possible to the standard but unfortunately the
    package is called roboptim-core and the SF project roboptim so I cannot
    use %{name} directly.

Source0: http://downloads.sourceforge.net/roboptim/%{name}-%{version}.tar.gz

    ...seems the best I can do.

* Requires
  - I removed pkgconfig from requirements in -devel.

  - I have replace roboptim-core = %{version}-%{release}
    by
    Requires: %{name} = %{version}-%{release}

  - I have added "Requires: boost-devel, libtool-ltdl-devel" to -devel.

* pkgconfig file
  - I fixed the bug in docdir & doxygendocdir
    in build-aux/pkg-config.pc.in. Thanks for reporting!

* Non-library in %_libdir
  - I now package my plug-in ``roboptim-core-dummy-plugin'' in
    the main package (roboptim-core) and install it in ``%{_libdir}/%{name}''.
    As I am developping the package and I do not want to bother patching
    the rpm only, I re-released to fix that issue.
    Also, note that ltdl_openext do not search for versioned shared libraries
    so roboptim-core-dummy-plugin.so is required in the main package.

* Timestamps
  - spec file now use
  ``make install DESTDIR="$RPM_BUILD_ROOT" INSTALL="install -p"''

* About -doc subpackage
  - My bad, documentation subpackage has been fixed (texlive-latex,
    texlive-dvips) are required to compile the documentation properly.

I checked again the rpm with koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1918737
This time it seems everything is alright.

BTW rpmlint do not indicate any error on the rpm except no-documentation for roboptim-core / roboptim-core-devel (which is ok since there is a documentation subpackage).

Thanks for your time and your precious help!

Comment 4 Mamoru TASAKA 2010-01-13 19:50:16 UTC
For 0.5-1:

* Source0
  - Again Source0 in your srpm does not coincide with the tarball
    I could download from the URL written in the spec file:
-------------------------------------------------------------
500600 2010-01-13 22:46 downloaded/roboptim-core-0.5.tar.gz
500604 2010-01-13 22:46 roboptim-core-0.5-1.fc11.src/roboptim-core-0.5.tar.gz
-------------------------------------------------------------

* BR (BuildRequires)
  - For tex related dependency we prefer to use virtual Provides
    name instead of using rpm names directly.
    i.e. Please use "BR: tex(latex), tex(dvips)"

* Directory ownership issue
  - The following directories (here saying the directories themselves)
    are not owned by any packages
-------------------------------------------------------------
%{_libdir}/%{name}/
%{_datadir}/doc/%{name}-%{version}/
-------------------------------------------------------------
     Please check:
     https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Common_Mistakes

* Documents
  - "COPYING" or so must be in main package. Please move
    "AUTHORS COPYING THANKS README NEWS" to the main package.

  ! NOTE
    - As I said in the previous comment 2, if you write
      "%doc AUTHORS COPYING THANKS README NEWS" in the main
      package, this will firstly remove all files under
      %buildroot%_docdir/%name-%version, including installed
      doxygen html files.
      To avoid this (and to fix directory ownership issue
      written above),

      - After "make install ....." line, add
-------------------------------------------------------------
install -cpm 644 AUTHORS COPYING THANKS README NEWS \
	$RPM_BUILD_ROOT%{_docdir}/%{name}-%{version}/
-------------------------------------------------------------
      - And in %files:
-------------------------------------------------------------
%files
%defattr(-,root,root,-)
%dir %{_docdir}/%{name}-%{version}/
%{_docdir}/%{name}-%{version}/[A-Z]*
....
....
%files doc
%defattr(-,root,root,-)
%{_docdir}/%{name}-%{version}/doxygen-html/
-------------------------------------------------------------
        By the way use %{_docdir} instead of %{_datadir}/doc :
        https://fedoraproject.org/wiki/Packaging/RPMMacros

        ! Just a note
          - All files/directories/etc under /usr/share/doc are
            automatically marked as %doc.

Comment 5 Thomas Moulard 2010-01-14 10:09:48 UTC
Ok, I have updated the spec file accordingly:
Spec URL: http://homepages.laas.fr/~tmoulard/fedora/roboptim-core-0.5-2.spec
SRPM URL:
http://homepages.laas.fr/~tmoulard/fedora/roboptim-core-0.5-2.fc11.src.rpm

* Source0
  My bad. Should be ok this time.

* BR (BuildRequires)
  - Use virtual dependencies latex/dvips.

* Directory ownership issue
  - Fixed rules to solve un-owned directory problem

* Documents
  - Fixed rules as indicated in previous comment.

Comment 6 Mamoru TASAKA 2010-01-15 16:17:01 UTC
Okay, now:

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

Comment 7 Mamoru TASAKA 2010-01-28 15:35:19 UTC
ping?

Comment 8 Thomas Moulard 2010-01-28 16:46:36 UTC
Sorry, I have been lacking time to continue the review process this week, but it will be better next week.

Comment 9 Thomas Moulard 2010-02-01 17:25:57 UTC
Pre-review available here: https://bugzilla.redhat.com/show_bug.cgi?id=560457

Comment 10 Mamoru TASAKA 2010-02-01 19:04:13 UTC
(In reply to comment #9)
> Pre-review available here: https://bugzilla.redhat.com/show_bug.cgi?id=560457    

Well, this review request contains a strange dependency loop
which cannot be solved currently, however your pre-review itself
seems good for initial comments.

--------------------------------------------------------------
   This package (roboptim-core) is APPROVED by mtasaka
--------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
After you request for sponsorship a mail will be sent to sponsor 
members automatically (which is invisible for you) which notifies 
that you need a sponsor. After that, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 11/12, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Comment 11 Thomas Moulard 2010-02-01 19:50:54 UTC
Thanks! My FAS login is ``thomasmoulard''.

Is there anything more to do to request for sponsorship (like sending a mail to a mailing-list or something), the wiki is not clear on that point?

Comment 12 Mamoru TASAKA 2010-02-01 20:00:17 UTC
Now I am sponsoring you. Please follow "Join" wiki again.

Comment 13 Thomas Moulard 2010-02-01 21:39:19 UTC
New Package CVS Request
=======================
Package Name: roboptim-core
Short Description: The RobOptim core C++ library
Owners: thomasmoulard
Branches: F-11 F-12
InitialCC:

Comment 14 Kevin Fenzi 2010-02-01 22:48:22 UTC
CVS done (by process-cvs-requests.py).

Comment 15 Fedora Update System 2010-02-02 13:47:56 UTC
roboptim-core-0.5-2.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/roboptim-core-0.5-2.fc12

Comment 16 Fedora Update System 2010-02-02 14:21:20 UTC
roboptim-core-0.5-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/roboptim-core-0.5-2.fc11

Comment 17 Mamoru TASAKA 2010-02-02 17:04:27 UTC
Closing.

Comment 18 Fedora Update System 2010-02-23 05:14:43 UTC
roboptim-core-0.5-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 19 Fedora Update System 2010-02-23 05:24:49 UTC
roboptim-core-0.5-2.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.


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