Petter,

Thanks for the patch!  And Chris, thanks for the review.  

A few notes from my look through:

On Mon, 24 Apr 2017 14:30:26 +0200
Petter <petter@mykolab.ch> wrote:

> 
> * gnu/packages/terminals.scm (cool-retro-term): New variable.
> ---
>  gnu/packages/terminals.scm | 62
>   ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62
>   insertions(+)
> 
> diff --git a/gnu/packages/terminals.scm b/gnu/packages/terminals.scm
> index 979278a53..e53062b16 100644
> --- a/gnu/packages/terminals.scm
> +++ b/gnu/packages/terminals.scm
> @@ -7,6 +7,7 @@
>  ;;; Copyright © 2016, 2017 José Miguel Sánchez García
> <jmi2k@openmailbox.org> ;;; Copyright © 2017 Tobias Geerinckx-Rice
> <me@tobias.gr> ;;; Copyright © 2017 Ricardo Wurmus <rekado@elephly.net>
> +;;; Copyright © 2017 Petter <petter@mykolab.ch>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -49,6 +50,7 @@
>    #:use-module (gnu packages xdisorg)
>    #:use-module (gnu packages xml)
>    #:use-module (gnu packages docbook)
> +  #:use-module (gnu packages qt)
>    #:use-module (srfi srfi-26))
>  
>  (define-public tilda
> @@ -433,3 +435,63 @@ its embedding program should provide it to draw on
> its behalf.  It avoids calling @code{malloc} during normal running
> state, allowing it to be used in embedded kernel situations.")
>      (license license:expat)))
> +
> +(define-public cool-retro-term
> +  (let ((commit "e48719fa44e5307df71dbd0fad234f8a6a53f863")
> +        (revision "1"))
> +  (package
> +    (name "cool-retro-term")
> +    (version (string-append "1.0.0-" revision "." (string-take commit
> 7)))
> +    (source (origin
> +              (method git-fetch)
> +              (modules '((guix build utils)))
> +              (file-name (string-append name "-" version "-checkout"))
> +              (uri (git-reference
> +                    (url (string-append
> "https://github.com/Swordfish90/" name))
> +                    (commit commit)
> +                    (recursive? #t)))

The 'qmltermwidget' project that's bundled here is a fork of LXDE's
'qtermwidget' project, but it seems to be missing at least one patch
from qtermwidget that might have security implications.  E.g.:

https://github.com/lxde/qtermwidget/commit/1727d921b2d99721b9a8493b15defc8f838baedd
https://github.com/lxde/qtermwidget/commit/f9a7d4ac5de5224c15f5f9184525f81fdcc8c23b

As well as a couple memory leaks:

https://github.com/lxde/qtermwidget/commit/08628fda19128b75248548357e416bc373f14f91
https://github.com/lxde/qtermwidget/commit/ef451424d774feb23b42bdcda56ec09d1cc96e2a

And performance improvements:

https://github.com/lxde/qtermwidget/commit/3ce71a3650d93ff7ca10274c1b3056c7c7494c61

Could you ping the developer about porting some of these fixes to their
fork?  I think we'd want to create a local patch for at least the first
commit.  The others could wait for a new release.

> +    (inputs
> +     `(("qt" ,qt)))

Does this package need all of qt, or could we use the component
packages instead?

> +    (synopsis "Terminal emulator mimicking old cathode ray tube (CRT)
> +screens")

I'd like to shorten this to "Terminal emulator"

> +    (description
> +     "Cool-retro-term (crt) is a terminal emulator which mimics the
> look and +feel of the old cathode ray tube (CRT) screens.  It has been
> designed to be +eye-candy, customizable, and reasonably lightweight.")
> +    (home-page "https://github.com/Swordfish90/cool-retro-term")
> +    (license (list license:gpl2 license:gpl3)))))

The license seems unclear: 1) there is both a gpl-2.txt and gpl-3.txt
in $topsrcdir, but no mention of which actually applies in any of the
source files, 2) $topsrcdir/packaging/debian/copyright claims GPL-3,
while 4) $topsrcdir/packaging/rpm/cool-retro-term.spec and
$topsrcdir/packaging/appdata/cool-retro-term.appdata.xml both claim
GPL-3.0+, and 5) the qmltermwidget component appears to be under
gpl2+.  IANAL but this suggests license:gpl2+ to me.  Does that seem
right?

It would not hurt to also list the SIL license, for
$topsrcdir/app/qml/fonts/modern-hermit, and the X11 license for
fonts/modern-pro-font-win-tweaked and fonts/modern-proggy-tiny.

The fonts/modern-envy-code-r font probably needs to be stripped; it's
license is non-free; from 'Read Me.txt':

  "Free to use but redistribution prohibited."

and the fonts/1977-apple2, fonts/1977-commodore-pet, probably also needs
to be stripped as non-free; from 'FreeLicense.txt':

  "1. The User may not sell copies of the Software for a fee."

The fonts/1979-atari-400-800 font seems to be non-free license, which is
not mentioned in the 'ReadMe.rtf' but only on the WayBack Machine at
https://web-beta.archive.org/web/20080907111008/http://www2.bitstream.net/~marksim/atarimac/fonts.html :

  "These fonts are freeware and may not be offered for sale seperately
  or as part of a collection."

The fonts/1982-commodore64 font is licensed under non-free terms; see
fonts/1982-commodore64/license.txt

The only mention of a license for fonts/1985-ibm-pc-vga is at
http://www.dafont.com/font-comment.php?file=perfect_dos_vga_437 where
the author says "The license is this: this font is free to use in
whatever you want.", but that probably "doesn't count".

I could not find license info for a few of the other fonts.

OTOH I recall a discussion on IRC recently about fonts embedded in
packages being treated as non-functional data, and thus OK from a FSDG
perspective.  But I would want verification on that.

Sorry for the dump.  I'd like to work these things out; it looks like a
fun terminal to use.

`~Eric