Bug 452195 - Review Request: Presto - A tilemap engine using the Allegro game programming library
Summary: Review Request: Presto - A tilemap engine using the Allegro game programming ...
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Ignacio Vazquez-Abrams
QA Contact: Fedora Extras Quality Assurance
URL: http://www.hypersonicsoft.org/project...
Depends On:
TreeView+ depends on / blocked
Reported: 2008-06-20 02:40 UTC by Peter Fernandes
Modified: 2020-05-27 16:42 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2020-05-27 16:42:12 UTC
Type: ---
ivazqueznet: fedora-review+
kevin: fedora-cvs+

Attachments (Terms of Use)

Description Peter Fernandes 2008-06-20 02:40:33 UTC
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
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-20 03:21:25 UTC
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-20 03:59:08 UTC
- 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-21 03:04:50 UTC
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.



Comment 5 Peter Fernandes 2008-06-21 07:22:06 UTC
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 04:00:38 UTC
Here's the update:

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


Comment 7 Peter Fernandes 2008-06-22 04:23:42 UTC
- -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-25 01:17:26 UTC
(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 21:39:53 UTC
> 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 22:23:29 UTC
Actually, with qmake, you have to use:
make install INSTALL_ROOT="%{buildroot}"

Comment 13 Ignacio Vazquez-Abrams 2008-07-22 02:31:45 UTC
From rpmlint:

presto.src: E: no-cleaning-of-buildroot %clean

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

presto-devel.i386: W: spurious-executable-perm

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 05:22:46 UTC
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 16:02:51 UTC
cvs done.

Comment 17 Mamoru TASAKA 2008-07-23 02:23:35 UTC
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)

Comment 18 Mattia Verga 2020-05-27 16:42:12 UTC
This package was approved and imported in repositories and it was later retired, but this review ticket was never closed.
I'm closing it now.

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