Spec file: http://web.phys.ntnu.no/~terjeros/eterm/eterm.spec SRPM : http://web.phys.ntnu.no/~terjeros/eterm/eterm-0.9.3-1.src.rpm Description: Eterm is a color vt102 terminal emulator intended as a replacement for xterm. Depends on libast which is also submitted.
Hi Terje, this is not a full review, just a few observations: good: + source matches upstream needswork: - BuildRoot should be: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) - rpmlint reports: E: eterm explicit-lib-dependency libast E: eterm binary-or-shlib-defines-rpath /usr/bin/Esetroot ['/usr/lib', '/usr/lib/Eterm'] E: eterm binary-or-shlib-defines-rpath /usr/bin/Eterm ['/usr/lib', '/usr/lib/Eterm'] amusing: + from the ./configure output: checking for life_signs in -lKenny... no Oh my god, they killed Kenny! You bastards! probably not acceptable in FC or FE: - from the Eterm-0.9.3/src/command.c file: * Copyright 1992 John Bovey, University of Kent at Canterbury. * * You can do what you like with this source code as long as * you don't try to make money out of it and you include an * unaltered copy of this message (including the copyright). Please contact upstream and inquire about the overall license. The vast majority of the code is BSD-style (per Michael Jennings) but the license on the command.c file explicitly forbids "making money" and AFAICT this is not acceptable for FC or FE (that is, folks should be allowed to sell copies of Fedora-packaged software if they desire).
> - BuildRoot should be: > %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) Fixed. > E: eterm explicit-lib-dependency libast Fixed. > E: eterm binary-or-shlib-defines-rpath > /usr/bin/Esetroot ['/usr/lib', '/usr/lib/Eterm'] > E: eterm binary-or-shlib-defines-rpath > /usr/bin/Eterm ['/usr/lib', '/usr/lib/Eterm'] Is this a showstopper? Don't know how to fix this, any ideas? > Please contact upstream and inquire about the overall license. The vast > majority of the code is BSD-style (per Michael Jennings) but the license on > the command.c file explicitly forbids "making money" and AFAICT this is not > acceptable for FC or FE (that is, folks should be allowed to sell copies of > Fedora-packaged software if they desire). Ok, I sent John a email, here is his respons: The code must originate from xvt, a lightweight replacement for xterm that I wrote many years ago. It was used by Rob Nation as the basis for rxvt and it looks like it has found its way into other terminal emulators. The comment about 'not making any money' was the original license but xvt has also been released in debian with a gpl license. I am happy for any terminal emulator code of mine to be on a gpl license. Updated package available here: Spec: http://web.phys.ntnu.no/~terjeros/eterm/eterm.spec SRPM: http://web.phys.ntnu.no/~terjeros/eterm/eterm-0.9.3-2.src.rpm
Hi Terje, I think the rpath error is a blocker (can anyone else comment here?) and will look into it some more. Also, I'll do a thorough review as soon as I can. Please try to get the license issues sorted out *within* the upstream source since they are a blocker. If the upstream maintainers clarify things (say, if they put a single COPYING or LICENSE file that makes it clear what the overall terms with--with *no* inconsistencies in the individual files), that would be ideal.
I sent a mail to Michael some time ago, however no feedback yet. You have seen the comments on #182175? Uhm, seems like this is a mess: src/command.{c,h}: Copyright John Bovey, not make money or GPL. src/grkelot.{c,h}: Copyright Angelo Haritsis, unknown. src/libscream.c : Copyright Azundris, LGPL. src/netdisp.c : Copyright Chuck Blake/mj olesen, not make money thingie. src/{scream.h,screancfg.h}, : Azundris, BSD. src/screen.h : Robert Nation and mj olesen, ?. the rest : Copyright Michael Jennings, BSD(?). BTW: I think Eterm was shipped in RHL 9 and is included in Debian 3.1.
Hi Terje, I'm not sure how to proceed here. It looks like parts of the source are licensed under conflicting terms. So, what to do? I think we should ask for feedback and will send an email today...
Hi Terje, I've contacted the debian eterm maintainer to point out the license mess within the source files (as was suggested on the fedora- extras-list). I'll leave this bug open for a couple weeks in the hope that upstream will respond to you and will sort it out (the files that have license problems are a fraction of the overall code so perhaps they can be removed or replaced?). If not, the package will be rejected. Not much else that we can do here...!
Ok, I give up for now. No Eterm package in Extras, sorry.
Hi Terje, Its not at all your fault. Thank you for trying to package it. Its a shame that upstream could not or would not sort out the licenses. I liked Eterm and used it and Enlightenment for a few years. When I started, I had no idea this would be the outcome of this review. Its not what I wanted. Bummer.
(In reply to comment #3) > Hi Terje, I think the rpath error is a blocker (can anyone else comment > here?) That is one of the "stupid policies" I mentioned in the other bug. There is nothing inherently wrong with rpath, and blindly rejecting packages because of it is ridiculous. Particularly since the paths Eterm uses are NOT WRITEABLE. The "fix" (I use the term loosely) is to edit Makefile.am to remove the -rpath parameter. > Please try to get the license issues sorted out *within* the upstream > source since they are a blocker. If the upstream maintainers clarify > things (say, if they put a single COPYING or LICENSE file that makes it > clear what the overall terms with--with *no* inconsistencies in the > individual files), that would be ideal. Eterm 0.9.4, which has just been released, has the appropriate LICENSE file to clarify the situation. (In reply to comment #4) > I sent a mail to Michael some time ago, however no feedback yet. Had I *actually* been contacted about this, I would've taken action sooner. As it is, no one who has posted on this bug contacted me about it, nor did the Debian maintainer or any debian developer. It wasn't until a USER named Nolius dropped me an e-mail with a link to the eterm package news page that I heard of this issue. Shame on you both. (In reply to comment #8) > Its a shame that upstream could not or would not sort out the licenses. I did, and I would've done it sooner had either of you e-mailed me directly about it. I was e-mailed about the other bug, WRT LibAST, but not Eterm.
New package based on 0.9.4 with o rpath problem fixed o LICENSE taken from upstream package SPEC: http://web.phys.ntnu.no/~terjeros/eterm/eterm.spec SRPM: http://web.phys.ntnu.no/~terjeros/eterm/eterm-0.9.4-1.src.rpm
Hi Terje, the license issue does appear to be cleaned up. The no-money one is gone but the LGPL-ed bits remain. I'm no lawyer but it seems OK to link together the LGPL-ed parts with BSD code. There are some suspicious bits such as: 1) literally hundreds of "pointer targets ... differ in signedness" warnings which are worrisome but perhaps ignorable 2) there appear to be some missing BuildRequires and/or some missing functionality such as: checking for Etwin support... checking for Tw_Open in -lTw... no configure: WARNING: *** Twin support has been disabled because libTw was not found *** and I think libXmu-devel needs to be a BR since I don't see how it gets pulled in by any of the other BRs. Please take a look. In any case, the remaining review items are: + source matches upstream + license now appears to be OK and is correctly included + builds on FC5 i386 + rpmlint reports no errors or warnings + package and spec naming OK + spec is legible + builds on FC5 i386 + no locales + shared libs OK + not relocatable + dir ownership looks good + no file dupes + permissions OK + has %clean + consistent use of macros + code not content (although there are a number of background pixmaps that could be split off into a separate package if one desires) + docs are small and not needed for execution + no static, *.la, or devel libs + no headers or pkgconfig + has desktop file with desktop-file-install which appears sane It'll be easy enough to sort out the BuildRequires with mock as soon as libast is in Extras so we can leave that for later. And I don't see any remaining blockers so its APPROVED.
Hi Terje, are you still interested in this package?
> 1) literally hundreds of "pointer targets ... differ in signedness" > warnings which are worrisome but perhaps ignorable I have used Eterm for years without problems... > 2) there appear to be some missing BuildRequires and/or some missing > functionality such as: > > checking for Etwin support... > checking for Tw_Open in -lTw... no > configure: WARNING: *** Twin support has been > disabled because libTw was not found *** I believe this is support for a Twin - a textmode window environment, (http://twin.sf.net), this issue cannot be a blocker. Warning is gone with --disable-etwin. > and I think libXmu-devel needs to be a BR since I don't see how it > gets pulled in by any of the other BRs. Please take a look. Yeah, added pcre-devel too. And fixed the desktop file. > It'll be easy enough to sort out the BuildRequires with mock as soon > as libast is in Extras so we can leave that for later. libast is in FE now :-) New files: SPEC: http://web.phys.ntnu.no/~terjeros/eterm/eterm.spec SRPMS: http://web.phys.ntnu.no/~terjeros/eterm/eterm-0.9.4-2.fc6.src.rpm