Bug 180430 - Review Request: Tong - a game of skill
Review Request: Tong - a game of skill
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-02-07 22:11 EST by Wart
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-02-23 22:29:53 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Wart 2006-02-07 22:11:45 EST
Spec Name or Url: http://www.kobold.org/~wart/fedora/tong.spec
SRPM Name or Url: http://www.kobold.org/~wart/fedora/tong-1.0-2.src.rpm
Description: 
Tong is a clever mixture of two other popular computer games.

The data files and core game are split into two packages; the small game package requires the much larger data files package.  Both come from the same source archive and spec file, however, so there won't be any space saving benefit from splitting this up into the two packages since an rebuild and release of the game will trigger a rebuild and release of the data package as well.
Comment 1 Jason Tibbitts 2006-02-19 00:40:13 EST
The source grabbed from http://www.nongnu.org/tong/tong-1.0.tar.gz doesn't match
what's in your SRPM.  Did upstream change their tarball recently?

rpmlint says:
E: tong-data only-non-binary-in-usr-lib
W: tong-data no-documentation

The stuff that tong-data installs really should go into /usr/share and the
binary should go into /usr/bin.  I know the program is broken in that it
requires the media directory to be in the same place as the binary, but honestly
I think it's simpler to apply a tiny patch to chdir("/usr/share/tong") at the
start of main() than to hack around the deficiency of the program.

A five-minute hack is at:
http://www.math.uh.edu/~tibbs/rpms/tong/tong.spec
http://www.math.uh.edu/~tibbs/rpms/tong/tong-1.0-3.src.rpm
Comment 2 Wart 2006-02-22 18:19:46 EST
(In reply to comment #1)
> The source grabbed from http://www.nongnu.org/tong/tong-1.0.tar.gz doesn't match
> what's in your SRPM.  Did upstream change their tarball recently?

That's funny.  I just downloaded it again and both diff and md5sum claim that
they match:
9f358a012639de1a5a8d3e0b323438de  tong-1.0.tar.gz

Can you check the md5sum on the file that you downloaded?

> rpmlint says:
> E: tong-data only-non-binary-in-usr-lib
> W: tong-data no-documentation
> 
> The stuff that tong-data installs really should go into /usr/share and the
> binary should go into /usr/bin.  I know the program is broken in that it
> requires the media directory to be in the same place as the binary, but honestly
> I think it's simpler to apply a tiny patch to chdir("/usr/share/tong") at the
> start of main() than to hack around the deficiency of the program.
>
> A five-minute hack is at:
> http://www.math.uh.edu/~tibbs/rpms/tong/tong.spec
> http://www.math.uh.edu/~tibbs/rpms/tong/tong-1.0-3.src.rpm

I'll see your patch, and raise you a #define so that %{_datadir} cab be passed
in during the compile step:

http://www.kobold.org/~wart/fedora/tong-1.0-4.src.rpm
http://www.kobold.org/~wart/fedora/tong.spec
Comment 3 Wart 2006-02-22 19:33:17 EST
Doh.  I should try running things before claiming success.  I'll get this
cleaned up...
Comment 4 Wart 2006-02-22 20:12:13 EST
Ok, all fixed.
Comment 5 Jason Tibbitts 2006-02-23 00:11:39 EST
Well, I tried again and the checksums do match what you get.  I had tried
multiple times before.  (I was using spectool, which uses wget, so there
shouldn't be a browser caching issue.)

Yes, props for the improved hack.  Much better.

rpmlint now complains only about tong-data having no documentation; no problem
there.

The only remaining issue is %{optflags} (or $RPM_OPT_FLAGS), which isn't getting
passed.  The Makefile doesn't support CFLAGS, but you can set CC:

make CC="%{__cc} %{optflags}" GAME_DATA_DIR=%{_datadir}/tong %{?_smp_mflags}

This produces a build that contains proper debug info (so the debuginfo package
actually contains something) and uses recommended flags like FORTIFY_SOURCE. 
Unfortunately I can't test the package from home so I'll have to wait until
tomorrow.

Beyond that I don't see any remaining issues; I'll test and run some mock builds
tomorrow.
Comment 6 Wart 2006-02-23 02:26:16 EST
(In reply to comment #5)
> The only remaining issue is %{optflags} (or $RPM_OPT_FLAGS), which isn't getting
> passed.  The Makefile doesn't support CFLAGS, but you can set CC:
> 
> make CC="%{__cc} %{optflags}" GAME_DATA_DIR=%{_datadir}/tong %{?_smp_mflags}

Done.  Even though this overrides the "-O3 -Wall" options in the Makefile, these
options are added back with optflags.  But it needs to be __cxx instead of __cc.
 I also updated the improved hack to suppress a compiler warning.

http://www.kobold.org/~wart/fedora/tong-1.0-5.src.rpm
http://www.kobold.org/~wart/fedora/tong.spec

I've been able to run it fine on FC4 i386 and x86_64, but it dies on FC5-i386. 
I suspect that this might be due to my misconfigured vmware box, though.
Comment 7 Jason Tibbitts 2006-02-23 11:33:45 EST
I found build failures in mock as well, but that was when I was mistakenly using
%{__cc}.  Everything builds fine with the SRPM you sent and tests out OK on an
i386 FC4 machine.

So, I think the last cleanup is getting rid of tong.sh since you aren't using it
any longer.  You might also want to either use $RPM_OPT_FLAGS instead of
%{optflags} just to be consistent with your last changelog entry.

Other than that I think it's ready to go.
Comment 8 Wart 2006-02-23 12:16:56 EST
Only the spec file has changed in this latest package:

http://www.kobold.org/~wart/fedora/tong-1.0-6.src.rpm
http://www.kobold.org/~wart/fedora/tong.spec

I think all issues have finally been addressed.

FWIW, I've requested upstream that the game data files be split into a separate
tarball, and will forward the GAME_DATA_DIR patch as well.
Comment 9 Jason Tibbitts 2006-02-23 12:51:21 EST
Cool.  Approved.
Comment 10 Wart 2006-02-23 22:29:53 EST
Imported and built.

Thanks!

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