Fedora Merge Review: lftp http://cvs.fedora.redhat.com/viewcvs/devel/lftp/ Initial Owner: mbarabas
Pending issues from ticket https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=211483 * Requirements and build requirement: comment #7 * Rpath problems: comments #9 and # 10
* RPM name is OK * Source lftp-3.5.1.tar.bz2 is the same as upstream * Builds fine in mock * File list looks OK Needs work: * BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) (wiki: PackagingGuidelines#BuildRoot) * Missing SMP flags. If it doesn't build with it, please add a comment (wiki: PackagingGuidelines#parallelmake) * Spec file: some paths are not replaced with RPM macros (wiki: QAChecklist item 7) * BuildRequires: gettext is missing (required to build the translations) * The %makeinstall macro should not be used (wiki: PackagingGuidelines#MakeInstall) * rpmlint is not silent, see below Minor: * Duplicate BuildRequires: autoconf (by automake), automake (by libtool) * Please honor $RPM_OPT_FLAGS rpmlint of lftp-3.5.1-2.fc6.i386.rpm:E: lftp binary-or-shlib-defines-rpath /usr/lib/liblftp-tasks.so. 0.0.0 ['/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.1/proto-ftp.so ['/usr/lib/lftp/3.5.1', '/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.1/proto-fish.so ['/usr/lib/lftp/3.5.1', '/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.1/proto-http.so ['/usr/lib/lftp/3.5.1', '/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.1/liblftp-network.so ['/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.1/proto-sftp.so ['/usr/lib/lftp/3.5.1', '/usr/lib'] W: lftp conffile-without-noreplace-flag /etc/lftp.conf W: lftp devel-file-in-non-devel-package /usr/lib/liblftp-jobs.so W: lftp devel-file-in-non-devel-package /usr/lib/liblftp-tasks.so E: lftp library-without-ldconfig-postin /usr/lib/liblftp-jobs.so.0.0.0 E: lftp library-without-ldconfig-postun /usr/lib/liblftp-jobs.so.0.0.0 E: lftp library-without-ldconfig-postin /usr/lib/liblftp-tasks.so.0.0.0 E: lftp library-without-ldconfig-postun /usr/lib/liblftp-tasks.so.0.0.0 [ruben@odin lftp]$ rpmlint lftp-3.5.1-2.fc6.src.rpm W: lftp mixed-use-of-spaces-and-tabs (spaces: line 11, tab: line 1) W: lftp patch-not-applied Patch2: lftp-3.4.1-dont_core.patch W: lftp patch-not-applied Patch181694: lftp-3.4.2-fix-redirect-coredump.patch W: lftp patch-not-applied Patch173276: lftp-3.3.5-bz173276.patch
Ruben, Why did you review lftp 3.5.1 from FC-6? Should you have reviewed lftp 3.5.9 available in rawhide (the patches have already been removed) ? jpo
Ah, you're right, my mistake. That leaves us with: * RPM name is OK * Source lftp-3.5.9.tar.bz2 is the same as upstream * This is the latest version * Builds fine in mock * File list looks OK Needs work: * BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) (wiki: PackagingGuidelines#BuildRoot) * Missing SMP flags. If it doesn't build with it, please add a comment (wiki: PackagingGuidelines#parallelmake) * Spec file: some paths are not replaced with RPM macros (wiki: QAChecklist item 7) * BuildRequires: gettext is missing (required to build the translations) * The %makeinstall macro should not be used (wiki: PackagingGuidelines#MakeInstall) * Preserve timestamps when installing files (wiki: PackagingGuidelines#Timestamps Minor: * Duplicate BuildRequires: autoconf (by automake), automake (by libtool) Rpmlint output: Source RPM: W: lftp mixed-use-of-spaces-and-tabs (spaces: line 11, tab: line 1) rpmlint of lftp: W: lftp incoherent-version-in-changelog 3.5.9 3.5.9-1.fc6 E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.9/proto-ftp.so ['/usr/lib/lftp/3.5.9', '/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.9/proto-fish.so ['/usr/lib/lftp/3.5.9', '/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.9/proto-http.so ['/usr/lib/lftp/3.5.9', '/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.9/liblftp-network.so ['/usr/lib'] E: lftp binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.9/proto-sftp.so ['/usr/lib/lftp/3.5.9', '/usr/lib'] W: lftp conffile-without-noreplace-flag /etc/lftp.conf W: lftp devel-file-in-non-devel-package /usr/lib/liblftp-jobs.so W: lftp devel-file-in-non-devel-package /usr/lib/liblftp-tasks.so E: lftp library-without-ldconfig-postin /usr/lib/liblftp-jobs.so.0.0.0 E: lftp library-without-ldconfig-postun /usr/lib/liblftp-jobs.so.0.0.0 E: lftp library-without-ldconfig-postin /usr/lib/liblftp-tasks.so.0.0.0 E: lftp library-without-ldconfig-postun /usr/lib/liblftp-tasks.so.0.0.0
Instead of "Requires: perl-String-CRC32" prefer: Requires: perl(String::CRC32)
(In reply to comment #5) > Instead of "Requires: perl-String-CRC32" prefer: > > Requires: perl(String::CRC32) A even better solution would be to drop this explicit requirement as rpmbuild has no problem in detecting and adding it to the requirements list. jpo
mbarabas, can you please create a new spec based on the comments above?
Upstream has released a new lftp version (3.5.10). * Short changelog (http://lftp.yar.ru/news.html) Version 3.5.10 - 2007-03-26 * fixed core dump when doing ls on file: connection. * fixed core dump when doing pget to write-protected directory.
From https://www.redhat.com/archives/fedora-cvs-commits/2007-April/msg00189.html : +%post +/sbin/chkconfig --add lftp +/sbin/ldconfig + +%postun +if [ "$1" = 0 ]; then +/sbin/chkconfig --del lftp +/sbin/ldconfig +fi +exit 0 + Why the call the to /sbin/chkconfig ? There are no servers in the lftp packagew? Cut-paste error?
Error message produced by the previous script during a yum update transaction (rawhide@2007-04-05): ... Finished Transaction Test Transaction Test Succeeded Running Transaction ... Updating : lftp ####################### [ 3/10] error reading information on service lftp: No such file or directory Updating : yum-updatesd ####################### [ 4/10] ...
> Running Transaction > ... > Updating : lftp ####################### [ 3/10] > error reading information on service lftp: No such file or directory > Updating : yum-updatesd ####################### [ 4/10] Yeah , coming from the calls to chkconfig in scripts. BTW: i guess automake and autoconf should be removed as buildreq? More strange things: (rpath issue?): # ldd /usr/lib/lftp/3.5.10/proto-file.so |grep ncur libncurses.so.5 => /lib/libncurses.so.5 (0x00fad000) # /usr/lib/lftp/3.5.10/proto-http.so /usr/lib/lftp/3.5.10/proto-http.so: error while loading shared libraries: /usr/lib/libncurses.so.5: file too short All lftp libs should be linked against libncurses in /lib, however because of rpath libncurses in /usr/lib is used by some libs: proto-sftp.so, proto-ftp.so, proto-fish.so liblftp-network.so proto-http.so Please read: http://fedoraproject.org/wiki/Packaging/Guidelines#head-a1dfb5f46bf4098841e31a75d833e6e1b3e72544
Also, there really shouldn't be a -devel subpackage for lftp. It does not provide an API for external things to use.
I'd like to repeat comment #9: lftp provides no libraries in the standard paths and doesn't add a new path in /etc/ld.so.conf.d. Therefore ldconfig in post/postun isn't required lftp isn't a daemon, running chkconfig in post/postun isn't required.
Ping?
Version: 3.5.14-2 /sbin/ldconfig -p | grep lftp liblftp-tasks.so.0 (libc6,x86-64) => /usr/lib64/liblftp-tasks.so.0 liblftp-jobs.so.0 (libc6,x86-64) => /usr/lib64/liblftp-jobs.so.0 There are libraries in the standard paths: /usr/lib64/liblftp-*.so.0.0.0 and therefore rpmlint yells, when there are no ldconfig postin(un) calls. chkconfig is removed.
Hi Maros, Are you planning on rebasing to 3.6.1? There are still a few things left to do: Source RPM: lftp.src: W: mixed-use-of-spaces-and-tabs (spaces: line 181, tab: line 1) lftp.src: W: invalid-license GPL I think this should be GPLv2+ Binary RPM: [ruben@odin reports]$ rpmlint lftp/lftp-3.5.14-2.fc9.i386.rpm lftp.i386: W: file-not-utf8 /usr/share/doc/lftp-3.5.14/NEWS lftp.i386: W: invalid-license GPL lftp.i386: E: binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.14/proto-ftp.so ['/usr/lib/lftp/3.5.14', '/usr/lib'] lftp.i386: E: binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.14/proto-fish.so ['/usr/lib/lftp/3.5.14', '/usr/lib'] lftp.i386: E: binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.14/proto-http.so ['/usr/lib/lftp/3.5.14', '/usr/lib'] lftp.i386: E: binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.14/liblftp-network.so ['/usr/lib'] lftp.i386: E: binary-or-shlib-defines-rpath /usr/lib/lftp/3.5.14/proto-sftp.so ['/usr/lib/lftp/3.5.14', '/usr/lib']
Hi Ruben, I was planning, but I'm no longer the maintainer of lftp. I'm sure Martin will take care of it. Thanks.
Hi, I rebased to 3.6.1 and have committed changes to CVS, rpmlint now finds only this: lftp.i686: W: conffile-without-noreplace-flag /etc/ld.so.conf.d/lftp-i386.conf This is okay, as it is a configuration file, but not to be changed by the user and it can be changed in the future. Please review, thanks!
Would be more easy if you could upload a srpm, well, well. spec seems fine, however it's crashing, try this: $ gdb (gdb) exec-file /usr/bin/lftp (gdb) run http://ftp.uninett.no/ (gdb) ls I get: cd ok, cwd=/ lftp ftp.uninett.no:/> ls `ls' at 0 [Waiting for response...] Program received signal SIGSEGV, Segmentation fault. parse_html ( buf=0x96f1987 "<META content=\"\" name=\"redaksjon.vev\">\n<META content=\"\" name=\"dato.verifisert\">\n<META content=\"\" name=\"forfatter.navn\">\n<META content=\"\" name=\"forfatter.epost\">\n<META content=\"\" name=\"forfatter.vev\">\n"..., buf_len=1645, eof=false, list=@0x96cd1b8, set=0x0, all_links=0x96cd1d0, prefix=0x96cd078, base_href=0x96cd1fc, lsopt=0x96cd200, color=1) at HttpDir.cc:732 732 while(*scan && is_ascii_digit(*scan)) Current language: auto; currently c++ (gdb) bt #0 parse_html ( buf=0x96f1987 "<META content=\"\" name=\"redaksjon.vev\">\n<META content=\"\" name=\"dato.verifisert\">\n<META content=\"\" name=\"forfatter.navn\">\n<META content=\"\" name=\"forfatter.epost\">\n<META content=\"\" name=\"forfatter.vev\">\n"..., buf_len=1645, eof=false, list=@0x96cd1b8, set=0x0, all_links=0x96cd1d0, prefix=0x96cd078, base_href=0x96cd1fc, lsopt=0x96cd200, color=1) at HttpDir.cc:732 #1 0x004fb00a in HttpDirList::Do (this=0x96cd188) at HttpDir.cc:1239 #2 0x0018b7a9 in SMTask::Schedule () at SMTask.cc:226 #3 0x001415af in Job::WaitDone (this=0x96a9ea0) at Job.cc:522 #4 0x0804c37e in ?? () #5 0x096a9ea0 in ?? () #6 0x096b91f0 in ?? () #7 0x00000000 in ?? ()
This was a bug in 3.6.1, this srpm contains fix, please test, thanks. http://people.redhat.com/mnagy/lftp-3.6.1-1.fc9.src.rpm
Yes, works fine now, thanks! rpmlint gives some warnings about "unused-direct-shlib-dependency", I leave those to the reviewer (Ruben).
That's strange, I only see the one warning about conffile I mentioned earlier.
I guess you must install the package and then run $ rpmlint lftp to see these warnings.
I've just rebuild the latest version in mock, and only see one warning: [ruben@odin ~]$ rpmlint reports/lftp/lftp-3.6.1-1.fc9.i386.rpm lftp.i386: W: conffile-without-noreplace-flag /etc/ld.so.conf.d/lftp-i386.conf But that's ok, and this package is approved. Thanks Martin and Terje for your time.
Are you sure that files in %{_libdir}/lftp/%{version}/ are not dlopened and that /etc/ld.so.conf.d/%{name}-%{_arch}.conf is useful? I think that the DEBUG stuff should be removed. The LDFLAGS="-L`pwd`/src/.libs $LDFLAGS"; certainly needs an explanation. I tried without and it built fine. Also why use the system libtool? Is it to avoid the rpath? If it doesn't work is it useful to keep it? And also is it really useful to use system libtool for the install? I also suggest doing iconv -f ISO88591 -t UTF8 NEWS -o NEWS.tmp touch -r NEWS NEWS.tmp mv -f NEWS.tmp NEWS to keep the NEWS file timestamp. The postun is wrong, it should be run each time.
(In reply to comment #25) I have made yet another srpm: http://people.redhat.com/mnagy/lftp-3.6.1-1.fc9.src.rpm > I think that the DEBUG stuff should be removed. I see no reason for it to be there, so done. > The > LDFLAGS="-L`pwd`/src/.libs $LDFLAGS"; > certainly needs an explanation. I tried without and it built fine. This is a bit odd, it's been there for a longer time than it is in CVS, so no idea who put it there. Anyway, I removed it and it built fine for me too. > Also why use the system libtool? Is it to avoid the rpath? If it > doesn't work is it useful to keep it? And also is it really > useful to use system libtool for the install? Hmm, good idea, I'll ask the previous maintainer if he remembers (this was done 6 years ago) why are we using system libtool. I fixed this in srpm. > I also suggest doing > iconv -f ISO88591 -t UTF8 NEWS -o NEWS.tmp > touch -r NEWS NEWS.tmp > mv -f NEWS.tmp NEWS > to keep the NEWS file timestamp. My fault, sorry about that, fixed. > The postun is wrong, it should be run each time. Fixed as well. I also changed the patch which is better now. Patrice: Thanks for spotting these. Ruben: I still haven't commited the changes into cvs, could you please review if these changes are good?
The latest version seems to be 3.6.3, although there was no announcement.
Hm, yes, a bit weird that it was not announced. I rebased anyway, thanks.
lftp is in good shape now, however I would use the %{_prefix} macro here: %configure --with-modules --disable-static --with-openssl=/usr --with-debug --disable-rpath like this: %configure --with-modules --disable-static --with-openssl=%{_prefix} --with-debug --disable-rpath
Will do, thanks.
All my concerns are fixed, now, thanks.