Bug 226195

Summary: Merge Review: newt
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: mlichvar, mnowak, redhat-bugzilla, richlv, yaneti
Target Milestone: ---Flags: j: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-04-09 19:04:35 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:
Bug Depends On:    
Bug Blocks: 426387    
Attachments:
Description Flags
Suggested specfile patch fixing several issues.
none
Patch to split off newt-python relative to current rawhide none

Description Nobody's working on this, feel free to take it 2007-01-31 20:16:08 UTC
Fedora Merge Review: newt

http://cvs.fedora.redhat.com/viewcvs/devel/newt/
Initial Owner: mlichvar

Comment 1 Jason Tibbitts 2007-03-01 02:42:38 UTC
First off, does newt have an upstream?  Or is Red Hat the upstream?  If there
is an upstream, a URL tag would be good if possible, and either a full URL in
the Source tag, or some instruction on making the tarball from CVS or
whatever is needed.

Some rpmlint complaints:

W: newt summary-ended-with-dot A development library for text mode user
interfaces.
W: newt-devel summary-ended-with-dot Newt windowing toolkit development files.
   Trivial fixes.

E: newt tag-not-utf8 %changelog
E: newt non-utf8-spec-file newt.spec
   This is due to Trond's name; in the changelog; a quick pass through iconv
   fixes things.

W: newt no-url-tag
   Depends on whether there's an upstream to link to.

E: newt configure-without-libdir-spec
   rpmlint is confused by the commented out "./configure" line.

W: newt macro-in-%changelog release
W: newt macro-in-%changelog version

E: newt script-without-shebang /usr/lib64/python2.5/site-packages/snack.py
   This is executable; if the user is supposed to run it, it should have a
   shebang line.  If not, then it shouldn't be executable.  It has no main, so
   I'm pretty sure it's not supposed to be run.

After that, the only real issue is the static library.  This is obviously
justified as the installer needs it, so all that's needed is a bit of
justification in a comment and a -static subpackage.

I'll attach a patch which fixes up the summaries, the non-utf8 bits, and the
macros in %changelog.  It also splits the static library off into a -static
subpackage and includes some justification.  (Obviously the installer will
need to be fixed to pull in the -static package.)

After this patch, all that remains are the questions of the where upstream is
and where the source comes from.

Checklist:
X can't compare source with upstream.  (Perhaps this is the upstream.)
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
? latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint has valid complaints
* final provides and requires are sane:
  newt-0.52.5-1.fc7.x86_64.rpm
   _snackmodule.so()(64bit)
   libnewt.so.0.52()(64bit)
   libnewt.so.0.52(NEWT_0.52)(64bit)
   snack = 0.52.5-1.fc7
   newt = 0.52.5-1.fc7
  =
   /sbin/ldconfig
   libnewt.so.0.52()(64bit)
   libnewt.so.0.52(NEWT_0.52)(64bit)
   libpopt.so.0()(64bit)
   libslang.so.2()(64bit)
   libslang.so.2(SLANG2)(64bit)
   python(abi) = 2.5

  newt-devel-0.52.5-1.fc7.x86_64.rpm
   newt-devel = 0.52.5-1.fc7
  =
   libnewt.so.0.52()(64bit)
   newt = 0.52.5
   slang-devel

* %check is not present; no test suite upstream
* shared libraries present; ldconfig 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.
* find-lang used appropriately.
* scriptlets are OK (ldconfig)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel subpackage.
* no pkgconfig files.
X static libraries present, not in -static and no justification.
* no libtool .la droppings.

Comment 2 Jason Tibbitts 2007-03-01 02:43:29 UTC
Created attachment 148984 [details]
Suggested specfile patch fixing several issues.

Comment 3 Miroslav Lichvar 2007-03-01 13:49:15 UTC
Thanks for the review. Red Hat is the upstream, CVS is at elvis.redhat.com, you
can checkout sources with
cvs -z3 -d :pserver:anonymous.com:/usr/local/CVS co newt

Comment 4 Jason Tibbitts 2007-03-01 17:32:14 UTC
Could you include info in the specfile which details how to create the tarball
you're using?  See http://fedoraproject.org/wiki/Packaging/SourceURL#RevisionControl

I'm not sure that just pulling the CVS HEAD is the best idea, because you lose
repeatability as new code is committed.  Usually you want to pull a tag.

Comment 5 Miroslav Lichvar 2007-03-02 13:33:22 UTC
Ok, should be fixed in newt-0.52.6-1.fc7.

Comment 6 Yanko Kaneti 2007-06-22 13:09:52 UTC
Created attachment 157617 [details]
Patch to split off newt-python  relative to current rawhide

Comment 7 richlv 2007-11-05 17:45:37 UTC
i wanted to suggest creating simple project page for newt, which would include 
source tarball downloads.

there are some projects that depends on newt (for example, partimage), thus 
such a thing would be beneficial for users of these packages.

some distributions are doing some repackaging themselves (debian, for example), 
but that's far from optimal.

if this would be more appropriate as a new report, i'll gladly file one.

Comment 8 Miroslav Lichvar 2007-11-06 14:18:20 UTC
I think the plan is to move projects from elvis CVS to hosted.fedoraproject.org,
but I'm not sure when it will happen.

Comment 9 Miroslav Lichvar 2008-03-21 12:34:47 UTC
Source and URL tag updated in newt-0.52.9-1.fc9.

Upstream tarballs are at https://fedorahosted.org/releases/n/e/newt/.

Comment 10 Jason Tibbitts 2008-10-09 15:45:55 UTC
Wow, somehow this got set to the modified state which causes it to drop completely out of every report I run to indicate the tickets I should be working on.  I have no idea why anyone would ever use that state, because it sure screws things up.  Let me take another look at things.

Comment 11 Michal Nowak 2009-04-08 16:08:39 UTC
Any progress? What's blocking this review?

Comment 12 Jason Tibbitts 2009-04-08 16:21:45 UTC
Well, damn, somehow I keep dropping this.  I'll look at it today.

Comment 13 Jason Tibbitts 2009-04-09 19:04:35 UTC
Looks like "today" turned into "tomorrow", but here we go.

I checked out and built the latest rawhide branch of newt.  rpmlint is down to:
  newt.x86_64: W: shared-lib-calls-exit /usr/lib64/libnewt.so.0.52.10 
   exit.5
which is a bad idea and arguably a bug in the library, but not a review blocker.

  newt-static.x86_64: W: no-documentation
which is fine.

So rpmlint is fine now, and upon inspection the other issues I had are fixed as well.  Really the only thing I see currently is that the license on the code is rather unclear.  There's nothing in the tarball that I can see which specifies which version of the LGPL is in use, which would, according to clause 13 of the included COPYING file, indicate that we are free to choose any version (i.e. LGPLv2+).  Only the included spec file indicates a specific version, and I'm not absolutely certain whether or not that's a sufficient statement of intent from a legal standpoint.

Still, since I believe you folks are the upstream and the specfile in the tarball does say LGPLv2 so I'm going to say that things are OK, but I do strongly suggest that you clarify the licensing, preferrably by including the proper license blocks at the start of the source files as recommended by the COPYING file, or at minimum at least say "version 2.1 of the LGPL only" somewhere in the documentation or code.

APPROVED, and closing.