[Bug 2098] Review request: dolphin-emu - Gamecube / Wii / Triforce Emulator

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=2098

--- Comment #16 from Jeremy Newton <alexjnewt at hotmail.com> 2012-02-21 17:53:43 CET ---
(In reply to comment #14)
> I'm about to make a more complete, informal review. But first two remarks:
> 
> srpm doesn't build on koji, f16 and f17. Links:
>    http://koji.fedoraproject.org/koji/taskinfo?taskID=3805865
>    http://koji.fedoraproject.org/koji/taskinfo?taskID=3805914
> 
> The source comment doesn't describe the complete procedure to build the
> tarball. As a consequence, I cannot verify md5sum of srpm's source vs upstream.

Interesting, I had no issue building this on F16, I'll have to look into this.

(In reply to comment #15)
> More complete review. Hope this is not bad news :)

It's quite alright, it's all a part of the review process ;)

> Issues:
> 1.  Bundles bochs/disasm in Externals
>     http://bochs.sourceforge.net/cgi-bin/lxr/source/disasm.
> 2.  Bundles clrun, from clunix in Externals
>     http://www.cmap.polytechnique.fr/~sylvain/clunix.

Alright, dully noted, I'll remove these when I get a chance.

> 3.  Source/Core/Core/Src/IPC_HLE contains the kernel headers hci.h
>     and l2cap.h, which seems like a bad bundling.

Hmmm that seem problematic... I may have to contact upstream about this.

> 4.  From README, dolphin-emu needs SSE2 support. Does this rule out
>     non-x86 architectures, which needs to be excluded?

I don't think this is an issue, as RPMFusion only supports x86 archs.

> 5.  Source URL is not completely commented on how to generate tarball.
>     Why don't you just use upstream tarball as-is, add the add-on files
>     as SourceX: and removes Externals/misc in %prep? This is more
>     transparent and makes source verification simple.

Unfortunately there is no upstream tarball to my knowledge, so that is why I
included a git command. What would you suggest with the %prep section? I'm
using a spin as suggested by Julian in comment 3:

>>>I think it would be better to spin your own tarball from upstream git than
>>>to rely on other distributions.

> 6.  A number of files have BSD licenses. Shouldn't these be part of
>     License tag like in "GPLv2 and BSD" or similar?

I must have missed that, I'll look into that and fix it.

> 8.  No review remark as such, but the install icons part is a little messy,
>     could possibly be rewritten like:
>     for size in 16 32 48 128 256; do
>         dim="${size}x${size}"
>         install -p -D -m 0644 %{name}$size.png \
>         %{buildroot}%{_datadir}/icons/hicolor/$dim/apps/%{name}.png
>     done
>     Similarly, the %files section icons could be written:
>         %{_datadir}/icons/hicolor/*/apps/%{name}.png
>     However, this boils down to personal preferences.

Good point, I that would clean things up.

> 
> Rpmlint:
> 
> dolphin-emu.i686: E: zero-length
> /usr/share/dolphin-emu/user/GameConfig/WBEEJV.ini

That reminds me, I need to contact upstream about the value of this file.

> dolphin-emu-debuginfo.i686: E: incorrect-fsf-address
> /usr/src/debug/dolphin-emu-3.0/Source/Core/Core/Src/DSP/DSPHWInterface.cpp
>   [repeated 11 times]

I have reported this and it should be fixed in the next stable version



Thanks for your help, I'll look into fixing these issues :)

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