[Bug 478470] Review Request: mrpt - The Mobile Robot Programming Toolkit (MRPT)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=478470





--- Comment #9 from Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx>  2009-01-10 12:57:38 EDT ---
Created an attachment (id=328632)
 --> (https://bugzilla.redhat.com/attachment.cgi?id=328632)
rpmlint for binary rpms

Well, for 0.6.4-2:

** Description etc
* License
  - License tag should be "GPLv3+".

* Source
  - Source tarball in your srpm differs from what I could
    download from the Source URL in your spec:
------------------------------------------------
9773759 2009-01-09 00:43 mrpt-0.6.4.tar.gz
9773812 2009-01-09 00:51 mrpt-0.6.4-2.fc10.src/mrpt-0.6.4.tar.gz
------------------------------------------------

* BuildRequires:
  - build.log says:
------------------------------------------------
   107  -- Looking for doxygen...
   108  -- Looking for doxygen... - found /usr/bin/doxygen
   109  -- Looking for dot tool...
   110  -- Looking for dot tool... - NOT found
------------------------------------------------
    Perhaps "BuildRequires: graphviz" is missing.

** %prep -> %install
* Build failure
  - Currently your srpm does not build by 4 reasons.
    1 %check fails as
       http://koji.fedoraproject.org/koji/taskinfo?taskID=1042528
       This is because at "make test" this test needs
       system-widely installed libmrpt-core.so.0.6, but on mockbuild
       apparently this library is not yet installed system-widely.

    2 The rebuilt libraries like libmrpt-core.so.0.6 are installed
      under %buildroot/usr/lib, not %buildroot%_libdir even with
      64 bits architecture

    3 CMakeLists.txt adds "-mtune=native" to CPPFLAGS, which
      is not recognized on ppc (maybe also on ppc64)
      http://koji.fedoraproject.org/koji/taskinfo?taskID=1042483
      Also, CMakeLists.txt adds "-O3" compilation option, however
      Fedora default optimation level is "-O2" so this should
      be removed.

    4 %files lists are wrong. Some files are installed
      under %{_datadir}/mrpt/datasets/ but no files are installed
      under %{_datadir}/mrpt/config_files/datasets/

    Possible solution:
    - For 2,3: at %prep:
-------------------------------------------
%prep
%setup -q
sed -i.misc \
 -e 's|-O3|-O2|' \
 -e 's|-mtune=native||' \
 -e '/DESTINATION/s|PREFIX}lib|PREFIX}lib\${LIB_SUFFIX}|' \
 $(find . -name CMakeLists.txt)
-------------------------------------------

    - For 1: at %check:
-------------------------------------------
%check
export LD_LIBRARY_PATH=$(pwd)/lib
make test VERBOSE=1 ARGS="-VV"
-------------------------------------------

    - For 4: Check %files list

** %files, etc
* Package splitting
  - First of all, please explain why you want to split each
    library into different subpackges.

* desktop files
  - desktop-file-install or so must be executed against
    desktop files to be installed.
   
https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage

* Scriptlets
  - Some files has MimeType, so please refer to:
    https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database

  - This package installs XML files under %_datadir/mime/packages:
    https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#mimeinfo

* Directory ownership
  - The directory %{_datadir}/mrpt is owned by 2 packages. Please
    change this so that only one package owns this directory
    ( I guess having this directory owned by -core package is
      an alternative solution)
  - %{_datadir}/doc/mrpt-doc/ is not owned by any packages.

* pkgconfig
  - pkgconfig/libmrpt.pc.in contains:
--------------------------------------------
     3  libdir=${exec_prefix}/lib
--------------------------------------------
    This is expanded as /usr/lib, even on 64 bits architecture,
    while this should be expanded as /usr/lib64.

  - Also installed libmrpt.pc contains:
--------------------------------------------
     9  Libs: -L${libdir}   -ldc1394 -lGL -lGLU -lglut -lftdi -lusb -l3ds -lz
-ljpeg -lrt -pthread -lmrpt-ann /usr/lib64/libboost_program_options-mt.so
-pthread -lwx_baseu-2.8 -lwx_gtk2u_core-2.8 -lwx_gtk2u_gl-2.8
-lwx_gtk2u_adv-2.8 -lwx_gtk2u_aui-2.8 -lmrpt-core -lmrpt-aria
--------------------------------------------
    Please check if these explicit linkage lists are _really_
    needed. I guess almost all of these can be removed
    if the installed libraries are already properly linked.

    Also, note that each "-lfoo" entry adds each corresponding
    package dependency to mrpt-devel package. For example
    "-lglut" means -devel package should have 
    "Requires: freeglut-devel", while I don't think this is needed.

* Header files dependency
  - Please check if proper dependency packages are installed
    with -devel packages to satisfy "include" macro dependencies
    in header files.
    - For example, gui/WxUtils.h contains
---------------------------------------------
    40  #include <wx/sizer.h>
    41  #include <wx/statbmp.h>
    42  #include <wx/menu.h>
---------------------------------------------
      This means that mrpt-devel package should have "Requires: wxGTK-devel"
      (not wxGTK).

* Duplicate files
  - Please check if all subpackages need "doc COPYING" or so.
    Also "%doc foo" creates its own document directory (named
    /usr/share/doc/mrpt-foo-%{version}) for each subpackage.

    Especially -doc subpackage has two document directories
    (/usr/share/doc/mrpt-doc and /usr/share/doc/mrpt-doc-%{version})
    I suggest these directories should be unified.

* rpmlint issue
  rpmlint output attached.
  - Please change non-UTF8 files to UTF-8 encoding
  - Usually libraries themselves should not call exit()
    (see $ rpmlint -I shared-lib-calls-exit)
  - Many files has Windows-like line terminators.
    Please fix these by "sed -i -e 's|\r||g'" or dos2unix

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]