Bug 383621 - Review Request: gnofract4d - Gnofract 4D is a Gnome-based program to draw fractals
Summary: Review Request: gnofract4d - Gnofract 4D is a Gnome-based program to draw fra...
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-11-15 00:31 UTC by Stewart Adam
Modified: 2007-11-30 22:12 UTC (History)
2 users (show)

Fixed In Version: 3.6-4.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-22 03:27:07 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Stewart Adam 2007-11-15 00:31:31 UTC
Spec URL: http://www.diffingo.com/downloads/diffingo-repo/review/gnofract4d.spec
SRPM URL: http://www.diffingo.com/downloads/diffingo-repo/review/gnofract4d-3.6-1.fc8.src.rpm
Description:
Gnofract 4D is a free, open source program which allows anyone to create
beautiful images called fractals. The images are automatically created by
the computer based on mathematical principles. These include the Mandelbrot
and Julia sets and many more. You don't need to do any math: you can explore
a universe of images just using a mouse.

rpmlint is silent on the SRPM but not in the binary RPM. 99% of the messages which appear are concerning a non-executable python script with shebang which I think is safe to ignore. The other two messages are:
gnofract4d.x86_64: W: file-not-utf8 /usr/share/gnome/help/gnofract4d/C/gnofract4d-manual.html
gnofract4d.x86_64: E: script-without-shebang

Again, I think both are safe to ignore considering it's only the HTML documentation file and the map file isn't a script anyways.

Comment 1 Mamoru TASAKA 2007-11-17 13:17:03 UTC
For 3.6-1:

* License
  - 3 files
------------------------------------------
lex.py
yacc.py
fract4dgui/FCTGen.py
------------------------------------------
    makes the license of this package LGPLv2+, not BSD.

* Redundant BuildRequires
  - Not a big issue, however some of BuildRequires are
    redundant
    * pkgconfig is needed by gtk2-devel
    * libpng-devel is needed by gtk2-devel

* desktop-file-install
  - build.log says %{buildroot}%{_datadir}/gnofract4d/gnofract4d.desktop
    does not exist.
  - Category "X-Fedora" is deprecated and should not be added.

* scriptlets
  http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
  - scrollkeeper-updates does not seem to be needed.
  - mime database must be updated
  - desktop database must be updated as installed desktop file
    contains MimeType key.

* %check
  - If some test program can be done, create %check section and write
    some test program with in the section (what is test.py for?)

* %files entry
  - By the way, the two lines
