Bug 628202

Summary: Review Request: gretl - A tool for econometric analysis
Product: [Fedora] Fedora Reporter: hannes <johannes.lips>
Component: Package ReviewAssignee: Peter Lemenkov <lemenkov>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, LeeMitchell.fedora, lemenkov, martin.gieseking, notting
Target Milestone: ---Flags: lemenkov: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-09-17 16:43:20 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description hannes 2010-08-28 15:31:32 UTC
Spec URL: http://hannes.fedorapeople.org/gretl.spec
SRPM URL: http://hannes.fedorapeople.org/gretl-1.9.1-1.fc13.src.rpm
Description:A cross-platform software package for econometric analysis, written in the C programming language.

Comment 1 Martin Gieseking 2010-09-06 16:21:42 UTC
*** Bug 580319 has been marked as a duplicate of this bug. ***

Comment 2 Martin Gieseking 2010-09-06 16:31:35 UTC
Hannes, could you please upload your files again? The above links currently don't work.

Comment 3 hannes 2010-09-06 18:54:56 UTC
Sorry I changed the folder structure:
Spec URL: http://hannes.fedorapeople.org/gretl/gretl.spec
SRPM URL: http://hannes.fedorapeople.org/gretl/gretl-1.9.1-1.fc13.src.rpm

Comment 4 Martin Gieseking 2010-09-06 19:33:44 UTC
Here are a couple of quick comments:

- adapt the URL in Source0 according to
  http://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

- the license seems to be GPLv3+

- add a comment to the spec file telling what the patch does

- the %description lines should not exceed 80 characters

- see http://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files
  how to install locale files

- development files (.h, .pc, .so if corresponding .so.* is present, ...) must 
  go to a -devel subpackage, see
  https://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages

- you must call ldconfig in %post and %postun, see
  https://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries

- use macro %{_datadir} rather than %{_datarootdir}

- add files COPYING, ChangeLog, CompatLog, README, README.audio with %doc

Comment 5 hannes 2010-09-09 07:20:45 UTC
Spec URL: http://hannes.fedorapeople.org/gretl/gretl.spec
SRPM URL: http://hannes.fedorapeople.org/gretl/gretl-1.9.1-2.fc13.src.rpm

Tried to fix all the mistakes.

Comment 6 Peter Lemenkov 2010-09-09 11:44:02 UTC
(In reply to comment #5)
> Spec URL: http://hannes.fedorapeople.org/gretl/gretl.spec
> SRPM URL: http://hannes.fedorapeople.org/gretl/gretl-1.9.1-2.fc13.src.rpm
> 
> Tried to fix all the mistakes.

Some more notes:

* %{_libdir}/libgretl*.la <-- *.la files must be removed.
* Unowned directories: %{_datadir}/%{name} and %{_includedir}/%{name}

Comment 7 hannes 2010-09-09 13:49:10 UTC
Spec URL: http://hannes.fedorapeople.org/gretl/gretl.spec
SRPM URL: http://hannes.fedorapeople.org/gretl/gretl-1.9.1-3.fc13.src.rpm

Ok, removed the static lib and changed the structure of the %files section.

Comment 8 Peter Lemenkov 2010-09-09 13:54:54 UTC
I'll review it

Comment 9 Peter Lemenkov 2010-09-09 14:03:49 UTC
Koji scratchbuild for F-13 (currently in progress):

http://koji.fedoraproject.org/koji/taskinfo?taskID=2457450

Comment 10 Peter Lemenkov 2010-09-10 12:13:15 UTC
REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

- rpmling output isn't silent:

work ~: rpmlint Desktop/gretl-*
gretl.i686: W: no-manual-page-for-binary gretl_x11
gretl.i686: W: no-manual-page-for-binary gretlcli
gretl.i686: E: invalid-desktopfile /usr/share/applications/gretl.desktop value "Application;Science;Econometrics" for string list key "Categories" in group "Desktop Entry" does not have a semicolon (';') as trailing character
gretl-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/gretl-1.9.1/plugin/heckit.c
gretl-devel.i686: W: no-documentation
3 packages and 0 specfiles checked; 1 errors, 4 warnings.
work ~:

The two 'no-manual-page-for-binary' and one 'no-documentation' messages may be omitted, while the rest two should be fixed. 

+ The package is named according to the Package Naming Guidelines .
+ The spec file name matches the base package %{name}, in the format %{name}.spec.

- The package does not meet the Packaging Guidelines.

-- Lost of *.la files in %{_libdir}/gretl-gtk2
-- Duplicated COPYING file in %{_datadir}/%{name}
-- Empty directory %{_datadir}/%{name}/doc - looks like a leftover.
-- Bundled font files in %{_datadir}/%{name}/fonts
-- Missing Requires: gtksourceview (owner of %{_datadir}/gtksourceview-1.0)

+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.

-/+ The License field in the package spec file matches the actual license. (GPLv3+) but does not reflect licensing conditions for 'cephes', 'minpack' and some plugins. Please add them (seems to be a BSD for minpack, MIT for plugin/mpack, but IANAL).

+ The file, containing the text of the license(s) for the package, is included in %doc
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package matches the upstream source, as provided in the spec URL:

Sulaco ~/rpmbuild/SOURCES: sha256sum gretl-1.9.1.tar.bz2*
b46916828132cc6955ed20cf4c9816d17cae3f692368a245d56f8e55f3efda39  gretl-1.9.1.tar.bz2
b46916828132cc6955ed20cf4c9816d17cae3f692368a245d56f8e55f3efda39  gretl-1.9.1.tar.bz2.1
Sulaco ~/rpmbuild/SOURCES:

+ The package successfully compiles and builds into binary rpms on at least one primary architecture (see koji link above)
+ All build dependencies are listed in BuildRequires.
+ The spec file handles locales properly.
+ The package calls ldconfig in %post and %postun.

+/- The package does NOT bundle copies of system libraries. In fact I'm not quite sure because 'plugin/zipunzip' contains portions of zlib - please unvestigate this.

0 The package isn't designed to be relocatable
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
+ Header files placed in a -devel package.
0 No static libraries (*.a)
+ The library files that end in .so (without suffix) placed in a -devel package.
+ devel sub-package requires the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} 

