Bug 333921 - Review Request: xgrav - A simple physics simulation for a large number of particles
Review Request: xgrav - A simple physics simulation for a large number of par...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-16 07:54 EDT by Gwyn Ciesla
Modified: 2007-11-30 17:12 EST (History)
4 users (show)

See Also:
Fixed In Version: 1.2.0-4.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-10-24 03:17:24 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
pertusus: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Gwyn Ciesla 2007-10-16 07:54:14 EDT
Spec URL: http://zanoni.jcomserv.net/fedora/xgrav/xgrav.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/xgrav/xgrav-1.2.0-1.fc7.src.rpm
Description: X-Grav simulates the effect of gravity, collisions, heat dissipation and a simple chemical reaction. The simulation is in no way meant to be
realistic but rather a toy with which you can create stars, planets and even simple solar systems.
Comment 1 Patrice Dumas 2007-10-16 08:53:24 EDT
Instead of patching the Makefile to add RPM_OPT_FLAGS,
I suggest overwriting LINUX_CFLAGS, like

make LINUX_CFLAGS="-c $RPM_OPT_FLAGS `pkg-config --cflags sdl` -DWITH_ROOTWINDOW"

You could also override LINUX_LDFLAGS with
LINUX_LDFLAGS="$RPM_OPT_FLAGS `pkg-config --libs sdl` -lGL `pkg-config --libs
x11` -lm"

zlib-devel isn't directly used, so if you override the LDFLAGS
as I suggest above you can remove that dependency.

Please add a comment stating where the png file comes
from.

In the .desktop file you don't need to add the .png to
the icon name (it is a suggestion).

Also it would be nice to honor rpm macros in the desktop file
and therefore do something like
sed 's;/usr/share;%_datadir;' %{SOURCE1} > xgrav.desktop
and then run desktop-file-install on xgrav.desktop.

To keep timestamps, you should use install -p in
install -p -m 644 example* %{buildroot}%{_datadir}/xgrav

You should use %{buildroot} or $RPM_BUILD_ROOT but not both.
Comment 3 Patrice Dumas 2007-10-16 17:01:14 EDT
The %build section is a bit broken. There are 2 make invocations
and LINUX_LDFLAGS is not used. I think that it should be:

make LINUX_CFLAGS="-c $RPM_OPT_FLAGS `pkg-config --cflags sdl` -DWITH_ROOTWINDOW" 
LINUX_LDFLAGS="$RPM_OPT_FLAGS `pkg-config --libs sdl` -lGL `pkg-config --libs
x11` -lm"

All on one line. I don't remember if putting the variables in 
the environment works, I think it doesn't.


The timestamp for the source file isn't kept. You can use
wget -N or spectool -g or similar curl option. Quite strangely,
the stamp is only 4 seconds different from upstream...

Source match upstream:
2214b0591ced40f27e00accdb8fa8f41  xgrav-1.2.0.tgz
Comment 4 Gwyn Ciesla 2007-10-16 17:12:04 EDT
Whoops, not enough caffiene. :)

Addressed.

Spec URL: http://zanoni.jcomserv.net/fedora/xgrav/xgrav.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/xgrav/xgrav-1.2.0-3.fc7.src.rpm
Comment 5 Patrice Dumas 2007-10-16 19:07:37 EDT
%{_bindir}/xgrav is listed twice
Comment 6 Gwyn Ciesla 2007-10-17 06:09:51 EDT
Spec URL: http://zanoni.jcomserv.net/fedora/xgrav/xgrav.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/xgrav/xgrav-1.2.0-4.fc7.src.rpm

Fixed.

But if the binary is built twice and listed twice, isn't the package just that
much BETTER? ;)
Comment 7 Patrice Dumas 2007-10-17 12:49:09 EDT
* rpmlint is silent
* follow guidelines
* free software license
* match upstream
2214b0591ced40f27e00accdb8fa8f41  xgrav-1.2.0.tgz
* use macros
* files section right
* build and works on devel

APPROVED
Comment 8 Gwyn Ciesla 2007-10-17 13:00:12 EDT
Thank you!

New Package CVS Request
=======================
Package Name: xgrav
Short Description: A simple physics simulation for a large number of particles
Owners: limb
Branches: FC-6 F-7 devel
InitialCC: 
Cvsextras Commits: yes
Comment 9 Marek Mahut 2007-10-17 13:09:32 EDT
What release note do you want to include and why?
Comment 10 Marek Mahut 2007-10-17 13:11:59 EDT
Oh, I guess you wanted to request CVS flag and not release note. 
Comment 11 Gwyn Ciesla 2007-10-17 13:15:09 EDT
Yeah, sorry, finger failure. :)
Comment 12 Kevin Fenzi 2007-10-17 20:18:42 EDT
cvs done.
Comment 13 Gwyn Ciesla 2007-10-18 07:55:37 EDT
Imported and built.
Comment 14 Fedora Update System 2007-10-24 03:17:22 EDT
xgrav-1.2.0-4.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Gwyn Ciesla 2007-11-08 14:47:39 EST
Package Change Request
======================
Package Name: xgrav
New Branches: F-8

Missed it at the mass branching, or maybe it was too close to the freeze and I
didn't request it.
Comment 16 Kevin Fenzi 2007-11-10 15:34:43 EST
cvs done.

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