Bug 226195 - Merge Review: newt
Summary: Merge Review: newt
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
Depends On:
Blocks: F9MergeReviewTarget
TreeView+ depends on / blocked
Reported: 2007-01-31 20:16 UTC by Nobody's working on this, feel free to take it
Modified: 2009-04-09 19:04 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2009-04-09 19:04:35 UTC
Type: ---
j: fedora-review+

Attachments (Terms of Use)
Suggested specfile patch fixing several issues. (3.64 KB, patch)
2007-03-01 02:43 UTC, Jason Tibbitts
no flags Details | Diff
Patch to split off newt-python relative to current rawhide (1.60 KB, patch)
2007-06-22 13:09 UTC, Yanko Kaneti
no flags Details | Diff

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

Initial Owner: mlichvar@redhat.com

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
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.

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:
   snack = 0.52.5-1.fc7
   newt = 0.52.5-1.fc7
   python(abi) = 2.5

   newt-devel = 0.52.5-1.fc7
   newt = 0.52.5

* %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@elvis.redhat.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 
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.

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