Bug 333921
Summary: | Review Request: xgrav - A simple physics simulation for a large number of particles | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gwyn Ciesla <gwync> |
Component: | Package Review | Assignee: | Patrice Dumas <pertusus> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, mmahut, notting, pertusus |
Target Milestone: | --- | Flags: | pertusus:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 1.2.0-4.fc7 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-10-24 07:17:24 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
Gwyn Ciesla
2007-10-16 11:54:14 UTC
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. Spec URL: http://zanoni.jcomserv.net/fedora/xgrav/xgrav.spec SRPM URL: http://zanoni.jcomserv.net/fedora/xgrav/xgrav-1.2.0-2.fc7.src.rpm Addressed. 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 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 %{_bindir}/xgrav is listed twice 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? ;) * 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 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 What release note do you want to include and why? Oh, I guess you wanted to request CVS flag and not release note. Yeah, sorry, finger failure. :) cvs done. Imported and built. 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. 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. cvs done. |