Bug 484386

Summary: Review Request: gri - A language for scientific illustration
Product: [Fedora] Fedora Reporter: D Haley <mycae>
Component: Package ReviewAssignee: Orion Poplawski <orion>
Status: CLOSED CANTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, kelley.dan, notting, orion, susi.lehtola
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-12-07 15:52:47 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 201449    

Description D Haley 2009-02-06 10:33:52 EST
SPEC URL: http://dhd.selfip.com/427e/gri.spec
SRPM URL: http://dhd.selfip.com/427e/gri-2.12.18-1.src.rpm 

Summary:
Gri is a language for scientific graphics programming.  It is a
command-driven application, as opposed to a click/point application.
It is analogous to latex, and shares the property that extensive power
is the reward for tolerating a modest learning curve.  Gri output is in
industry-standard PostScript, suitable for incorporation in documents
prepared by various text processors.

Koji Build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1109441

rpmlint output:
$ rpmlint -iv ../RPMS/i386/gri-2.12.18-1.i386.rpm ../SRPMS/gri-2.12.18-1.src.rpm ./gri.spec 
gri.i386: I: checking
gri.src: I: checking
gri.src:39: W: rpm-buildroot-usage %build make DESTDIR=$RPM_BUILD_ROOT libdir=$RPM_BUILD_ROOT/usr/share/gri
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it will
break short circuiting.

./gri.spec:39: W: rpm-buildroot-usage %build make DESTDIR=$RPM_BUILD_ROOT libdir=$RPM_BUILD_ROOT/usr/share/gri
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it will
break short circuiting.

2 packages and 1 specfiles checked; 0 errors, 2 warnings.


I think the warning about touching RPM_BUILD_ROOT in the build section is not a problem, as it is not being modified, just used to pass options to the build.
Comment 1 Susi Lehtola 2009-02-08 06:17:32 EST
Just a few comments:

- License is GPLv2+, not GPLv2.

- Remove Requires: readline, this is automatically picked up.

- Remove pushd and popd from setup, you don't need them.

- Change make command to
make %{?_smp_mflags}
as this works fine. The paths are already set by %configure.

- Replace "rm -Rf" with "rm -rf" to be consistent.

- Absolute paths in %files need to be replaced with %{_bindir}, %{_datadir} and so on.

- Changelog is way too long, IMHO you may remove everything that is before 2006.
Comment 2 Susi Lehtola 2009-02-08 06:20:18 EST
Also, since the package includes Emacs files, you need to read
http://fedoraproject.org/wiki/Packaging/Emacs

You'll have to branch the emacs script in its own subpackage, as gri probably works without it.
Comment 3 D Haley 2009-02-10 04:06:40 EST
SPEC URL: http://dhd.selfip.com/427e/gri-2.spec
SRPM URL: http://dhd.selfip.com/427e/gri-2.12.18-2.src.rpm 

rpmlint output:
$ rpmlint ../SRPMS/gri-2.12.18-2.fc10.src.rpm ../RPMS/i386/gri-2.12.18-2.fc10.i386.rpm  gri.spec 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.


Koji Build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1116786

Changelog:
* Tue Feb 10 2009 D Haley  <mycae@yahoo.com> 2.12.18-2
- Cleared many old changelog entries by D, Kelly & T Powers (year<=2005)
- Drop readline buildreq.
- Fix lic. GPLv2 to GPLv2+
- Create emacs subpackages
- Fix release line to include dist macro



>License is GPLv2+, not GPLv2.
Done.

>Remove Requires: readline, this is automatically picked up.
Done.

>Remove pushd and popd from setup, you don't need them.
Done.

>Change make command to
make %{?_smp_mflags}
as this works fine. The paths are already set by %configure.
Done.

>Replace "rm -Rf" with "rm -rf" to be consistent.
Done.

>Absolute paths in %files need to be replaced with %{_bindir}, %{_datadir} and
so on.
Whoops. Fixed.

>Changelog is way too long, IMHO you may remove everything that is before
2006.
I removed all but the last entry before myself, just to make it clear that this is based upon someone else's work.

>You'll have to branch the emacs script in its own subpackage, as gri probably
works without it.
Created subpackages emacs-%{name} and emacs-%{name}-el. I am not an emacs user, (brought up on vi(m)) so this is based upon reading refs [1-3]. So there may be a mistake in my understanding here.

[1] http://chitlesh.fedorapeople.org/RPMS/dinotrace.spec
[2] http://fedoraproject.org/wiki/Packaging/Emacs 
[3] http://en.wikipedia.org/wiki/Emacs_lisp#Source_code
Comment 4 D Haley 2009-02-10 04:09:21 EST
Correct SRPM url:
SRPM URL: http://dhd.selfip.com/427e/gri-2.12.18-2.fc10.src.rpm 

