Bug 620000 - Review Request: hatari - An Atari ST emulator suitable for playing games
Summary: Review Request: hatari - An Atari ST emulator suitable for playing games
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
medium
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-07-31 10:12 UTC by Andrea Musuruane
Modified: 2010-11-14 17:56 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-11-14 17:56:49 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lemenkov: fedora-review+


Attachments (Terms of Use)

Description Andrea Musuruane 2010-07-31 10:12:35 UTC
Spec URL: http://musuruan.fedorapeople.org/hatari.spec
SRPM URL: http://musuruan.fedorapeople.org/hatari-1.4.0-1.fc12.src.rpm
Description:
Hatari is an emulator for the Atari ST, STE, TT and Falcon computers.
More precisely, it is an adaption of the WinSTon source code to 
Linux, using the UAE CPU core instead of the original, non-portable 
assembler CPU core.

The Atari ST was a 16/32 bit computer system which was first released 
by Atari in 1985. Using the Motorola 68000 CPU, it was a very popular 
computer having quite a lot of CPU power at that time.

Unlike many other Atari ST emulators which try to give you a good 
environment for running GEM applications, Hatari tries to emulate the 
hardware of a ST as close as possible so that it is able to run most 
of the old ST games and demos.


I am the current hatari maintainer. I ask a re-review as discussed in fedora-devel because:
* Hatari 1.4.0 drops the old build system based on autotools and now
it only support cmake. 
* An (optional) python GUI is now bundled with the emulator.

Comment 1 Andrea Musuruane 2010-11-12 18:04:55 UTC
Spec URL: http://musuruan.fedorapeople.org/hatari.spec
SRPM URL: http://musuruan.fedorapeople.org/hatari-1.4.0-2.fc12.src.rpm

Changelog:
- fixed Requires
- added manpages from Debian
- removed script extensions (.sh, .py) from scripts installed into /usr/bin
- macro usage is more consistent now

Comment 2 Andrea Musuruane 2010-11-12 18:05:40 UTC
Ops... sorry, correct URL:

Spec URL: http://musuruan.fedorapeople.org/hatari.spec
SRPM URL: http://musuruan.fedorapeople.org/hatari-1.4.0-2.fc14.src.rpm

Comment 3 Peter Lemenkov 2010-11-14 11:27:05 UTC
I'll review it

Comment 4 Peter Lemenkov 2010-11-14 11:32:55 UTC
Koji scratch build for F-14:

http://koji.fedoraproject.org/koji/taskinfo?taskID=2599856

Comment 5 Peter Lemenkov 2010-11-14 12:04:13 UTC
REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is silent

sulaco ~: rpmlint ~/Desktop/hatari-*
hatari.src: W: invalid-url Source1: hatari-1.4.0-debian-manpages.tar.gz
4 packages and 0 specfiles checked; 0 errors, 1 warnings.
sulaco ~: 

This may be safely ignored.

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.

+/- The package meets the Packaging Guidelines. I have only few (maybe not so important) notes:

* I don't like this "Requires %{name}" line in ui sub-package. Does it means that UI should work fine with previous versions of hatari?
* Regarding python support - the explicit "Requires: python2" also worries me - I strongly suggest you to test whether hatari-ui works with python3 since we already ship python3 in F-14 an higher.

Anyway these two are not a blockers - just a friendly reminders.

+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (GPLv2 or later).

- The file, containing the text of the license(s) for the package (gpl.txt), MUST be included in %doc.

+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

sulaco ~/rpmbuild/SOURCES: sha256sum hatari-1.4.0.tar.bz2*
690e21bd2210a7e86af5d76ccc7f4e608aae37df466d2ead2ac4d105a637bc7b  hatari-1.4.0.tar.bz2
690e21bd2210a7e86af5d76ccc7f4e608aae37df466d2ead2ac4d105a637bc7b  hatari-1.4.0.tar.bz2.1
sulaco ~/rpmbuild/SOURCES:

+ The package successfully compiles and builds into binary rpms on at least one primary architecture. See koji link above.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files in some of the dynamic linker's default paths.
+ The package does NOT bundle copies of system libraries.
0 The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files without a suffix (e.g. libfoo.so).
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
+ The package includes a %{name}.desktop file, and this file is properly installed with desktop-file-install in the %install section.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Ok, so, please, mark gpl.txt as %doc and I'll finish it. Also it would be great if you comment my notes regarding dependencies in *-ui sub-package.

Comment 6 Andrea Musuruane 2010-11-14 14:04:18 UTC
(In reply to comment #5)
> * I don't like this "Requires %{name}" line in ui sub-package. Does it means
> that UI should work fine with previous versions of hatari?

No. You are right. I changed the Require to use a fully versioned dependency.

> * Regarding python support - the explicit "Requires: python2" also worries me -
> I strongly suggest you to test whether hatari-ui works with python3 since we
> already ship python3 in F-14 an higher.

AFAIK Python3 is an optional component since F13. Critical system components will continue to use Python 2:
https://fedoraproject.org/wiki/Features/Python3F13

Anyway, I mailed upstream author asking if hatari-ui will support python3 in the future.


Spec URL: http://musuruan.fedorapeople.org/hatari.spec
SRPM URL: http://musuruan.fedorapeople.org/hatari-1.4.0-3.fc14.src.rpm
Changelog:
- ui subpackage now requires the fully versioned base package
- more consistent macro usage

Comment 7 Peter Lemenkov 2010-11-14 14:49:49 UTC
Ok, good. But still no gpl.txt in docs. I advice you to add something like this into %install section:

install -p -m 0644 gpl.txt %{buildroot}%{_docdir}/%{name}-%{version}

This is the only issue left unresolved.

Comment 8 Andrea Musuruane 2010-11-14 15:27:37 UTC
Spec URL: http://musuruan.fedorapeople.org/hatari.spec
SRPM URL: http://musuruan.fedorapeople.org/hatari-1.4.0-4.fc14.src.rpm
Changelog:
- added license among docs

Comment 9 Peter Lemenkov 2010-11-14 15:45:01 UTC
Ok, good. I don't see any other issues, so this package is

APPROVED.

Comment 10 Andrea Musuruane 2010-11-14 17:56:49 UTC
Thanks for the review.

Built and published for rawhide only. Asked a FESCo updates process exception for at least F14:
https://fedorahosted.org/fesco/ticket/493

Closing.


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