[Bug 2098] Review request: dolphin-emu - Gamecube / Wii / Triforce Emulator
https://bugzilla.rpmfusion.org/show_bug.cgi?id=2098
--- Comment #16 from Jeremy Newton <alexjnewt(a)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.