Bug 452195 - Review Request: Presto - A tilemap engine using the Allegro game programming library
Review Request: Presto - A tilemap engine using the Allegro game programming ...
Status: ASSIGNED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Ignacio Vazquez-Abrams
Fedora Extras Quality Assurance
http://www.hypersonicsoft.org/project...
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-19 22:40 EDT by Peter Fernandes
Modified: 2013-11-21 10:12 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ivazqueznet: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Fernandes 2008-06-19 22:40:33 EDT
Spec URL: http://www.hypersonicsoft.org/projects/downloads/Presto/misc/presto.spec
SRPM URL: http://www.hypersonicsoft.org/projects/downloads/Presto/misc/presto-0.1.1-1.fc9.src.rpm
Description:
Presto is a general-use tilemap engine coded in C that uses Allegro for graphics rendering, and therefore is intended for use in games using Allegro.  It can handle rectangular tiles of any height and width (and different height from width), loading tilemaps from files, tile blending, and the capability to change most of these elements on the fly.

This is my first package, and I need a sponsor.
Comment 1 Peter Fernandes 2008-06-19 23:21:25 EDT
I forgot to mention that I'm the upstream developer for this package.  I have a
project page on my website for the library, found here:
http://www.hypersonicsoft.org/projects/showproject.php?id=29.  I'm an
experienced C programmer, and I definitely appreciate anyone who is willing to
help me with this first package!
Comment 2 Ignacio Vazquez-Abrams 2008-06-19 23:59:08 EDT
- Descriptions should be wrapped at 79 characters
- Examples, README, and COPYING should be in %doc
- Changelog entries should be separated by a newline
- Fails to build in mock on F8/i386 and F9/i386
- -devel is missing a Requires of allegro-devel, and should have no BuildRequires
Comment 3 Peter Fernandes 2008-06-20 23:04:50 EDT
I've fixed the errors you pointed out, Ignacio, as well as a few other things. 
Most notably, Presto no longer links with Allegro during build.  This is because
Allegro contains some non-PIC assembly code that rpmlint doesn't want to allow.
 rpmlint now complains about undefined references to Allegro functions due to
not linking, but the end user can link with Allegro when using Presto.  The only
other solution to this would be to either remove or change the assembly code in
Allegro, neither of which I want to do.  Here are the updated SRPM and spec
links.  I've built the SRPM using mock with no errors, and gotten rid of all the
rpmlint errors.

http://www.hypersonicsoft.org/projects/downloads/Presto/misc/presto-0.1.1-6.fc9.src.rpm
http://www.hypersonicsoft.org/projects/downloads/Presto/misc/presto.spec

Thanks!
Comment 5 Peter Fernandes 2008-06-21 03:22:06 EDT
I have more changes than I thought to make to the package.  I'll update the
links once again tomorrow.
Comment 6 Peter Fernandes 2008-06-22 00:00:38 EDT
Here's the update:
http://www.hypersonicsoft.org/projects/downloads/Presto/release/presto-0.1.3-1.fc9.src.rpm
http://www.hypersonicsoft.org/projects/downloads/Presto/release/presto.spec

The package builds cleanly on at least Fedora i386 and x86_64 systems.  rpmlint
gives no errors (but still the tolerable warnings that I mentioned above).

Thanks!
Comment 7 Peter Fernandes 2008-06-22 00:23:42 EDT
- -devel is missing a Requires of allegro-devel, and should have no BuildRequires
>> The -devel package depends on the main package, which depends on allegro-devel.
Comment 8 Ignacio Vazquez-Abrams 2008-06-24 21:17:26 EDT
(In reply to comment #7)
> - -devel is missing a Requires of allegro-devel, and should have no BuildRequires
> >> The -devel package depends on the main package, which depends on allegro-devel.

No, the main package has allegro-devel as a BuildRequires. Software built using
presto-devel will fail due to the fact that presto.h (in presto-devel) requires
allegro.h (in allegro-devel), but presto-devel does not require allegro-devel.
Comment 10 Kevin Kofler 2008-07-03 17:39:53 EDT
> qmake-qt4 PREFIX="%{buildroot}/%{_prefix}" \
> LIBFINALDIR="%{buildroot}/%{_libdir}"

That's invalid, this should be just:
qmake-qt4 PREFIX="%{_prefix}" LIBFINALDIR="%{_libdir}"
and then use:
make install DESTDIR="%{buildroot}"
instead of just make install.
Comment 11 Kevin Kofler 2008-07-03 18:23:29 EDT
Actually, with qmake, you have to use:
make install INSTALL_ROOT="%{buildroot}"
(INSTALL_ROOT, not DESTDIR)
Comment 13 Ignacio Vazquez-Abrams 2008-07-21 22:31:45 EDT
From rpmlint:

presto-0.1.3-3.fc8.src.rpm:
presto.src: E: no-cleaning-of-buildroot %clean

Looks like you have a $ in %clean where you should have a %.

presto-devel-0.1.3-3.fc8.i386.rpm:
presto-devel.i386: W: spurious-executable-perm
/usr/share/doc/presto-0.1.3/examples/build

This one I'm willing to let slide.

Just fix the error in %clean and this one is approved.
Comment 15 Peter Fernandes 2008-07-22 01:22:46 EDT
New Package CVS Request
=======================
Package Name: presto
Short Description: A tilemap engine using the Allegro game programming library
Owners: hypersonic
Branches: F-8 F-9
InitialCC: hypersonic
Cvsextras Commits: yes
Comment 16 Kevin Fenzi 2008-07-22 12:02:51 EDT
cvs done.
Comment 17 Mamoru TASAKA 2008-07-22 22:23:35 EDT
I just watch the commit log of this package but directory ownership issue is
not correctly addressed. Please fix it (i.e. make it sure that all directories
which are created when installing this package are correctly owned by this package)

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