Hi Petter, Thanks again for your patience with this review. On Fri, 28 Apr 2017 14:10:37 +0200 Petter wrote: > On Thu, 27 Apr 2017 20:47:54 -0500 > Eric Bavier wrote: > > > > > > + (inputs > > > > > + `(("qt" ,qt))) > > > > > > > > Does this package need all of qt, or could we use the component > > > > packages instead? > > > > > > I don't know. I'll do some experiments and see. > > > > OK, great. > > Fixed! > > Now it is: > (inputs > `(("qtbase" ,qtbase) > ("qtdeclarative" ,qtdeclarative) > ("qtgraphicaleffects" ,qtgraphicaleffects) > ("qtquickcontrols" ,qtquickcontrols))) Excellent. > > > > > > + (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? > > > > > > I can't answer this, hopefully someone else will chime in. > > > > I did a little more looking: The sources in app/qml seem to have > > license headers marked as GPLv3+, so we can mark it as such. > > I changed the GPL3 license GPL3+. > > The owner has replied in an issue about this. > > ---[Swordfish90 commented on Dec 1, 2015]--- > Hi @nadams810 and sorry for the late response. Since the project is split > into two different components, the idea behind the licensing was the > following: > > 1. All the files belonging to QMLTermWidget which is a separate library > forked by qtermwidget are licensed under GPL2 (as the original library). > 2. All the files belonging to CRT are licensed under GPL3. > > To be fair I'm not really an expert when it comes to licensing and if there > is something wrong with the following approach just let me know. > ---end-quote--- > > https://github.com/Swordfish90/cool-retro-term/issues/269 "gpl2" in the license field should be "gpl2+" according to qmltermwidget license headers. Could you also include a comment noting the component split? Nothing fancy, just "for qmltermwidget" or so next to gpl2+ and "for cool-retro-term" next to gpl3+. > > > > > > > > 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. > > Added. > > > > > 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." > > > > > > This font was removed Mar 13, 2016. > https://github.com/Swordfish90/cool-retro-term/commit/c505dfaa11166054686df7390cf04dba324ad869 I see. I must have been browsing while pinned at an older commit. No rationale given. Hmm. > > > > > 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. > > > > > > Let's see if someone can shine some light on this. > > > > > > > Leo pointed me at https://bugs.gnu.org/26588 which references > > https://www.gnu.org/distros/free-system-distribution-guidelines.html#non-functional-data . > > I think many of these fonts would need to be stripped out or > > substituted because their license prohibits commercial/non-commercial > > redistribution. > > Recipe is now removing the following from disk and GUI: > '("1977-apple2" > "1977-commodore-pet" > "1979-atari-400-800" > "1982-commodore64" > "1985-ibm-pc-vga") The code you've added to delete these fonts and adjust the source accordingly would be better in an origin 'snippet' so that `guix build -S cool-retro-term` does not include the fonts. > > Should more be removed? modern-monaco. It's a font from Apple which is specifically *not* freely distributable, but the ttf file is still floating around the internet, giving many projects the impression that it is. > > What do we do about the fonts without licensing info? I agree with Chris that we need to remove them. > > Also, I've added 3 patches from lxde/qtermwidget. The 2 with possible security > implications, and 1 memory leak (of those you listed). With the remaining two > you linked, memory leak and perfomance fix, git was unable to apply to the > current qmltermwidget codebase. Excellent. Thanks, `~Eric