Bug 505917
Summary: | Review Request: libpuzzle - Library to quickly find visually similar images | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Andrew Colin Kissa <andrew> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | fedora-package-review, notting, steve.traylen |
Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 0.11-5.el5 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-06-23 16:27:57 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: |
Description
Andrew Colin Kissa
2009-06-14 17:22:27 UTC
Hi Andrew, I am not able to sponsor your package but I can hopefully offer some useful help. For me the compile won't pass an rpath check as shown below. rpmbuild -ba libpuzzle-0.11-1.fc11.src.rpm runs a check since I have in my ~/.rpmmacros %__arch_install_post /usr/lib/rpm/check-rpaths /usr/lib/rpm/check-buildroot as put there by "rpmdev-setuptree" Steve + /usr/lib/rpm/check-rpaths /usr/lib/rpm/check-buildroot ******************************************************************************* * * WARNING: 'check-rpaths' detected a broken RPATH and will cause 'rpmbuild' * to fail. To ignore these errors, you can set the '$QA_RPATHS' * environment variable which is a bitmask allowing the values * below. The current value of QA_RPATHS is 0x0000. * * 0x0001 ... standard RPATHs (e.g. /usr/lib); such RPATHs are a minor * issue but are introducing redundant searchpaths without * providing a benefit. They can also cause errors in multilib * environments. * 0x0002 ... invalid RPATHs; these are RPATHs which are neither absolute * nor relative filenames and can therefore be a SECURITY risk * 0x0004 ... insecure RPATHs; these are relative RPATHs which are a * SECURITY risk * 0x0008 ... the special '$ORIGIN' RPATHs are appearing after other * RPATHs; this is just a minor issue but usually unwanted * 0x0010 ... the RPATH is empty; there is no reason for such RPATHs * and they cause unneeded work while loading libraries * 0x0020 ... an RPATH references '..' of an absolute path; this will break * the functionality when the path before '..' is a symlink * * * Examples: * - to ignore standard and empty RPATHs, execute 'rpmbuild' like * $ QA_RPATHS=$[ 0x0001|0x0010 ] rpmbuild my-package.src.rpm * - to check existing files, set $RPM_BUILD_ROOT and execute check-rpaths like * $ RPM_BUILD_ROOT=<top-dir> /usr/lib/rpm/check-rpaths * ******************************************************************************* ERROR 0001: file '/usr/bin/puzzle-diff' contains a standard rpath '/usr/lib64' in [/usr/lib64] error: Bad exit status from /var/tmp/rpm-tmp.O9FHYw (%install) Hi Steve, Thanks for taking time to review my rpm, its very weird that happens when you build, i have tested using koji as well as locally and could not reproduce your error. I have how ever added a fix to the spec, please test again and see if it builds. Thanks again. Andrew Hi, It builds now. There some rpmlint errors that need fixing. $ rpmlint libpuzzle-0.11-1.fc11.x86_64.rpm libpuzzle.x86_64: E: standard-dir-owned-by-package /usr/share/man/man3 libpuzzle.x86_64: E: standard-dir-owned-by-package /usr/share/man/man8 libpuzzle.x86_64: E: zero-length /usr/share/doc/libpuzzle-0.11/ChangeLog libpuzzle.x86_64: E: zero-length /usr/share/doc/libpuzzle-0.11/NEWS see $ rpmlint -I standard-dir-owned-by-package to get details on the individual messages. In particular for the first one see the first paragraph of: https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership There are few trivial ones as well in the php package. $ rpmlint php-libpuzzle-0.11-1.fc11.x86_64.rpm php-libpuzzle.x86_64: E: zero-length /usr/share/doc/php-libpuzzle-0.11/EXPERIMENTAL php-libpuzzle.x86_64: W: file-not-utf8 /usr/share/doc/php-libpuzzle-0.11/similar.php php-libpuzzle.x86_64: W: non-conffile-in-etc /etc/php.d/libpuzzle.ini Also I don't think you need or shoudl have the Requires: gd since this is picked up automatically anyway. Check for php bits as well but am less sure. See: https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires Hi Steve, Fixed the issues raised, the rpmlint now comes out clean on my side. If you can give it a run again. I would appreciate it if you can critic my other submission https://bugzilla.redhat.com/show_bug.cgi?id=505848 Thanks. $ rpmlint ../RPMS/x86_64/php-libpuzzle-0.11-1.fc11.x86_64.rpm php-libpuzzle.x86_64: W: file-not-utf8 /usr/share/doc/php-libpuzzle-0.11/similar.php 1 packages and 0 specfiles checked; 0 errors, 1 warnings. still present. Agreed the libpuzzle-* ones pass just fine. Reading the INSTALL file since there is "make check" is sensible or possible to call this after the build? Maybe the .ini file should be created in the %build stage before being installed in the %install stage. Genrally manipulations of the BUILD area should be in build. i.e %{__rm} -rf php %{__mv} php-plain php should move up to %build if possible? Will look at the php bit later. I have made the changes rpmlint is now clean on all the 3 rpm's. Would you post the URLs of your latest spec/srpm? Note that please change the release number of your spec/srpm every time you modify your spec file to avoid confusion. The spec and the SRPM are here. http://www.topdog-software.com/oss/SRPMS/fedora/libpuzzle/libpuzzle-0.11-2.fc11.src.rpm http://www.topdog-software.com/oss/SRPMS/fedora/libpuzzle/libpuzzle.spec Some notes: * chrpath - We usually recommend to avoid to use chrpath as the means to delete rpath (and to add "chrpath" as BR(BuildRequires)) and instead recommend to try to remove rpath by modifying configure/libtool. For this packge: ----------------------------------------------------------------- %build %{__cat} <<'EOF' >libpuzzle.ini extension=libpuzzle.so EOF %configure --disable-static sed -i.rpath -e 's|^\(hardcode_libdir_flag_spec=\).*|\1""|' libtool sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool make %{?_smp_mflags} ....... ....... cd php/libpuzzle phpize %ifarch x86_64 ppc64 LDFLAGS="$LDFLAGS -L%{_builddir}/prelim2/usr/lib64"; export LDFLAGS %endif sed -i.rpath -e 's|\$ld_runpath_switch\$ai_p||g' configure %configure --with-libpuzzle=%{_builddir}/prelim2/usr sed -i.rpath -e 's|^\(hardcode_libdir_flag_spec=\).*|\1""|' libtool sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool make %{?_smp_mflags} ........ ........ ----------------------------------------------------------------- will remove rpath: http://koji.fedoraproject.org/koji/taskinfo?taskID=1426380 Ref: https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath * Requires - php-%{name} should have "Requires: php-common" for directory ownership issue * Usage of %_builddir ----------------------------------------------------------------- make DESTDIR=%{_builddir}/prelim2 install ----------------------------------------------------------------- - This is problematic is some other srpm use prelim2 directory. Please move this temporary directory to under %_builddir/%{name}-%{version} (and using "rm -rf %{_builddir}/prelim2" at clean is not desired) * Timestamps - Add the option 'INSTALL="install -p"' to 'make install' to keep timestamps on installed files. This method usually works for Makefiles generated by recent autotools. * Documents - There is no need to include the same document files to %doc of both libpuzzle and -devel packages. - Usually section 3 man pages are to explain functions, api or so and should belong to -devel subpackage. * %changelog - It is useful when importing this package into Fedora CVS if you put one line between each %changelog entry like: ------------------------------------------------------------------ * Thu Jun 18 2009 Andrew Colin Kissa <andrew.net> - 0.11-2 - Fix rpmlint issues * Sun May 14 2009 Andrew Colin Kissa <andrew.net> - 0.11-1 - Initial release ------------------------------------------------------------------ Hi Thanks for the review, I have addressed all the points that you raised. The updated spec and source rpm can be found at. http://www.topdog-software.com/oss/SRPMS/fedora/libpuzzle/libpuzzle.spec http://www.topdog-software.com/oss/SRPMS/fedora/libpuzzle/libpuzzle-0.11-3.fc11.src.rpm For -3: * Macros consistency - Well, if you want to use %{__cat} or %{__sed} style, please use %{__make}, %{__rm} for consistency. * Timestamps - Would you consider to use --------------------------------------------------------------- make install DESTDIR=%{buildroot} INSTALL="install -p" --------------------------------------------------------------- for example to keep timestamps on installed files? * Misc issue --------------------------------------------------------------- %files devel %defattr(-,root,root,-) %doc --------------------------------------------------------------- - Empty %doc line is unneeded. * Macros consistency - Fixed (__command style used) * Timestamps - Fixed (install -p used) * Misc - Fixed (doc line removed from devel package) Updated Spec: http://www.topdog-software.com/oss/SRPMS/fedora/libpuzzle/libpuzzle.spec Updated SRPM: http://www.topdog-software.com/oss/SRPMS/fedora/libpuzzle/libpuzzle-0.11-4.fc11.src.rpm Okay. + This package itself is now okay + You have other review requests, which seem fine from very quick glance at them ------------------------------------------------------------------- This package (libpuzzle) is APPROVED by mtasaka ------------------------------------------------------------------- Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Install the Client Tools (Koji)". Now I am sponsoring you. If you want to import this package into Fedora 10/11, 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. Removing NEEDSPONSOR. New Package CVS Request ======================= Package Name: libpuzzle Short Description: Library to quickly find visually similar images (gif, png,jpg) Owners: topdog Branches: F-10 F-11 InitialCC: mtasaka cvs done. libpuzzle-0.11-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/libpuzzle-0.11-4.fc10 libpuzzle-0.11-4.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/libpuzzle-0.11-4.fc11 Now closing. Package Name: libpuzzle New Branches: EL-5 Owners: topdog cvs done. libpuzzle-0.11-5.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/libpuzzle-0.11-5.el5 libpuzzle-0.11-4.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. libpuzzle-0.11-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. libpuzzle-0.11-5.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report. |