[Bug 740] Review request: meka - Multi machine emulator for MS-DOS, MS-Windows and GNU/Linux

[RPM Fusion Bugzilla]

http://bugzilla.rpmfusion.org/show_bug.cgi?id=740





--- Comment #16 from Andrea Musuruane <musuruan at gmail.com>  2010-12-04 19:25:38 ---
(In reply to comment #15)

> === Issues ===
> 1. meka.i686: W: invalid-license Distributable
> Fedora doesn't accept "Distributable" as a license any more.  In this case
> there are several different licenses in effect, and this should be noted by
> enumerating them with "and" in between. 
> (https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios)
>  Most code is covered by the license of sources.txt, but I also found LGPLv2+
> (libs/seal/include/audio.h), zlib/libpng license (libs/libpng/* and
> srcs/libaddon/zip/*), and a non-commercial use license (srcs/z8marat and
> srcs/m6502/*).  Even if it gets a bit long, it is probably best to enumerate
> them.

Seal is not used. Zlib and libpng are external system libraries. Even if their
sources are inside meka, the system libraries are used and AFAIK you don't have
to list the license of the package you BR in the License tag.

The License: field refers to the licenses of the contents of the *binary* RPM:
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License:_field

Meka is licensed under the "MEKA License 1.1" (stated in the README file), a
customized BSD like license. Other parts, as you noted, are for non commercial
use. Therefore Distributable seems fine to me. We are in RPM Fusion and not in
Fedora. Anyway this implies that meka will go in the nonfree repo.

> 2. meka.i686: W: executable-stack /usr/libexec/meka/meka
> I believe the problem here is that nasm sets __OUTPUT_FORMAT__ "elf32" rather
> than "elf".  Thus, the %ifidn test gets false, and your patch has no effect.  I
> haven't tried it all the way through, though; leave that to you! :-)

Good catch. It worked :)

> 3. meka.i686: W: no-manual-page-for-binary meka
> It's a "should", but I guess it could be hard to convince upstream to make one,
> given that meka is in maintenance mode.  But it can't hurt to suggest it, I
> guess. :-)

I won't provide a man page unless I find one ready on the net. Too much work
for an emulator that has a nice GUI :)

> 4. meka.src:28: W: macro-in-comment %{ix86}
> Maybe not a big deal, but could you explain why you have commented out the use
> of the %ix86 macro, and hardcoded i686?

I removed that comment but I'll explain the meaning of it. I had to hardcode
i686 otherwise plague on RPM Fusion will build packages for i386, i486, i585
and i686 if you use the %ix86 macro.

> 6. There are a couple of binary libraries in the source package, like
> libs/seal/lib/Win32/audw32vc_s.lib and libs/libpng/lib/libpng.lib for example. 
> As far as I can tell they are not used, but to make sure they should be removed
> in %prep according to
> https://fedoraproject.org/wiki/Packaging/Guidelines#No_inclusion_of_pre-built_binaries_or_libraries

Done.

> 7. I think it would be a good idea to have a comment in the spec file
> explaining that the REASON you only want to build on i686 is that the package
> contains assembly code.

Done.

> 8. You use %buildroot as an RPM macro but $RPM_OPT_FLAGS as a shell variable. 
> The packaging guidelines prefers if you choose one or the other.

Done.

> 9. If you want Swedish translations of summary and descriptions, you can use
> these: :-)

I changed the summary. I hope now it is more meaningful now. 

Anyway, I won't use your translations because I won't be able to maintain them.
I'm sorry but I don't speak Swedish.

> 10. The start script does a "cd" to begin with, does some setup, and then
> executes the real binary with the same arguments.  The first argument is the
> ROM image.  But if this was given with a relative path, that path will no
> longer be valid.  Maybe you could check if $1 begins with a slash, and if not,
> prepend $PWD?

It would be not so simple. The first parameter can be something that is not a
path (e.g. a parameter). Anyway this script is just slightly adapted form the
Games SIG Guidelines:
https://fedoraproject.org/wiki/SIGs/Games/Packaging

Therefore I think all game wrappers in Fedora have this issue.

> 11. There is some kind of character encoding issue that maybe should be
> reported upstreams.  My home directory is "/home/göran", UTF-8 encoded.  When
> starting, meka doesn't find meka.dat.  Strace:ing I found it is trying to open
> it under the wrong path:

I'll report this issue upstream.


http://www.webalice.it/musuruan/RPMS/reviews/meka.spec
http://www.webalice.it/musuruan/RPMS/reviews/meka-0.73-3.fc14.src.rpm

Changelog:
- Changed summary
- Removed pre-built lib files
- Fixed execstack patch
- Fixed consistent use of macros
- Minor changes


-- 
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.