-------------------------------------------------
%dir %{python_sitearch}/fract4d/
%{python_sitearch}/fract4d/*
-------------------------------------------------
    can be replaced by one line
-------------------------------------------------
%{python_sitearch}/fract4d/
-------------------------------------------------

* About non-executable-script
  - Non executable script should not have shebangs.

Comment 2 Mamoru TASAKA 2007-11-17 13:18:36 UTC
By the way, I would appreciate it if you would review my review
request (bug 380081).

Comment 3 Stewart Adam 2007-11-17 17:58:14 UTC
I've looked at test.py, two tests fail every time.

One checks the .desktop file's "Version=" key and compares it to the program
version, but this is invalid as Version= is used to designate the freedesktop
standard version, not the program version so that test has been patched out.

The second one tests generating an AVI file from fractals frames. Since
Transcode isn't in the Fedora repositories that test has also been patched out.
Selecting Director from the GUI informs the user that transcode isn't installed,
no harm done.

SPEC: http://www.diffingo.com/downloads/diffingo-repo/review/gnofract4d.spec
SRPM:
http://www.diffingo.com/downloads/diffingo-repo/review/gnofract4d-3.6-2.fc8.src.rpm

Changes:
* Sat Nov 17 2007 Stewart Adam <s.adam at diffingo.com> - 3.6-2
- License is actually LGPLv2+ because of lex.py, yacc.py, FCTGen.py
- Add patch for test suite files since two tests are invalid
- Update MIME and desktop databases
- Use virtual X for GUI test suite
- Remove redundant BR
- Remove redundant entries from %%files
- Fix rpmlint's errors about executable files/shebangs

Comment 4 Mamoru TASAKA 2007-11-18 07:00:13 UTC
Well, rebuild failed at least on dist-f9, i386.
http://koji.fedoraproject.org/koji/taskinfo?taskID=246812

By the way:
If you want, you can try to rebuild your arbitrary srpm on koji by
$ koji build --scratch <target> <srpm_you_want_to_try>
Currently <target> can be either dist-f9, dist-f8-updates-candidate or
dist-fc7-updates-candidate.
When rebuild is successful, the result binary rpms and some logs are
put under
http://koji.fedoraproject.org/scratch/<your_FAS_name>/task_<taskID> .

Comment 5 Stewart Adam 2007-11-18 17:11:37 UTC
I didn't know that - Thanks for the tip!

Missing two BRs... Fixed in 3.6-3:
SPEC: http://www.diffingo.com/downloads/diffingo-repo/review/gnofract4d.spec
SRPM:
http://www.diffingo.com/downloads/diffingo-repo/review/gnofract4d-3.6-3.fc8.src.rpm

Koji build is here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=247177

Comment 6 Mamoru TASAKA 2007-11-19 12:51:15 UTC
Well, for 3.6-3:

* desktop-file-install
-----------------------------------------------------------
desktop-file-install --vendor fedora --delete-original \
        --dir $RPM_BUILD_ROOT%{_datadir}/applications \
        $RPM_BUILD_ROOT%{_datadir}/%{name}/%{name}.desktop
-----------------------------------------------------------
  - However the build.log shows:
-----------------------------------------------------------
   765  creating
/var/tmp/gnofract4d-3.6-3.fc9-root-kojibuilder/usr/share/applications
   766  copying gnofract4d.desktop ->
/var/tmp/gnofract4d-3.6-3.fc9-root-kojibuilder/usr/share/applications
   767  creating /var/tmp/gnofract4d-3.6-3.fc9-root-kojibuilder/usr/share/mime
..................................
   775  + desktop-file-install --vendor fedora --delete-original --dir
/var/tmp/gnofract4d-3.6-3.fc9-root-kojibuilder/usr/share/applications
/var/tmp/gnofract4d-3.6-3.fc9-root-kojibuilder/usr/share/gnofract4d/gnofract4d.desktop
-----------------------------------------------------------
    So at this stage, gnofract4d.desktop is installed under
    $RPM_BUILD_ROOT%_datadir/applications, not under
    $RPM_BUILD_ROOT%_datadir/%name .

* URL
  - Source0 URL must be
    http://downloads.sourceforge.net/.... , not 
    http://download.sourceforge.net/...

* Directory ownership issue
  - Well I don't know why I missed this issue on previous check, however
------------------------------------------------------------
[tasaka1@localhost ~]$ LANG=C rpm -qf /usr/share/gnofract4d/maps/
gnofract4d-3.6-3.fc9
[tasaka1@localhost ~]$ LANG=C rpm -qf /usr/share/gnofract4d/
file /usr/share/gnofract4d is not owned by any package
------------------------------------------------------------
    The %files entry %{_datadir}/%{name}/* must be replaced by
    %{_datadir}/%{name}/ .

Comment 7 Stewart Adam 2007-11-20 00:25:41 UTC
Good call on the directory ownership, I missed that too. I fixed the problems
you mentioned above, and the desktop file is now installed to the right dir:

SPEC: http://www.diffingo.com/downloads/diffingo-repo/review/gnofract4d.spec
SRPM:
http://www.diffingo.com/downloads/diffingo-repo/review/gnofract4d-3.6-4.fc8.src.rpm

Comment 8 Mamoru TASAKA 2007-11-20 07:39:38 UTC
Okay.

-------------------------------------------------------
   This package (gnofract4d) is APPROVED by me
-------------------------------------------------------

Comment 9 Stewart Adam 2007-11-20 22:13:52 UTC
Thanks for the review!
--
New Package CVS Request
=======================
Package Name: gnofract4d
Short Description: Gnofract 4D is a Gnome-based program to draw fractals
Owners: firewing
Branches: F-7 F-8
InitialCC: firewing
Cvsextras Commits: yes

Comment 10 Kevin Fenzi 2007-11-21 03:47:07 UTC
cvs done.

Comment 11 Fedora Update System 2007-11-22 03:26:57 UTC
gnofract4d-3.6-4.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.