Bug 226489 - Merge Review: tftp
Merge Review: tftp
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Package Reviews List
: Reopened
Depends On:
  Show dependency treegraph
Reported: 2007-01-31 16:10 EST by Nobody's working on this, feel free to take it
Modified: 2008-01-28 04:42 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-01-28 04:42:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
pertusus: fedora‑review+

Attachments (Terms of Use)
correct 2 typos in instal and INSTAL (906 bytes, patch)
2008-01-25 04:38 EST, Patrice Dumas
no flags Details | Diff
use rpm macros instead of hardcoded paths (1.41 KB, patch)
2008-01-25 18:31 EST, Patrice Dumas
no flags Details | Diff

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 16:10:26 EST
Fedora Merge Review: tftp

Initial Owner: mbarabas@redhat.com
Comment 1 Michael DeHaan 2007-02-16 18:30:34 EST
rpmlint *.src.rpm:

W: tftp summary-ended-with-dot The client for the Trivial File Transfer Protocol

This should be easy to fix :)

W: tftp no-url-tag

This should be something like Url:  http://foo.redhat.com at minimum or maybe
you could point at the upstream home of tftp.

W: tftp buildprereq-use tcp_wrappers-devel

I'm not really familiar with BuildRequires vs BuildPreRequires here, though it
seems like BuildRequires should work fine.

W: tftp mixed-use-of-spaces-and-tabs (spaces: line 54, tab: line 55)

This isn't hurting anything, though moving install up to the previous line would
make this go away.  Depends how much you care about rpmlint being whiney.


Other things I looked over:

service logic looks good.  xinetd is reloaded on an uninstall or after
newinstalls or upgrades.

config is noreplaced (good), permissions, good, etc.

One potential problem is that the BuildRoot doesn't contain version info (see

Comment 2 Ruben Kerkhof 2007-06-26 14:48:31 EDT
Is there a reason this bug is closed?
Comment 3 Maros Barabas 2007-07-09 04:57:04 EDT
Hi, please take a look at last version in devel. 
Comment 4 Patrice Dumas 2007-11-28 08:12:45 EST
Martin, looks like you are the new maintainer, so the bug shouldn't 
be assigned to you, since the bug is assigned to the reviewer. In 
general the submitter is the Reporter, but not for merge reviews. 
So, since it is a merge review, you should just be in CC (though it may 
be nice if you say 'hey I am the current maintainer' when you put
yourself in CC)
Comment 5 Martin Nagy 2007-12-11 06:39:24 EST
Hi, I am the new maintainer. Thanks Patrice, for cleaning this up, I don't know
why I was assigned to this bug.

Could somebody please take the review? The packages in rawhide look good.
Comment 6 Patrice Dumas 2007-12-17 12:08:52 EST
This is not the latest version.

The current version source timestamp is not kept, so please keep 
it next time.

Some files in %doc are missing, like README (but first update).

I suggest adding INSTALL='install -p' on the make install command line
to keep timestamps, and also use -p for tftp-xinetd install command.

I suggest using %defattr(-,root,root,-) instead of %defattr(-,root,root)

I'll certainly review the updated package.
Comment 7 Martin Nagy 2008-01-22 08:25:56 EST
I have updated the package in CVS, please review. Thanks.
Comment 8 Patrice Dumas 2008-01-25 04:38:43 EST
Created attachment 292923 [details]
correct 2 typos in instal and INSTAL
Comment 9 Patrice Dumas 2008-01-25 04:39:34 EST
Looking at the ./configure --help output, it looks like a 
readline-devel buildrequires is missing?
Comment 10 Martin Nagy 2008-01-25 05:14:33 EST
It looks like it can manage without readline-devel, but it's of course better
with it, so I added the dependency, updated in CVS.
Comment 11 Patrice Dumas 2008-01-25 18:31:58 EST
Created attachment 293022 [details]
use rpm macros instead of hardcoded paths
Comment 12 Patrice Dumas 2008-01-25 18:33:16 EST
I proposed a patch to use rpm macros to have config file
matching the real installation paths.

I'd prefer if you applied it, but even if you don't this is

Comment 13 Martin Nagy 2008-01-28 04:42:36 EST
Fixed in tftp-0.48-1.fc9
Thanks for the review Patrice!

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