Adding needsponsor blocker bug as you need a sponser, assigning to me since I'm
going to review this and then sponsor you as time permits. (I know musuruan from
the dribble repo which has copied/borrowed the FE review process, so this isn't
really his first package).

p.s. musuruan you should create an account in the Fedora Account system and get
your CLA done, so that things are ready to go when I find the time to review
this. Please do not add/request member ship of the Fedora Extras group until I'm
ready to sponsor you, once someone has requested access, all the sponsors get
multiple nag mails a day from the account system.

MUSTFIX:

Package claims to be GPL'ed but contains a tos.img binary, without any copyright
nor licence disclaimer nor any remark on this binary's origin or source code.

Original AtariST tos.img's are closed source and non-distributable.

AFAIK that is a free GPL tos replacement (am I right Andrea?), but that does
have a seperate upstream and thus should really be packaged seperately from its
own upstream and then Required by this package, Andrea can you make a seperate
package for this?

(In reply to comment #4)
> AFAIK that is a free GPL tos replacement (am I right Andrea?), 
I know there exist free tos replacements, but ... this package doesn't contain
any indication about its tos.img's origin nor copyright/licence.

> but that does
> have a seperate upstream and thus should really be packaged seperately from its
> own upstream and then Required by this package, Andrea can you make a seperate
> package for this?
Well, don't see why we would need a separate package, but I can't avoid having
to insist on a clear notice about this tos.img's copyright/licence and origin.

(In reply to comment #4)
> AFAIK that is a free GPL tos replacement (am I right Andrea?), but that does
> have a seperate upstream and thus should really be packaged seperately from its
> own upstream and then Required by this package, Andrea can you make a seperate
> package for this?

As Hans noted, that is not an original Atari ST TOS image. It is an EmuTOS image
supplied with the source archive of Hatari. EmuTOS is GPL'ed too.

Please see:
http://emutos.sourceforge.net/

I could do a separate package for EmuTOS using the already compiled ROM's. But I
don't think that is the preferred way.

If you wish a package compiled from sources, it goes beyond my current skills,
as it requires build dependencies that are not packaged in Fedora (the
cross-mint-gcc toolchain from the sparemint project) and that are only tested to
work on old RH 7.0/8.0 and based on old gcc 2.95.

If you choose to leave it along with hatari, would it be enough to put a notice
in the description filed of the RPM to state that EmuTOS is included and what
licence it has?

If itsn't feasible with reasonanble effort to buit the EmuTOS image from source
(and it sounds like it isn't), then keeping the tos image in the hatari srpm is
fine with me.

With regards to the tosimage, I dunno if I a statement in the RPM / an extra
readme under %doc, will suffice, it would be best if you could get a statement
from the upstream developers that this is an unmodified emutos image, or verify
yourself with md5sum's. The unmodified part is important because if the hatari
developers modified it they should give out the source too.

(In reply to comment #7)
> With regards to the tosimage, I dunno if I a statement in the RPM / an extra
> readme under %doc, will suffice, it would be best if you could get a statement
> from the upstream developers that this is an unmodified emutos image, or verify
> yourself with md5sum's. The unmodified part is important because if the hatari
> developers modified it they should give out the source too.

OK. I just verified that they are the same:

$ md5sum hatari-0.90/src/tos.img
b741a201e719ee15c09342cba0105f19  hatari-0.90/src/tos.img
$ md5sum emutos-0.8.1/etos256us.img
b741a201e719ee15c09342cba0105f19  emutos-0.8.1/etos256us.img
 

BTW, running Hatari, inside the emulated Atari ST you boot EmuTOS. Using the
appropriate menu item (Desk->Desktop Info...) you get appropriate copyright and
licence statement. 

I don't know if this is enough.

I believe that is enough, please add a README.tos or something like that with an 
full URL to the emutos-0.8.1 tarbal and a piece of text saying that the included
tos.img is identical to the etos256us.img included in this tarbal and that this
tos image is licenses under the GNU GPL, whose conditions can be found in the
COPYING file (which should alreayd be present from hatari itself.

(In reply to comment #7)
> If itsn't feasible with reasonanble effort to buit the EmuTOS image from source
> (and it sounds like it isn't), then keeping the tos image in the hatari srpm is
> fine with me.
No disagreement at this point, but ...

(In reply to comment #9)
> BTW, running Hatari, inside the emulated Atari ST you boot EmuTOS. Using the
> appropriate menu item (Desk->Desktop Info...) you get appropriate copyright and
> licence statement. 
> 
> I don't know if this is enough.
No it isn't. In general we need a legally valid license as part of the source.

A document/file or describing this files copyright/license situation, also
containing a link to where to download the sources, would appear sufficent to
me, but IANAL.

Another alternative would be to move this tos.img outside of hatari's source
tarball and to use a second SourceX URL inside of the *.spec, pointing to the
original sites and documents.

(In reply to comment #10)
> I believe that is enough,
I do not think it's enough to have a runtime exposed license. Consider if a
binary is violating a licence or law, users building from source don't have a
chance to check in adance to running the application.

Running the application already would be lawful, may-be even
untarring/downloading the tarball could be considered lawful.

(In reply to comment #12)
> (In reply to comment #10)
> > I believe that is enough,
> I do not think it's enough to have a runtime exposed license. Consider if a
> binary is violating a licence or law, users building from source don't have a
> chance to check in adance to running the application.
> 
> Running the application already would be lawful, may-be even
> untarring/downloading the tarball could be considered lawful.
> 
> 

Woops, poor choice of words, I didn't mean to suggest that the menu item
mentioned in comment 9 is enough, what I meant is that the md5sum check is
enough to proof that it is unmodified from upstream. To be able to make the
license situation clear to the end user (and to make clear where to get the
source for the tos image) the readme I describe in comment #10 must be added.

Once that readme is in place I believe things are good to go (legally, I still
need todo a review).

http://www.webalice.it/musuruan/reviews/hatari.spec
http://www.webalice.it/musuruan/reviews/hatari-0.90-4.src.rpm

Added README.tos to explain that Hatari is shipped with EmuTOS.

http://www.webalice.it/musuruan/reviews/hatari.spec
http://www.webalice.it/musuruan/reviews/hatari-0.90-5.src.rpm

Added a patch not to strip binaries during make install
Added hicolor-icon-theme to Requires

Very good, I was working on a review earlier todat and that was going to contain
a must fix on the hicolor-icon-theme Requires and I didn't even spot the other one.

Anyways here is a full review:

MUST:
=====
* rpmlint output is clean
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (GPL) ok
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on FC-6 x86_64
* BR: ok
* No locales
* No shared libraries
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code and permissible (gpl TOS) content
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* no -devel package needed, no libs / .la files.
* .desktop file as required and properly installed

Approved!

I'm ready to sponsor you know, please ask Fedora Extras group membership and
I'll sponsor you asap, once thats done you can import this as decribed on the wiki.

Hi,

I see you've imported this, but haven't requested a build yet, may I ask why?

With FE things are not like Dribble where Ian does all the building, with FE
we've got an automated buildsys, but you must instruct it to build, see:
http://fedoraproject.org/wiki/Extras/Contributors#head-0956b12959af46cfe0aa12d09ed15e573bfd9ef4
and:
http://fedoraproject.org/wiki/Extras/Contributors#head-e2f7f3048aae892d69bba2b1d1563aed5c63a1ff

I know. I was studing the Extras/Contributors doc very carefully.

I haven't done it yet because I requested branches for FC5 and FC6 and not only
devel. I had to wait for this. 

I just sow that my request has been completed.

I think I'll be able to request a build soon. I'm really busy right now.

Build succeded on FC5 and FC6.

It failed on devel because of bug #212350. When it will be corrected, I'll
request a new build for devel.

Released for devel too.