+1 for wanting to see this reviewed and in Fedora.

I'll do a full review in a couple of days. Meanwhile, could you please try to preserve the timestamps (by using a touch -r after iconv and using install="INSTALL-p") ? Now all files (but those included with %doc), including the source from -devel, get the rpmbuilding time

As a sidenote, I tested it in Centos 5 and it failed to see my USB mouse. gpm was running. Is it normal / expected  / or maybe untested ?

(In reply to comment #2)
> I'll do a full review in a couple of days. Meanwhile, could you please try to
> preserve the timestamps (by using a touch -r after iconv and using
> install="INSTALL-p") ? Now all files (but those included with %doc), including
> the source from -devel, get the rpmbuilding time

Fixed, I hope I didn't miss anything because unfortunately the sources are very inconsistent regarding the usage of 'install'.

> As a sidenote, I tested it in Centos 5 and it failed to see my USB mouse. gpm
> was running. Is it normal / expected  / or maybe untested ?

Untested, I'll look at it later.

New spec file: http://mjakubicek.fedorapeople.org/twin/twin.spec
New SRPM: http://mjakubicek.fedorapeople.org/twin/twin-0.6.0-2.fc9.src.rpm

(There are also x86_64 RPMs in the same directory available)

 There seem to be a couple of problems:
- gpm-devel is needed as BR in order to get a functional mouse (at least on C5)
- since you package twsetroot, I suggest including (in %doc) the README.twsetroot file and maybe setroot.sample too. In the /docs directory there are also some other files which seem to have interesting content (diagram.txt, FAQ). Do you have a special reason to not include them in the final rpm ?

 And now the real problems:
- twmapscrn is built against the clients/mapscrn folder which seems to contain a private (old and slightly modified) copy of kbd (ftp://ftp.win.tue.nl/pub/linux-local/utils/kbd/kbd-1.06.tar.gz), which in turn is a system package. As usage of private copies of system libs is explicitly forbidden, I'd say we have an issue here
- from the licensing point of view, we have a small mess
a) lots of files have headers defining them as GPLv2+ (good)
b) headers of some other files specifu Public Domain as license (good again)
c) however  there are several files ( for instance clients/threadtest.c and many files under /lib  ) which have no license specified. What reason can we invoke in order to assume that they are like all the others, Public Domain or GPLv2+ ? 
  In addition to that, the sourcefarge page of the project (http://sourceforge.net/projects/twin/) claims that the project is licensed as GPL and LGPL, but LGPL is only mentioned in the source through the presence of the standard LGPL license file; I have not been able to locate any other trace of it. Public Domain + GPLv2+ = no problem, but the presence of files with no specific license make me ask for help. Anyone more experienced in licensing willing to shed some light ?

(In reply to comment #4)

Sorry for the delay,

>  There seem to be a couple of problems:
> - gpm-devel is needed as BR in order to get a functional mouse (at least on C5)

Unfortunately I can't get it working even with BR: gpm-devel (outside of X of course), I'm still getting:

GPM_InitMouse() failed: unable to connect to `gpm'.
make sure you started `twin' from the console
and/or check that `gpm' is running.
xterm_InitMouse() failed: this `linux' terminal
has no support for xterm-style mouse reporting.

I'll try to contact author in order to fix this.

> - since you package twsetroot, I suggest including (in %doc) the
> README.twsetroot file and maybe setroot.sample too. In the /docs directory
> there are also some other files which seem to have interesting content
> (diagram.txt, FAQ). Do you have a special reason to not include them in the
> final rpm ?

No, I'll include them of course, thanks for hint.

>  And now the real problems:
> - twmapscrn is built against the clients/mapscrn folder which seems to contain
> a private (old and slightly modified) copy of kbd
> (ftp://ftp.win.tue.nl/pub/linux-local/utils/kbd/kbd-1.06.tar.gz), which in turn
> is a system package. As usage of private copies of system libs is explicitly
> forbidden, I'd say we have an issue here

Hm, I just tried to remove twmapscrn and symlink it to the mapscrn provided by kbd -- and it works. I'll do more tests to check whether it really works, but if yes, this would be solution, wouldn't it?


> - from the licensing point of view, we have a small mess
> a) lots of files have headers defining them as GPLv2+ (good)
> b) headers of some other files specifu Public Domain as license (good again)
> c) however  there are several files ( for instance clients/threadtest.c and
> many files under /lib  ) which have no license specified. What reason can we
> invoke in order to assume that they are like all the others, Public Domain or
> GPLv2+ ? 
>   In addition to that, the sourcefarge page of the project
> (http://sourceforge.net/projects/twin/) claims that the project is licensed as
> GPL and LGPL, but LGPL is only mentioned in the source through the presence of
> the standard LGPL license file; I have not been able to locate any other trace
> of it. Public Domain + GPLv2+ = no problem, but the presence of files with no
> specific license make me ask for help. Anyone more experienced in licensing
> willing to shed some light ?

CC'ing Tom Callaway: do we have to ask upstream to specify license in each file?

ref:
 gpm: waiting for solution. 
 docs: excellent, thanks
 twmapscrn: of course, as long as you make sure all the deps are required/buildrequired.
 license: waiting for advices

(In reply to comment #6)

OK, just to let you know I didn't forget about this:

> ref:
>  gpm: waiting for solution. 

Fixed, this was plain wrong on my side.

>  docs: excellent, thanks
>  twmapscrn: of course, as long as you make sure all the deps are
> required/buildrequired.
>  license: waiting for advices

Waiting for upstream reaction -- still:( I finally succeeded to contact the author (e-mail address didn't work anymore), today I sent the second request to fix the license problem, I hope I'll get an answer (soon).

Thank you for your patience.

OK, finally there is a new upstream release solving the licensing issues:

New SPEC file: http://mjakubicek.fedorapeople.org/twin/twin.spec
New SRPM: http://mjakubicek.fedorapeople.org/twin/twin-0.6.1-1.fc10.src.rpm

I've also tested that it builds with gcc 4.4, see the koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1133535

...just forgot to mention that I've also created and packaged a LICENSING.INFO file which recapitulates the state of licensing, there are finally 5 licenses in use:(

"
MIT: admin/install-sh
Public domain:  admin/mkinstalldirs clients/{cat.c,clip.c,clutter.c,demo.c,demo2.c,demo3.c,findtwin.c,lsobj.c,restackM.c,restackW.c,sendmsg.c,sysmon.c}
                include/md5.h,server/{md5.c,wrapper.c}
BSD with advertising: include/my_ttydefaults.h
LGPLv2+: lib/*
GPLv2+: others
"

Normally, the BSD with advertising file would be a problem, but the Copyright holder on that file is The Regents of the University of California, and they issued a blanket statement years ago dropping that clause.

ftp://ftp.cs.berkeley.edu/pub/4bsd/README.Impt.License.Change

You should tell upstream about this, and have them drop the advertising clause in their copy of that code. You also need to correct the License tag to drop the advertising clause.

Aside from that issue, it looks okay.

More a "pong" than a review, I';; be back a bit later:

rpmlint of twin:
twin.x86_64: W: file-not-utf8 /usr/share/doc/twin-0.6.1/setroot.sample
twin.x86_64: W: spurious-executable-perm /usr/share/doc/twin-0.6.1/LICENSING.INFO
twin.x86_64: W: file-not-utf8 /usr/share/doc/twin-0.6.1/README.twsetroot

More serious stuff, I am a bit puzzled by this snipplet from the build log (http://koji.fedoraproject.org/koji/getfile?taskID=1133537&name=build.log ):
checking for X... 
libraries , headers 
checking X11/xpm.h usability... 
no
checking X11/xpm.h presence... 
no
checking for X11/xpm.h... 
no
checking for XpmReadFileToPixmap in -lXpm... 
no
checking for gtk-config... 
no
checking gtk/gtk.h usability... 
no
checking gtk/gtk.h presence... 
no
checking for gtk/gtk.h... 
no
checking for gtk_init in -lgtk... 
no

It seems that BR gtk2-devel is not sufficient ?

(In reply to comment #11)
> More a "pong" than a review, I';; be back a bit later:
> 
> rpmlint of twin:
> twin.x86_64: W: file-not-utf8 /usr/share/doc/twin-0.6.1/setroot.sample
> twin.x86_64: W: spurious-executable-perm
> /usr/share/doc/twin-0.6.1/LICENSING.INFO
> twin.x86_64: W: file-not-utf8 /usr/share/doc/twin-0.6.1/README.twsetroot

Ah, forgot to rpmlint again, sorry, fixed.

> More serious stuff, I am a bit puzzled by this snipplet from the build log
> (http://koji.fedoraproject.org/koji/getfile?taskID=1133537&name=build.log ):
[...]
> checking for gtk-config... 
> no
[...]
> It seems that BR gtk2-devel is not sufficient ?

Yeah, gtk-config is in gtk+-devel, thanks for noticing that!

New SPEC file: http://mjakubicek.fedorapeople.org/twin/twin.spec
New SRPM: http://mjakubicek.fedorapeople.org/twin/twin-0.6.1-2.fc10.src.rpm
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1143787

P.S. The BSD/BSD with advertising issue is just pending releasing new sources from upstream, should be soon (probably without bumping release number), I've modified the licensing info in spec file in advance.

libXpm-devel was also missing as BR and the date of the last changelog entry was wrong (Fridat was Feb 20, not 21)

new koji scratch build at http://koji.fedoraproject.org/koji/taskinfo?taskID=1144478. I'll do the review later today.

I've tried to use the package (both with and without the libXpm-devel BR) in a F-10 guest and results were... strange:
- if run from a GUI (direct access from the VM GUI), I can see the message "press PAUSE or mouse right button for menu" on the topmost line of the screen. The rest of the screen is filled with an ugly backgroun, vaguely similar to a text screen (the aspect from the turbo vision GUI from the '90s ) full of stars (one star per cell). The only way to interact is to press the right button of the mouse, keep it pressed while moving over the desired menu entry and release the button only after the mouse is over the desired entry. Not the most intuitive interface... I've never seen that before on any system that I've worked with, since the era of SFDX 
- if run from a text console (ssh from the host text console), the mouse is not detected at all (I am offered the choice RETURN to start the app without mouse support or CTRL-C to kill it). After starting without mouse support, I failed to identify the right keyboard combination to trigger any menu action. The screen is once again filled with some blue-ish sort of background

(In reply to comment #14)
> I've tried to use the package (both with and without the libXpm-devel BR) in a

I'll add the BR as soon as there will be new sources with fixing the very last licensing problem (BSD with ad.)

> F-10 guest and results were... strange:
> - if run from a GUI (direct access from the VM GUI), I can see the message
> "press PAUSE or mouse right button for menu" on the topmost line of the screen.
> The rest of the screen is filled with an ugly backgroun, vaguely similar to a
> text screen (the aspect from the turbo vision GUI from the '90s ) full of stars
> (one star per cell). The only way to interact is to press the right button of
> the mouse, keep it pressed while moving over the desired menu entry and release
> the button only after the mouse is over the desired entry. Not the most
> intuitive interface... I've never seen that before on any system that I've
> worked with, since the era of SFDX 

Yes, me too, and I'm afraid this was intended (but I'll ask upstream to make sure it is not just misconfiguration or something like that).

> - if run from a text console (ssh from the host text console), the mouse is not
> detected at all (I am offered the choice RETURN to start the app without mouse
> support or CTRL-C to kill it). After starting without mouse support, I failed
> to identify the right keyboard combination to trigger any menu action. The
> screen is once again filled with some blue-ish sort of background

Please make sure that gpm is running (service gpm start) -- ?

(In reply to comment #14)
> - if run from a GUI (direct access from the VM GUI), I can see the message
> "press PAUSE or mouse right button for menu" on the topmost line of the screen.
> The rest of the screen is filled with an ugly backgroun, vaguely similar to a
> text screen (the aspect from the turbo vision GUI from the '90s ) full of stars
> (one star per cell). The only way to interact is to press the right button of
> the mouse, keep it pressed while moving over the desired menu entry and release
> the button only after the mouse is over the desired entry. Not the most
> intuitive interface... I've never seen that before on any system that I've
> worked with, since the era of SFDX 

I'm quoting the reply from upstream:

"
First of all, twin is not really supposed to be run inside a generic terminal
(or terminal emulator) as the features will be limited and the display will
look ugly (stars instead of pseudo-graphic characters, limited or missing
mouse support, ...)
For the best experience it is supposed to be used on Linux console or,
if you feel silly, inside an X11 session with an Unicode font or inside
itself.

As for the retro look reminding Turbo Vision GUI, it is intentional.
After all, twin means Text-mode WINdowing environment.

The look-and-feel can be customized up to a certain limit, but basicly
twin is supposed to sit "in the background" while the user interacts with
windows drawn inside it.
So, maybe the default configuration should start at least a maximized terminal
with user's shell running in it...

Also, trying to judge twin _server_ user interface is a bit funny:
it's like saying that running X11 server from the command line brings up
a checkered background with a cross-shaped mouse pointer and you cannot
do anything useful with it.
While technically true, it's not how users are supposed to use X11:
just as people use window managers or desktop environments (kde/gnome/...)
running inside X11, it would be more fair (and useful) to comment at least
twdm or twterm running inside twin.
"

The sources have been updated, I've changed the source URL to SourceForge.net on upstream's demand, added BR: libXpm-devel, fixed the typo in changelog:

New SPEC file: http://mjakubicek.fedorapeople.org/twin/twin.spec
New SRPM: http://mjakubicek.fedorapeople.org/twin/twin-0.6.1-3.fc10.src.rpm

Manuel, I'm just considering that I'd include also the small twutils (contains a kcalc-like app for twin) and xmms-twin sources into this one package (please look at http://linuz.sns.it/~max/twin/) -- what do you think about it? For me it doesn't seem to be worth making extra packages for all of them.

Sorry for mass-mailing today, I was talking a lot with upstream etc.

Explanations are sane enough and to be honest I am not surprised, which is why I tried to run it from a plain console, too. I'll test the new version too, but gpm was always started and the mouse was not working, so I guess there is (was ? ) a bug somewhere ... maybe in the mouse detection code ?

As of twutils and xmms-twin, I would create separate binary packages, even if coming from the same src.rpm. After all they are different projects, aren't they ?

(In reply to comment #18)
> As of twutils and xmms-twin, I would create separate binary packages, even if
> coming from the same src.rpm. After all they are different projects, aren't
> they ?  

Of course -- that's what I meant...needed a bit upstream communication regarding this, but finally we have it here:

New SPEC file: http://mjakubicek.fedorapeople.org/twin/twin.spec
New SRPM: http://mjakubicek.fedorapeople.org/twin/twin-0.6.1-4.fc10.src.rpm

Unfortunately xmms-twin segfaults right after startup, hence I'll disable the whole subpackage then until this works (maybe this is a x86_64 issue, haven't verified yet).

Things look mostly OK. A small issue is that part of the documentation is included twice:
-rw-r--r--    1 root    root                    18302 Feb 17 02:32 /usr/share/doc/twin-0.6.1/COPYING
-rw-r--r--    1 root    root                    25633 Feb 17 02:32 /usr/share/doc/twin-0.6.1/COPYING.LIB
-rw-r--r--    1 root    root                    48139 Feb 17 03:04 /usr/share/doc/twin-0.6.1/Changelog.txt
--rw-r--r--    1 root    root                     3620 Feb 17 02:32 /usr/share/doc/twin-0.6.1/README
-rw-r--r--    1 root    root                    18302 Feb 17 02:32 /usr/share/twin/COPYING
-rw-r--r--    1 root    root                    25633 Feb 17 02:32 /usr/share/twin/COPYING.LIB
-rw-r--r--    1 root    root                    48139 Feb 17 03:04 /usr/share/twin/Changelog.txt
-rw-r--r--    1 root    root                     3620 Feb 17 02:32 /usr/share/twin/README

And INSTALL (under share/twin) is useless I presume


I'll come back with a full review after I do a koji build for F10 + tests

First tests say that twin is not selinux-happy. When tryng to start te application I get the message:

twfindtwin: error while loading shared libraries: /usr/lib/libTw.so.4: cannot restore segment prot after reloc: Permission denied

and audit.log reveals:

type=AVC msg=audit(1235075038.804:69): avc:  denied  { execmod } for  pid=2256 comm="twin_real" path="/usr/lib/libTw.so.4.6.1" dev=dm-0 ino=149161 scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tcontext=system_u:object_r:lib_t:s0 tclass=file

later on, as soon as I started to test the various programs included in the package I got:

type=AVC msg=audit(1237006694.766:49): avc:  denied  { execmod } for  pid=3240 comm="twdetach" path="/usr/lib/libTw.so.4.6.1" dev=dm-0 ino=149160 scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tcontext=system_u:object_r:lib_t:s0 tclass=file


according to http://danwalsh.livejournal.com/6117.html?thread=23525 this message usually reveals a bug in the code.
I have used chcon -t textrel_shlib_t to get over it and the application starts and runs after that.

The license header is wrong, it should be just GPLv2+. The license header refers to the license of the compiled binary; even if the sources are under (GPLv2+ and LGPLv2+ and MIT and Public Domain and BSD) every license used is compatible with GPLv2+ which thus overrides everything.

Well, actually, it's not "wrong", its just "overly thorough". The license on the compiled binary is actually "GPLv2+ and LGPLv2+ and MIT and Public Domain and BSD". In reality, that simplifies down to meaning the exact same thing as "GPLv2+", but it doesn't make the original assertion incorrect.

I generally leave it up to maintainers on whether they wish to be overly verbose or not.

(In reply to comment #23)
> Well, actually, it's not "wrong", its just "overly thorough". The license on
> the compiled binary is actually "GPLv2+ and LGPLv2+ and MIT and Public Domain
> and BSD". In reality, that simplifies down to meaning the exact same thing as
> "GPLv2+", but it doesn't make the original assertion incorrect.
> 
> I generally leave it up to maintainers on whether they wish to be overly
> verbose or not.  

Right; it's more of an issue of clarity and terseness.

Ping Milos, Manuel?

There is an year of inactivity from submiter. Closing. If you decide to continue, please reopen.