Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 192606 - Review Request: yafc: yet another ftp client
Summary: Review Request: yafc: yet another ftp client
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-21 21:11 UTC by Chris Petersen
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-13 06:21:47 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)
Patch for compiling with krb5 (441 bytes, patch)
2006-07-09 11:21 UTC, Gérard Milmeister
no flags Details | Diff

Description Chris Petersen 2006-05-21 21:11:50 UTC
Spec URL: http://rpm.forevermore.net/yafc/yafc.spec
SRPM URL: http://rpm.forevermore.net/yafc/yafc-1.1.1-1.src.rpm
Description:

yafc is Yet Another FTP Client.  It is an interactive interface to the FTP
and SFTP protocols.

FEATURES
  - cached directory listings
  - uses readline (tab completion, emacs/vi editing keys, history file, etc.)
  - extensive tab completion
  - multiple connections open
  - aliases
  - colored ls (i.e., ls --color, uses $LS_COLORS like GNU ls)
  - autologin and bookmarks
  - recursive get/put/rm/ls
  - nohup mode get and put
  - tagging (queueing) of files for later transferring
  - automagically enters nohup-mode when SIGHUP received (in get and put)
  - redirection to local command or file ('>', '>>' and '|')
  - proxy support
  - more...

Comment 1 Gérard Milmeister 2006-06-05 13:10:08 UTC
* Probably better summary: "Yet Another FTP/SFTP Client"
* Better to take the description from http://sourceforge.net/projects/yafc:
  "Yafc is an OpenSource console mode FTP client. It has support for Kerberos 4/5  
   authentication and sftp (ssh2). Other features include tab completion,
   directory cache, powerful aliases, recursive file commands and bookmarks with
   autologin."
* fails to build in mock:
  RPM build errors:
    File not found: /var/tmp/yafc-1.1.1-1-root-mockbuild/usr/share/info/dir

  The dir file has not been created. Instead of "%exclude", simply
  use "rm -f" in the %install section, this works even if dir has not
  been created.
* Normally "install-info --delete" goes into %postun instead of %preun
* You don't need "Req: readline", rpmbuild takes care of this
* Simply removing the static keyworkd in gssapi.c makes
  it compile with kerberos enabled
* I don't think, that it is a good idea to enable color always. This
  does only work in ANSI terminals, it is a mess in dumb terminal, for
  example with emacs. In fact the doc states that, just as the real ls,
  you have to add the --color option to ls to enable color.

  


Comment 2 Chris Petersen 2006-07-03 20:56:48 UTC
I've fixed most of the concerns (haven't posted yet since I want to address the
other concerns, first).

I don't know what you mean about the fix for kerberos.  I think you meant to
type "static keywords" but don't know which ones you're talking about.  I don't
know much about compiling things, so I'm having a hard time figuring out what
you mean.  I'm also asking upstream, in hope that they can just fix the problem
so I don't need to maintain a patch.

Comment 3 Gérard Milmeister 2006-07-09 11:21:10 UTC
Created attachment 132127 [details]
Patch for compiling with krb5

Here is the patch for compiling with krb5.
You need of course add krb5-devel to the BR.

Comment 4 Chris Petersen 2006-07-23 22:24:29 UTC
spec:  http://rpm.forevermore.net/yafc/yafc.spec
srpm:  http://rpm.forevermore.net/yafc/yafc-1.1.1-3.src.rpm

ok, patch, etc applied.  Builds in mock, too.

Comment 5 Gérard Milmeister 2006-07-25 19:13:05 UTC
rpmlint result:
E: yafc zero-length /usr/share/doc/yafc-1.1.1/doc/yafc.info

The package does indeed contain an empty compressed file yafc.info.gz.

Comment 6 Ralf Corsepius 2006-07-26 05:46:20 UTC
(In reply to comment #5)
> rpmlint result:
> E: yafc zero-length /usr/share/doc/yafc-1.1.1/doc/yafc.info
> 
> The package does indeed contain an empty compressed file yafc.info.gz.

The cause is building triggers regeneration of the *.info while "makeinfo" is
missing.

/builddir/build/BUILD/yafc-1.1.1/support/missing: line 46: makeinfo: command not
found
WARNING: `makeinfo' is missing on your system.  You should only need it if
         you modified a `.texi' or `.texinfo' file, or any other file
         indirectly affecting the aspect of the manual.  The spurious
         call might also be the consequence of using a buggy `make' (AIX,
         DU, IRIX).  You might want to install the `Texinfo' package or
         the `GNU make' package.  Grab either from any GNU archive site.

=> BR: /usr/sbin/makeinfo
could be applied to work around this issue.


But .. the actual cause is deeper: The tarball is not packaged properly.
It contains a raw preconfigured CVS snapshot with all temporary files and broken
timestamps inside.

Due to this I strongly recommend to add a 
make distclean
to %prep to assure the subsequent %configure doesn't "reconfigure" the source
tree, but to configure it anew.

Further issues:

* You put *.texi's in %doc. These *.texi's are the sources of yafc.info. It
doesn't make much sense to put them into %docdir

* Related to the previous issue, the "prepare for %doc" block doesn't seem
useful to me. The package ships and installs mans and infos, its Makefiles
handle them correctly. There is no for any special treatment.



Comment 7 Chris Petersen 2006-09-07 20:22:42 UTC
I think I addressed the requested issues.  rpmlint is quiet now, too.

spec:  http://rpm.forevermore.net/yafc/yafc.spec
srpm:  http://rpm.forevermore.net/yafc/yafc-1.1.1-4.src.rpm

Comment 8 Kevin Fenzi 2006-09-12 21:57:19 UTC
Gérard or Ralf: Are either of you going to do a formal review on this package?

If not, I would be happy to step in and do so...

Comment 9 Gérard Milmeister 2006-09-12 23:28:25 UTC
(In reply to comment #8)
> Gérard or Ralf: Are either of you going to do a formal review on this package?
> 
> If not, I would be happy to step in and do so...
Yes, go on.

Comment 10 Kevin Fenzi 2006-09-13 02:11:53 UTC
OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
832d074183a36ee15b47553ed5962fce  yafc-1.1.1.tar.bz2
832d074183a36ee15b47553ed5962fce  yafc-1.1.1.tar.bz2.1
OK - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
See below - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package doesn't own any directories other packages own.
OK - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
OK - Should build in mock.

Issues:

1. You might change your default attributes from:
%defattr(-, root, root)
to
%defattr(-, root, root,-)

Aside from that I see no issues.
Feel free to change issue #1 above when you check the package in.

Pending that, this package is APPROVED.


Comment 11 Chris Petersen 2006-09-13 06:16:19 UTC
Which license?  The GPL is there already (document is called COPYING, which it
is in many GPL programs).  Package builds fine in mock in my system, too.

Anyway, issue #1 is fixed, and I will add the package to extras soon.

Comment 12 Kevin Fenzi 2006-09-13 15:24:25 UTC
In reply to comment #11: 

Yeah, thats my checklist of items when looking at packages... 
the OK there means that they License was included and is ok. :) 

Comment 13 Chris Petersen 2007-06-22 18:26:00 UTC
Might as well get all of my good packages into EPEL:

Package Change Request
======================
Package Name: yafc
New Branches: EL-5

Comment 14 Kevin Fenzi 2007-06-22 19:50:42 UTC
cvs done.


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