(The other one now links to the same file, but has the wrong name).
Comment 5 Jason Tibbitts 2009-06-04 00:21:27 EDT
This fails to build.  Here's a scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1392985
Comment 6 D Haley 2009-06-04 19:56:23 EDT
That build looked OK to me, am I missing something, or were you referring to my other package: Bug 481355 ?
Comment 7 Jason Tibbitts 2009-06-04 20:12:33 EDT
Weird; it does look OK.  I can't really explain what happened; I guess I shouldn't do builds late at night.  Please accept my apologies; I'll try to look at this soon.
Comment 8 Jason Tibbitts 2009-07-10 20:29:33 EDT
Well, for some definition of "soon".  My time available for reviews comes infrequently and at random intervals.  Anyway, I do have some time now.  So, some questions and comments:

The spec file on your web site seems to be out of date compared to what's in the src.rpm in comment 4.

My understanding is that the emacs files work for both emacs and xemacs.  Did you also intend to provide an xemacs package?  (I know the emacs/xemacs thing is rather insane, but that's just the way it is.  If you package for one and not the other, you can probably expect some bug reports about it.)  Yes, that means four packages with one file apiece and another block of macros.  If you only intend to build for Fedora, you can make them noarch, though.

The package includes a test suite in doc/tst_suite; is there any reason not to run it in a %check section?

Note that the emacs files are GPLv2+, not GPLv2.  The source code actually contradicts the README file here.  This package in general seems to be a bit lax about the license version in use.  (license.txt doesn't specify a version, README does but says v2 only for the emacs mode while gri-mode.el explicitly says v2+.)  Always trust what's in the source code, but do ask upstream to be clearer about whether they intended GPLv2 only or GPLv2+.

rpmlint says:
  gri.src: W: mixed-use-of-spaces-and-tabs
   (spaces: line 92, tab: line 28)
I don't particularly care; fix this if you like.

  emacs-gri.x86_64: W: no-documentation
  emacs-gri-el.x86_64: W: no-documentation
These are fine.

I note you're not using the %dist tag on this pacakge.  It's not mandatory, but I do need to ask if you understand the issues that occur when you don't use it and the procedure for making sure that you keep proper ordering between release branches.

Your manual Requires: readline should be unnecessary.  rpm correctly finds the dependency on libreadline.

Your scriptlets should conform to https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Texinfo unless you have some reason why that doesn't work.
Comment 9 D Haley 2009-07-12 01:16:02 EDT
SPEC URL: http://dhd.selfip.com/427e/gri-3.spec
SRPM URL: http://dhd.selfip.com/427e/gri-2.12.18-3.fc10.src.rpm

Koji builds:
F10: http://koji.fedoraproject.org/koji/taskinfo?taskID=1468583
F11: http://koji.fedoraproject.org/koji/taskinfo?taskID=1468584

RPMLint:
======
$ rpmlint -i gri.spec ../SRPMS/gri-2.12.18-3.fc10.src.rpm ../RPMS/i386/gri-*3.fc10* ../RPMS/i386/*-gri*3.fc10* && sudo rpm -e gri && sudo rpm -i ../RPMS/i386/gri-2.12.18-3.fc10.i386.rpm && rpmlint gri
emacs-gri.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

emacs-gri-el.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

xemacs-gri.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

xemacs-gri-el.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

7 packages and 1 specfiles checked; 0 errors, 4 warnings.
[sudo] password for makerpm: 
^[[A^[[A^[[B1 packages and 0 specfiles checked; 0 errors, 0 warnings.
======

>Well, for some definition of "soon".  
I don't think we are on a tight schedule :)

>Your manual Requires: readline should be unnecessary.  rpm correctly finds the
>dependency on libreadline.
Fixed

>I note you're not using the %dist tag on this pacakge.  It's not mandatory, but
>I do need to ask if you understand the issues that occur when you don't use it
>and the procedure for making sure that you keep proper ordering between release
>branches.

This was the out-of-date spec bit. I must have forgotten to rebuild before uploading. Anyway, this is therefore no longer an issue.

>The package includes a test suite in doc/tst_suite; is there any reason not to
run it in a %check section?

Their test suite doesn't actually have any kind of test target, in contradiction to what is stated in the manual (http://gri.sourceforge.net/gridoc/html/TestSuite.html). The manual says one exists and can be invoked with "make test", but make test is not a valid target for any makefile. There is no clear way to verify the functioning of their test suite in an automated fashion, AFAIK.

$ for i in `find ./ -name Makefile`; do pushd . ; cd $(dirname $i); make test; popd; done
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18
~/rpmbuild/BUILD/gri-2.12.18 ~/rpmbuild/BUILD/gri-2.12.18
make: *** No rule to make target `test'.  Stop.
~/rpmbuild/BUILD/gri-2.12.18

>  gri.src: W: mixed-use-of-spaces-and-tabs
Fixed.

> This package in general seems to be a bit lax about the license version in use. 
Submitted upstream as a bug (https://sourceforge.net/tracker/?func=detail&aid=2820229&group_id=5511&atid=105511)

Actually I am going to revert the package licence back to GPLv2. The licence on their site, and in their docs is GPLv2. (http://gri.sourceforge.net/gridoc/html/License.html and doc/gri.texi) GPLv2+ gives you the option of using a later version, or GPLv2. Therefore, mixing this with the GPLv2+ el files, means that the package as a whole must be distributed under 2 only, as far as I can see. 

>Your scriptlets should conform to
>https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Texinfo unless you
>have some reason why that doesn't work.  
Fixed.
Comment 10 Dan Kelley 2009-07-13 13:49:10 EDT
As the author of gri, I would be happy to add licensing text as required to my source (and other) files.  Actually, I was surprised to hear of this issue today, since Debian (which is keen on licenses) accepted Gri quite a long time ago.

It would be very helpful if someone could suggest a URL that gave me clear instructions for a license that would satisfy both Fedora and Debian.

My guess is that I share with other developers a neutral opinion regarding the choices of license.  All I want is to share my labour.

Dan.
Comment 11 Jason Tibbitts 2009-07-13 14:05:33 EDT
Please note that there isn't any question of whether the license is acceptable.  The question is whether it's restricted to "GPLv2 only" or whether it's "GPLv2 or later".  This is important because it is not uncommon these days for packages to relicense to GPLv3+ (for example, this just happened with libreadline) and we need to know at a glance whether this is an issue for a particular program.

We get the "Debian didn't care" argument pretty often, but the simple fact is that Fedora cares more about this kind of thing.  We're just paying attention to details here.  Since there's doubt, it seems safer to assume that the bulk of the software is "GPLv2 only" and cannot be linked against GPLv3 code, but if that's not what you intend then please do feel free to clarify.  The GPL itself tells you how to be completely clear about this, by putting proper license blocks in each source file and using well-defined terminology when referring to the license.  This information is down near the end of the GPL text, at "How to Apply These Terms to Your New Programs".
Comment 12 D Haley 2009-07-13 20:50:26 EDT
Just to make sure everyone is on the same page, please read the upstream bug at the gri tracker:
http://sourceforge.net/tracker/?func=detail&atid=105511&aid=2820229&group_id=5511

The developer (Dan) has stated that he is keen to help us, but my interpretation of the situation is that Dan is not certain about the best licence to use to satisfy both Debian and Fedora needs, and what needs to be done once the author has chosen a licence.

I would tentatively suggest that GPLv2+ (GPL version 2 or any later) would be the best option, as this allows both debian and fedora to use the author's work, and makes packaging a breeze, if the author wants to relicence. 

If not, GPLv2 is OK, but this also needs to be listed in the source code boiler-plate, as specified in the how to. Using GPLv2 makes packaging a little bit harder, as we cannot link to GPLv3 code.

> This information is down near the end of the GPL text, at "How to
> Apply These Terms to Your New Programs". 

As you say, All that needs to happen is the source files need to comply with the GPL how-to (http://www.gnu.org/licenses/gpl-howto.html)
Comment 13 Jason Tibbitts 2009-07-13 22:54:08 EDT
And I just want to reiterate that the issue is not finding a license which satisfies Fedora, because either of the possibilities satisfies Fedora just fine.  Fedora only asks for clarification.
Comment 14 Dan Kelley 2009-07-14 06:48:04 EDT
Do I infer correctly, from the comments of D Haley, that declaring it GPLv2+ in a few spots can let me make the change without altering each and every source file?

That would be wonderful, because altering the source files has a bit of a negative effect.  (The modification date is a useful thing, in indicating at a glance which parts of a code have been reworked, and which worked from the start.  I'm a scientist, and I trust 1960s fortran subroutines more than months-old c++.)

I would love to get a clear statement on what I should do.  I am hoping, from D Haley's comment, that I may satisfy Fedora's needs by modifying just a few files ... but which ones?  (I have modified my gri.spec, but I don't know if that's being used by Fedora, actually.)

PS. I know I'm being a nuisance on this, but I do think it is better for me to use my time on the code, rather than on reading documents about licenses.

PPS. Gri is quit old, which explains why I say "GPL" in some places ... that's all there was, once upon a time!

Dan
Comment 15 D Haley 2009-07-14 12:39:03 EDT
Hi Dan,

> I do think it is better for me to
> use my time on the code, rather than on reading documents about licenses.

Fair enough. So I have put together a quick and dirty script to make life a little easier:
http://dhd.selfip.com/427e/gri-relic.tar.gz

The script allows you to choose from GPL2,GPL2+,GPL3 or GPL3+, and will modify the source files, without changing the timestamps (Not sure this is a 100% good idea. This may affect cvs behaviour, you will have to force a commit of all .cc and .hh files with the -f flag, see CVS manpage). It  replaces the COPYING file and license.txt

I also include a quick GPL intro as a printf statment in gri.cc, as well as modifying the README-linux-redhat and README-SunOS5 files, depending upon licence version. I don't touch the .el file -- you may need to ask your co-author about that licence and modify (just change the GPL version number in the .el file).

The patches apply cleanly to the release version, not the CVS. But the patches are only very minor. Changes should be obvious from looking at them.

To use the script, untar it into your gri folder
$tar -zxvf PATH_TO/gri-relic.tar.gz
$./relic/gri-license.sh

Each time you run the script it will prepend the GPL boilerplate, so take care only to run it once :) 

As always please check the script to make sure I am not doing anything particularly stupid that may damage your build or system. As I have limited time at the moment, I have not tested this very much, so your mileage will vary.
Comment 16 Dan Kelley 2009-07-14 12:59:02 EDT
I think I've done most of this, in the CVS.

My remaining question is whether it is mandatory that I include the license in the cc and hh files.
Comment 17 D Haley 2009-07-14 20:46:01 EDT
> My remaining question is whether it is mandatory that I include the license in
> the cc and hh files.  

Yes, it is part of the licensing -- if you are worried about timestamps, the script may help...
Comment 18 Dan Kelley 2009-07-15 08:27:47 EDT
Thanks very much for the script.  I've used it and committed the results to CVS.  (The patch didn't work.)

I do not have access to a machine with rpmlint, so I can't test whether things are better now, but I hope so!

PS. my debian pal will probably be back in a week or so, so that sets a timescale for any possible tarball release ... we're in CVS land until then.
Comment 19 D Haley 2009-07-18 22:41:21 EDT
Dan Kelley released a new gri with GPLv3+ licence:

SPEC URL: http://dhd.selfip.com/427e/gri-2.12.19-1.spec
SRPM URL: http://dhd.selfip.com/427e/gri-2.12.19-1.fc10.src.rpm

Changelog:
* Sun Jul 19 2009 D Haley <mycae(a!t)yahoo.com> 2.12.19-1
- Update to gri-2.12.19

Koji:
F10: http://koji.fedoraproject.org/koji/taskinfo?taskID=1484728
F11: http://koji.fedoraproject.org/koji/taskinfo?taskID=1484723

Rpmlint:
$ rpmlint -i gri.spec ../SRPMS/gri-2.12.19-1.fc10.src.rpm ../RPMS/i386/gri-*19-1.fc10* ../RPMS/i386/*-gri-*19-1.fc10* && sudo rpm -e gri && sudo rpm -i ../RPMS/i386/gri-2.12.19-1.fc10.i386.rpm && rpmlint gri
emacs-gri.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

emacs-gri-el.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

xemacs-gri.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

xemacs-gri-el.i386: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

7 packages and 1 specfiles checked; 0 errors, 4 warnings.
[sudo] password for makerpm: 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 20 Orion Poplawski 2009-10-29 17:56:45 EDT
I'll take the review.  First comments:

Let's change all the %define statements to %global.

I'm a little leery about the defaults if pkg-config emacs doesn't work.  What's the motivation?  Perhaps other conditionals would be more appropriate?
Comment 21 Bug Zapper 2009-11-18 06:00:12 EST
This message is a reminder that Fedora 10 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 10.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '10'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 10's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 10 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 22 Mamoru TASAKA 2009-11-18 10:13:59 EST
(Changing the version to rawhide)
Comment 23 D Haley 2010-02-19 16:13:30 EST
Hi, 

Sorry about the stall here. I have only just recently become less busy, and have to obtain a replacement fedora machine (mine went belly-up).

Meanwhile I have uploaded the same spec to here:
http://mycae.fedorapeople.org/SPECS/gri.spec

I'll try to get a new machine and build in a few weeks.
Comment 24 Orion Poplawski 2010-11-07 00:35:55 EDT
D - are you still interested in pursuing this package?