- The package must NOT contain any .la libtool archives, these must be removed in the spec if they are built. See my upper notes.

-/+ The package includes a %{name}.desktop file, and that file MUST be properly installed with desktop-file-install in the %install section. Unfortunately it is simply copied with 'install' utility, so, please use desktop-file-validate (adds additional BuildRequires: desktop-file-utils) to check that everything is ok

+ The package does not own files or directories already owned by other packages.
+ All filenames in rpm packages are valid UTF-8.

That's all so far. Please comment/fix issues, mentioned above, and I'll continue.

Comment 11 hannes 2010-09-10 17:22:37 UTC
Ok I hope I have fixed most of the errors. I have wrote an e-mail upstream concerning the questionable files in the plugin directory. Also I asked what the appropriate license is. I added the license where I was sure.
Put it into Education because I really wasn't sure which one to choose.

Spec URL: http://hannes.fedorapeople.org/gretl/gretl.spec
SRPM URL: http://hannes.fedorapeople.org/gretl/gretl-1.9.1-4.fc13.src.rpm


Greetings

Comment 12 Peter Lemenkov 2010-09-10 18:09:11 UTC
(In reply to comment #11)
> Ok I hope I have fixed most of the errors. I have wrote an e-mail upstream
> concerning the questionable files in the plugin directory. Also I asked what
> the appropriate license is. I added the license where I was sure.
> Put it into Education because I really wasn't sure which one to choose.
> 
> Spec URL: http://hannes.fedorapeople.org/gretl/gretl.spec
> SRPM URL: http://hannes.fedorapeople.org/gretl/gretl-1.9.1-4.fc13.src.rpm
> 
> 
> Greetings

Ok, good.

Regarding fonts - I'm not sure that the proper way is simply to remove them w/o adding some new Requires. Please, ensure that fonts removal doesn't hurt usability. That's the last request from me.

Comment 13 Martin Gieseking 2010-09-10 18:41:50 UTC
Sorry for chiming in. Just a minor additional note: The Group of the devel package should be "Development/Libraries".

Comment 14 hannes 2010-09-13 08:05:09 UTC
Ok, asked upstream for clarification regarding the license of the plugins and he sent me a list where he lists everything. Added this list - gretl_plugins.txt - to the package. Only questionable plugin might be lad.c but I wasn't able to remove it properly. Any advice on this? Upstream already removed it in their cvs.

I symlinked system fonts and added BuildRequires and Requires for those fonts. Also asked upstream if they are necessary or if we could perhaps just drop them but I am waiting for a response.

Spec URL: http://hannes.fedorapeople.org/gretl/gretl.spec
SRPM URL: http://hannes.fedorapeople.org/gretl/gretl-1.9.1-5.fc13.src.rpm

Comment 15 hannes 2010-09-16 05:31:05 UTC
Removed lad.c but the symlinks seem to be necessary as a workaround for gnuplot.
 
Spec URL: http://hannes.fedorapeople.org/gretl/gretl.spec
SRPM URL: http://hannes.fedorapeople.org/gretl/gretl-1.9.1-6.fc13.src.rpm


rpmlint gretl-1.9.1-6.fc13.x86_64.rpm 
gretl.x86_64: W: dangling-symlink /usr/share/gretl/fonts/FreeSans.ttf /usr/share/fonts/gnu-free/FreeSans.ttf
gretl.x86_64: W: dangling-symlink /usr/share/gretl/fonts/Vera.ttf /usr/share/fonts/bitstream-vera/Vera.ttf
gretl.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/gretl-1.9.1/gretl_plugins.txt
gretl.x86_64: W: dangling-symlink /usr/share/gretl/fonts/VeraMono.ttf /usr/share/fonts/bitstream-vera/VeraMono.ttf
gretl.x86_64: W: no-manual-page-for-binary gretl_x11
gretl.x86_64: W: no-manual-page-for-binary gretlcli
1 packages and 0 specfiles checked; 0 errors, 6 warnings.

Comment 16 Peter Lemenkov 2010-09-16 06:27:44 UTC
Ok, good. All issues found during review were addressed and the package builds fine in Koji:

http://webplanet.ru/knowhow/security/nadyafrank/2010/09/16/watch.html

So this package is 

APPROVED.

Comment 17 hannes 2010-09-16 06:36:08 UTC
New Package SCM Request
=======================
Package Name: gretl
Short Description: A tool for econometric analysis
Owners: hannes
Branches: f13 f14
InitialCC:

Comment 18 Peter Lemenkov 2010-09-16 07:16:56 UTC
LOL. I just saw that I submitted wrong link instead of one to koji build:)
Anyway it builds fine.

Comment 19 Peter Lemenkov 2010-09-16 07:20:49 UTC
Proper

http://koji.fedoraproject.org/koji/taskinfo?taskID=2470695

Comment 20 Kevin Fenzi 2010-09-16 23:00:53 UTC
Git done (by process-git-requests).

Comment 21 hannes 2010-09-17 16:44:28 UTC
build in rawhide:
http://koji.fedoraproject.org/koji/packageinfo?packageID=10925