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...
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
Depends On:
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)

Clone Of:
Last Closed: 2007-11-22 03:27:07 UTC
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
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
    makes the license of this package LGPLv2+, not BSD.

* Redundant BuildRequires
  - Not a big issue, however some of BuildRequires are
    * 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
  - 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/
    can be replaced by one line

* 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

* 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.

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
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

Koji build is here:

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

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

* 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/
[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

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

   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.