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 #14 from Jose Luis <joseluisblancoc@xxxxxxxxx> 2009-01-18 14:41:27 EDT --- Well, here is the new revision: SPEC: http://babel.isa.uma.es/mrpt/src-repo/rpm/mrpt.spec SRPM: http://babel.isa.uma.es/mrpt/src-repo/rpm/mrpt-0.6.5-0.1.20090118svn746.fc10.src.rpm I finally decided to use the "snapshot versions format" so I can integrate all the fixes in upstream. btw, if I got it right, a package, say "foo-1.2.3-0.1.20090118svn" MUST have an associated tarball "foo-1.2.3.tar.gz", without any indication of the snapshot?? I'd prefer more descriptive names containing the snapshot part, so please confirm me if that is the preferred naming or I can create tarballs with the svn number prefix. Next are the answers to your points: (In reply to comment #9) > ** Description etc > * License > - License tag should be "GPLv3+". Done. > * 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. This is not an issue. "dot" is actually not used. In fact the CMake command is "look for doxygen", but it internally also looks for dot... > ** %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. Fixed with your "LD_LIBRARY_PATH..." solution. > 2 The rebuilt libraries like libmrpt-core.so.0.6 are installed > under %buildroot/usr/lib, not %buildroot%_libdir even with > 64 bits architecture Fixed in upstream CMakeLists.txt's. > 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. Fixed through a cmake argument "-DCMAKE_MRPT_IS_RPM_PACKAGE=1" which internally disables "-mtune" and "O3". > 4 %files lists are wrong. Some files are installed > under %{_datadir}/mrpt/datasets/ but no files are installed > under %{_datadir}/mrpt/config_files/datasets/ Solved. > ** %files, etc > * Package splitting > - First of all, please explain why you want to split each > library into different subpackges. Comments added to specfile. > * desktop files > - desktop-file-install or so must be executed against > desktop files to be installed. Done, at %install. > > * 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 Both done at %post/postun of mrpt-apps. > * 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) The -core package now owns that directory. > - %{_datadir}/doc/mrpt-doc/ is not owned by any packages. It's now of the package "mrpt-doc". > * 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. Fixed using the LIB_SUFFIX variable in CMake (not tested...). > - 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 > -------------------------------------------- You're right! Most of the -lxxx were not required. > * 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). Ok, done. > * 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. I tried removing COPYING and README, but rpmlint complains about packages without documentation, so I finally left them. > 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. Done, /usr/share/doc/mrpt-doc is the only directory now. > * 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 All fixed. -- 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