Bug 319031

Summary: Review Request: nget - Command line NNTP file grabber
Product: [Fedora] Fedora Reporter: Debarshi Ray <debarshir>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting
Target Milestone: ---Flags: j: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-10-12 19:02:21 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 Debarshi Ray 2007-10-04 18:53:03 UTC
Spec URL: http://rishi.fedorapeople.org/nget.spec
SRPM URL: http://rishi.fedorapeople.org/nget-0.27.1-5.fc7.src.rpm

Description:
nget is a command line NNTP file grabber. It automatically pieces together
multipart postings for easy retrieval, even substituting parts from multiple
servers and newsgroups. Handles disconnects gracefully, resuming after the
last part successfully downloaded.

Comment 1 Debarshi Ray 2007-10-04 18:54:40 UTC
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=183243

Comment 2 Jason Tibbitts 2007-10-04 19:18:08 UTC
The debuginfo package comes out empty.  I believe this is because the makefile
strips the binaries:

/usr/bin/install -c -s -m 0755 nget /var/tmp/nget-0.27.1-5.fc8-u22592/usr/bin
/usr/bin/install -c -s -m 0755 ngetlite /var/tmp/nget-0.27.1-5.fc8-u22592/usr/bin

This must be corrected; unfortunately this package uses autotools so I've no
idea how you'd go about doing that.

Comment 3 Debarshi Ray 2007-10-05 02:51:46 UTC
Spec: http://rishi.fedorapeople.org/nget.spec
SRPM: http://rishi.fedorapeople.org/nget-0.27.1-6.fc7.src.rpm

Removing the '-s' option to install in Makefile.in fixed the 'empty debuginfo'
issue.


Comment 4 Jason Tibbitts 2007-10-05 20:09:58 UTC
This is a clean package; I have only a couple of questions:

Why do you rename "Changelog" to "ChangeLog"?  Is there something wrong with the
name upstream chose for this file?

I see that you have a build dependency on cppunit so I figured you were trying
to run the test suite; what you really need to run it is build dependencies on
cppunit-devel and python, plus
  %check
  make test
but unfortunately, while the test suite runs properly, some tests fail because
subterfuge isn't there (and doesn't seem to be available in Fedora).  It seems
that it isn't as optional as the nget documentation seems to imply.

So I guess there's no point in trying to run the test suite at this time, and so
you should probably remove the cppunit build depdendency and add some commentary
about the impossibility of running the test suite without subterfuge.

* source files match upstream:
   6ec0339c8e0e3e31504f5171f479f154d3c0ba2108377a0105fe4a5f5082ea69  
   nget-0.27.1.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
X BuildRequires include unnecessary cppunit dependency.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   nget = 0.27.1-6.fc8
  =
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libpcre.so.0()(64bit)
   libpcreposix.so.0()(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)
   libz.so.1()(64bit)
? %check is not present; test suite doesn't seem to run properly.  This should 
   be documented.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.
* statically links against uulib; properly BuildRequires: uulib-static

Comment 5 Debarshi Ray 2007-10-06 10:39:22 UTC
(In reply to comment #4)
> Why do you rename "Changelog" to "ChangeLog"?  Is there something wrong
> with the name upstream chose for this file?

ChangeLog seems to be the de-facto standard. Just some statistics from my Fedora
7 system:
[rishi@freebook ~]$ find /usr/share/doc -name "ChangeLog" -print | wc -l
273
[rishi@freebook ~]$ find /usr/share/doc -name "Changelog" -print | wc -l
5
[rishi@freebook ~]$ find /usr/share/doc -name "CHANGELOG" -print | wc -l
5

Would it not be better to go with the more often used naming pattern? That would
help in writing automated scripts, for example.

Comment 6 Jason Tibbitts 2007-10-06 14:37:09 UTC
That's upstream's choice to make, though.  (Remember our motto about how we
stick close to upstream?)  There are all sorts of things you could do to
upstream documentation to make it more uniform, but in general we don't do that
kind of thing unless there's a conflict with our guidelines.

If you want to propose a guideline regulating the naming of the changelog file,
feel free.

Comment 7 Debarshi Ray 2007-10-07 04:33:51 UTC
Spec: http://rishi.fedorapeople.org/nget.spec
SRPM: http://rishi.fedorapeople.org/nget-0.27.1-7.fc7.src.rpm

Both issues addressed.

Comment 8 Jason Tibbitts 2007-10-08 15:17:57 UTC
Sorry, I had mistakenly thought I approved this already.

Everything looks good to me; APPROVED

Comment 9 Debarshi Ray 2007-10-09 06:01:27 UTC
New Package CVS Request
=======================
Package Name: nget
Short Description: Command line NNTP file grabber.
Owners: rishi
Branches: FC-6, F-7
InitialCC: 
Cvsextras Commits: no

Comment 10 Kevin Fenzi 2007-10-09 15:43:38 UTC
cvs done.

Comment 11 Fedora Update System 2007-10-11 22:52:05 UTC
nget-0.27.1-7.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update nget'

Comment 12 Fedora Update System 2007-10-18 02:29:19 UTC
nget-0.27.1-7.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.