Bug 287801 - (sirius) Review Request: Sirius - Reversi game for Gnome
Review Request: Sirius - Reversi game for Gnome
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
http://sirius.bitvis.nu/
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-09-12 11:41 EDT by Arindam Ghosh
Modified: 2007-11-30 17:12 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-10-08 09:14:36 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Arindam Ghosh 2007-09-12 11:41:40 EDT
Spec URL: http://makghosh.googlepages.com/sirius.spec
SRPM URL: http://makghosh.googlepages.com/sirius-0.8.0-5.src.rpm
Description: 

Hi! I just finished packaging up Sirius. This is my first package, and I would appreciate a review so that I can get it into the Fedora repository.

Sirius is a program for playing the game of othello. The program includes an AI (Artificial Intelligence) opponent which plays at a very challenging level and is actually quite hard to beat. The AI opponent's strength can therefore be adjusted in several ways to give you a suitable opponent.

The AI opponent uses a plain alpha-beta search with hashing to figure out which move to make. To be able to tell a god position from a bad one, it uses a pattern based evaluation function. The pattern used is the 9 discs surrounding each corner and the 8 discs creating the edge of the board. The evaluation function also takes mobility, potential mobility and parity into count. For the initial 9 moves the AI opponent optionally uses a simple opening book. During midgame it searches and evaluates about 200.000 nodes per second on a PIII 750 MHz, in the endgame this number is significantly higher due to more transpositions and a less expensive evaluation function.
Comment 1 kushaldas@gmail.com 2007-09-12 14:36:53 EDT
Can not sponsor you. But will try to check it :)
Comment 2 kushaldas@gmail.com 2007-09-12 14:43:56 EDT
* dist tag missing in release
* Don't use %makeinstall try make install DESTDIR=$RPM_BUILD_ROOT
* Take out 	--add-category X-Fedora				\
	        --add-category Application			\
* Please bump the release number in the proper way
Comment 3 Arindam Ghosh 2007-09-12 15:24:15 EDT
(In reply to comment #2)
> * dist tag missing in release
> * Don't use %makeinstall try make install DESTDIR=$RPM_BUILD_ROOT
> * Take out 	--add-category X-Fedora				\
> 	        --add-category Application			\
> * Please bump the release number in the proper way

Ok. I will redo these advised changes :)
Comment 4 Arindam Ghosh 2007-09-13 00:16:18 EDT
Now I made the following changes in the spec file and rebuild the rpms:
* added dist tag in the release
* Replaced %makeinstall with make install DESTDIR=%{buildroot}
* Removed     --add-category X-Fedora                         \
              --add-category Application                      \
* Bumped up the release number. So its' now sirius-0.8.0-6

So the updated links for spec and SRPM are at: http://makghosh.googlepages.com/

Specifically,
Spec file: http://makghosh.googlepages.com/sirius.spec
SRPM: http://makghosh.googlepages.com/sirius-0.8.0-6.fc7.src.rpm
Comment 5 Arindam Ghosh 2007-09-14 15:13:03 EDT
Fixed the uniformity of %{name} as suggested by Debarshi Ray
Comment 6 Arindam Ghosh 2007-09-17 16:13:36 EDT
The first Koji scratch build of sirius failed for x86_64 architechture. You can
see the build.log (of x86_64) over here:
[http://koji.fedoraproject.org/koji/getfile?taskID=160171&name=build.log]

The build.log showed up an error that "error: XML::Parser perl module is
required for intltool". So i added perl-XML-parser as build require. After that,
Koji build was successful. The task is:
[http://koji.fedoraproject.org/koji/taskinfo?taskID=161229]

So did following things:
* added perl-XML-parser as build require
* bumped up release number to 7

The updated links of the spec and srpm are,
Spec file URL: http://makghosh.fedorapeople.org/SPECS/sirius.spec
SRPM URL: http://makghosh.fedorapeople.org/SRPMS/sirius-0.8.0-7.fc7.src.rpm
Comment 7 Arindam Ghosh 2007-09-17 16:30:16 EDT
Also rpmlint gives no errors/warnings:

$ rpmlint .../sirius-0.8.0-7.fc7.src.rpm
$ rpmlint .../sirius-0.8.0-7.fc7.i386.rpm
$ rpmlint .../sirius-debuginfo.0.8.0-7.fc7.i386.rpm

all these simply gave no errors/warnings
Comment 8 Mamoru TASAKA 2007-09-19 11:52:50 EDT
For 0.8.0-2:

* First for long sirius-0.8.0-autoreconf.patch.bz2
  - Plese don't use such uselessly long patch.
    This makes it harder to find what actually has to be
    fixed by this patch.
    * If you have to use autotools, please use autotools,
      not use such long patch. 
    * However using autotools is highly _not_ recommended.
      Please investigate the points which should really
      be fixed.
  - By the way, is this really needed? ppc64 rebuild succeeds
    without this patch.

* perl modules BuildRequires
  (also refer to http://fedoraproject.org/wiki/Packaging/Perl )
  - For perl modules, please don't write rpm name for
    BuildRequires. Instead, write the module name for BuildRequires
    like:
    BuildRequires: perl(XML::Parser)

* License
  - License seems to be GPLv2+.

* CFLAGS
-----------------------------------------------
%build
%configure
make %{?_smp_mflags} CFLAGS="%{optflags}"
-----------------------------------------------
  - Check if CFLAGS option in the last line is really needed.
    Usually this is not needed (please check what %configure
    does by:
    $ rpm --eval %configure )

* Timestamp
  - To keep timestamp, I recommend:
-----------------------------------------------
make install DESTDIR=%{buildroot} INSTALL="%{__install} -p"
-----------------------------------------------
    The method above usually works on recent Makefiles.

* desktop category
  - Category "Application" is deprecated and should be removed.

* rpmlint
------------------------------------------------
sirius.i386: W: file-not-utf8 /usr/share/doc/sirius-0.8.0/AUTHORS
sirius.i386: W: file-not-utf8 /usr/share/doc/sirius-0.8.0/ChangeLog
sirius.i386: W: file-not-utf8 /usr/share/doc/sirius-0.8.0/README
------------------------------------------------
     - Please change the encoding of these files to UTF-8.
Comment 9 Mamoru TASAKA 2007-09-19 12:01:39 EDT
One more comment:

--------------------------------------------------
board.c: In function 'transposition_hash':
board.c:436: warning: dereferencing type-punned pointer will break
strict-aliasing rules
board.c: In function 'calculate_legal':
board.c:578: warning: integer constant is too large for 'long' type
board.c:579: warning: integer constant is too large for 'long' type
board.c:580: warning: integer constant is too large for 'long' type
board.c:581: warning: integer constant is too large for 'long' type
board.c:582: warning: integer constant is too large for 'long' type
board.c:583: warning: integer constant is too large for 'long' type
board.c:584: warning: integer constant is too large for 'long' type
board.c:585: warning: integer constant is too large for 'long' type
board.c:588: warning: integer constant is too large for 'long' type
board.c:589: warning: integer constant is too large for 'long' type
<and many lines follow>
----------------------------------------------------------
  - Perhaps you want to fix these warnings, especially warnings
    for "integer constant".
Comment 10 Arindam Ghosh 2007-09-20 13:09:01 EDT
(In reply to comment #8)

I am working on the comments given by you. Will post about the updates by tommorrow
Comment 11 Arindam Ghosh 2007-09-21 15:31:02 EDT
Issues Resolved:

* perl modules BuildRequires fixed to perl(XML::Parser)

* License tag updated to GPLv2+

* CFLAGS option removed from make. So its' now:
-----------------------------------------------
%build
%configure
make %{?_smp_mflags} 
-----------------------------------------------

* INSTALL option added to make install. Its' now:
-----------------------------------------------
make install DESTDIR=%{buildroot} INSTALL="%{__install} -p"
-----------------------------------------------

* UTF-8 encoding of "AUTHORS", "Changelog", "README":
  - These three files were in ISO-8859 encoding. So i used the following
commands to change them to UTF-8 encoding:
-----------------------------------------------
[Joy@localhost sirius-0.8.0]$ iconv -f ISO-8859-1 -t UTF-8 AUTHORS
[Joy@localhost sirius-0.8.0]$ iconv -f ISO-8859-1 -t UTF-8 Changelog
[Joy@localhost sirius-0.8.0]$ iconv -f ISO-8859-1 -t UTF-8 README
-----------------------------------------------
  - So now the encoding of these files :
-----------------------------------------------
[Joy@localhost sirius-0.8.0]$ file AUTHORS ChangeLog README 
AUTHORS:   UTF-8 Unicode text
ChangeLog: UTF-8 Unicode English text
README:    UTF-8 Unicode text
-----------------------------------------------
  - The output of rpmlint is given as follows for i386 rpm:
-----------------------------------------------
[Joy@localhost redhat]$ rpmlint -i RPMS/i386/sirius-0.8.0-8.fc7.i386.rpm 
[Joy@localhost redhat]$
-----------------------------------------------

Issues that still needs work:

* Will try to fix the sirius-0.8.0-autoreconf.patch.bz2 problem. Seeing the ways
 to fix this. By the way, when I tried to do a rpmbuild by commenting out %patch
part in the %prep section, it returned with following errors:
-----------------------------------------------
<more lines above>
+ /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump
+ /usr/lib/rpm/brp-python-bytecompile
+ /usr/lib/rpm/redhat/brp-java-repack-jars
error: Bad file: /usr/src/redhat/SOURCES/sirius-0.8.0-autoreconf.patch.bz2: No
such file or directory


RPM build errors:
    Bad file: /usr/src/redhat/SOURCES/sirius-0.8.0-autoreconf.patch.bz2: No such
file or directory
-----------------------------------------------

* Also trying to fix: "warning: integer constant is too large for 'long' type"

* desktop-category: I think there's no such category in spec file. If I
seriously misunderstood you for this, please clarify.
Comment 12 Arindam Ghosh 2007-09-21 15:42:42 EDT
The updated links of the spec and srpm are,
Spec file URL: http://makghosh.fedorapeople.org/SPECS/sirius.spec
SRPM URL: http://makghosh.fedorapeople.org/SRPMS/sirius-0.8.0-8.fc7.src.rpm

The new Koji sratch build task:
[http://koji.fedoraproject.org/koji/taskinfo?taskID=169401]
Comment 13 Mamoru TASAKA 2007-09-22 07:12:23 EDT
(In reply to comment #11)

> * desktop-category: I think there's no such category in spec file. If I
> seriously misunderstood you for this, please clarify.

Please check the real desktop file
-----------------------------------------------
$ cat /usr/share/applications/fedora-sirius.desktop 

[Desktop Entry]
Encoding=UTF-8
Name=Sirius
Name[de]=Sirius
Name[fr]=Sirius
Name[ru]=Сириус
Name[sv]=Sirius
Comment=Sirius, a othello playing program
Exec=sirius
Icon=sirius.png
Terminal=false
Type=Application
Categories=GNOME;Application;Game;
X-Desktop-File-Install-Version=0.13
-------------------------------------------------------
Comment 14 Arindam Ghosh 2007-09-22 10:03:10 EDT
(In reply to comment #13)

Oh!! I got it.
Comment 15 Subhodip Biswas 2007-09-25 11:34:56 EDT
please also check for 
Group:Amusements/Games 

please take a look at this.
Comment 16 Arindam Ghosh 2007-09-25 11:42:13 EDT
(In reply to comment #15)

I've already fixed this. But couldn't upload the srpm and spec until now due to
lack of current/internet for last two days at my place.

Comment 17 Arindam Ghosh 2007-10-02 09:20:11 EDT
(In reply to comment #8 & comment #13)

Issues fixed:
- Patch file removed. As you said it is not required.
- Destop Entry fixed

As i'm not getting access to fedorapeople.org (don't know why), the updated
links are:
Spec file URL: http://makghosh.googlepages.com/sirius.spec
SRPM URL: http://makghosh.googlepages.com/sirius-0.8.0-9.fc7.src.rpm
Comment 18 Arindam Ghosh 2007-10-02 09:24:20 EDT
The Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=180345
Comment 19 Arindam Ghosh 2007-10-02 09:28:40 EDT
(In reply to comment #15)

The group is "ok" i think. Otherwise rpmlint would have given errors. You may
check it out
Comment 20 Mamoru TASAKA 2007-10-02 14:54:26 EDT
For -9:

* Tarball itself
------------------------------------------------------------------
[tasaka1@localhost sirius]$ ls -al *gz */*gz
-rw-rw-r-- 1 tasaka1 tasaka1 1623435 2007-09-13 12:34
sirius-0.8.0-7.fc7/sirius-0.8.0.tar.gz
-rw-rw-r-- 1 tasaka1 tasaka1 1647269 2007-09-22 00:45
sirius-0.8.0-8.fc7/sirius-0.8.0.tar.gz
-rw-rw-r-- 1 tasaka1 tasaka1 1647346 2007-10-02 21:19
sirius-0.8.0-9.fc7/sirius-0.8.0.tar.gz
-rw------- 1 tasaka1 tasaka1 1623435 2003-08-22 19:37 sirius-0.8.0.tar.gz
[tasaka1@localhost sirius]$ md5sum *gz */*gz
e01d079ca62aa3cc85f542bb5e9389a5  sirius-0.8.0.tar.gz
e01d079ca62aa3cc85f542bb5e9389a5  sirius-0.8.0-7.fc7/sirius-0.8.0.tar.gz
51b3e3ea619ef43bc5b6cfa9fbda1776  sirius-0.8.0-8.fc7/sirius-0.8.0.tar.gz
84b76cc383a42f8321ccf9c6245de8d0  sirius-0.8.0-9.fc7/sirius-0.8.0.tar.gz
------------------------------------------------------------------
  - Well, the tarball differs between -7, -8, and -9.
    Don't modify tarball itself! tarball used in srpm must be the
    same with the source written in spec file.
    If you have to modify some files in the tarball, patch against
    those files or so.

* Fedora specific compilation flags
  - Fedora specific compilation flags are not honored.
    You can check the fedora compilation flags by
    `rpm --eval %opflags

    For this package, configure.in contains unneeded line
---------------------------------------------------------
CFLAGS="-g -O3"
---------------------------------------------------------
    This overrides CFLAGS value. To pass correct cflags for
    this package, 
---------------------------------------------------------
make %{?_smp_mflags} CFLAGS="%{optflags}"
---------------------------------------------------------
    is needed (i.e. again add CFLAGS option).
Comment 21 Arindam Ghosh 2007-10-03 16:57:27 EDT
(In reply to comment #20)

Issues resolved:

> For -9:
> 
> * Tarball itself

The original tarball sirius-0.8.0.tar.gz is restored (md5sum:
e01d079ca62aa3cc85f542bb5e9389a5). Also I have added a sirius-0.8.0-docs.patch
to patch the required changes.

> * Fedora specific compilation flags
>   - Fedora specific compilation flags are not honored.
>     You can check the fedora compilation flags by
>     `rpm --eval %opflags
> 
>     For this package, configure.in contains unneeded line
> ---------------------------------------------------------
> CFLAGS="-g -O3"
> ---------------------------------------------------------
>     This overrides CFLAGS value. To pass correct cflags for
>     this package, 
> ---------------------------------------------------------
> make %{?_smp_mflags} CFLAGS="%{optflags}"
> ---------------------------------------------------------
>     is needed (i.e. again add CFLAGS option).

Added the CFLAGS option again to make tag for that useless line in configure.in.

The updated links are:
Spec URL: http://makghosh.fedorapeople.org/SPECS/sirius.spec
SRPM URL: http://makghosh.fedorapeople.org/SRPMS/sirius-0.8.0-10.fc7.src.rpm

The Koji scratch build is also successful:
http://koji.fedoraproject.org/koji/taskinfo?taskID=181839
Comment 22 Mamoru TASAKA 2007-10-04 03:42:42 EDT
Okay.

-------------------------------------------------------------
    This package (sirius) is APPROVED by me
-------------------------------------------------------------
Comment 23 Arindam Ghosh 2007-10-04 06:48:34 EDT
New Package CVS Request
=======================
Package Name: sirius
Short Description: Othello game for GNOME 
Owners: makghosh
Branches: F-7
InitialCC: makghosh, mtasaka, rishi
Cvsextras Commits: yes
Comment 24 Arindam Ghosh 2007-10-04 14:23:02 EDT
Package Change Request
======================
Package Name: sirius
New Branches: F-8
Comment 25 Kevin Fenzi 2007-10-04 19:19:35 EDT
As was mentioned on irc, can you remove any reference to "Othello" 
"Othello is a registered trademark of Tsukuda Original, licensed by Anjar Co.,
copyright 1973, 1990 Pressman Toy Corporation."

Please reset the fedora-cvs flag to ? once thats in place? 
Also, unless you plan on releasing a development/test version, do you really
need a F-8 branch now? 
Comment 26 Arindam Ghosh 2007-10-05 06:56:19 EDT
(In reply to comment #25)

> As was mentioned on irc, can you remove any reference to "Othello" 
> "Othello is a registered trademark of Tsukuda Original, licensed by Anjar Co.,
> copyright 1973, 1990 Pressman Toy Corporation."

I have removed reference to "Othello" from following places (Also I replaced
Othello word with Reversi):
* Summary of this bug
* Specfile
* Desktop entry of sirius updating the patch sirius-0.8.0-docs.patch

Is it Okay? Then I can put "fedora-cvs?" again 

The updated links are:
Spec URL: http://makghosh.fedorapeople.org/SPECS/sirius.spec
SRPM URL: http://makghosh.fedorapeople.org/SRPMS/sirius-0.8.0-11.fc7.src.rpm

The Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=184127
Comment 27 Arindam Ghosh 2007-10-05 07:02:17 EDT
Package Change Request
======================
Package Name: sirius
Updated Description: Reversi game for Gnome
Comment 28 Kevin Fenzi 2007-10-05 12:56:48 EDT
Looks good. cvs done. 
Did you really need a F-8 branch? Right now those are only for folks that need
to have a stable version for F-8 and check in major changes for the next devel
cycle. All packages will get a mass F-8 branch before release. 
Comment 29 Arindam Ghosh 2007-10-05 13:14:15 EDT
Okay...F-7 branch will be fine for now. And you can give me the F-8 branch later
as you say before F8 release :-)
Comment 30 Arindam Ghosh 2007-10-05 13:18:42 EDT
(In reply to comment #28)

Thanks a lot!!
Comment 31 Arindam Ghosh 2007-10-05 18:54:19 EDT
Kevin, though "$ make tag" says that "sirius-0_8_0-11_fc8" and
"sirius-0_8_0-11_fc7"  tags are owned by me why is it that in Koji buildsystem
[http://koji.fedoraproject.org/koji/packageinfo?packageID=4658] for all tags of
sirius owner is orphan.

Whereas for my other package gurlchecker, Koji buildsystem
[http://koji.fedoraproject.org/koji/packageinfo?packageID=4605] says the
dist-fc7 tag is owned by me.

Also after "$ make build" was done, sirius is listed in "Your Recent Builds" in
Koji but not there in "Your Packages". gurlchecker is there in "Your packages
but not sirius. Is there something wrong? Please see to it...
Comment 32 Kevin Fenzi 2007-10-05 20:10:46 EDT
Yeah, something didn't sync up right there. 
I think it's fixed now. Can you check koji and see if the problem is solved?
Comment 33 Arindam Ghosh 2007-10-06 00:26:46 EDT
Yeah...Koji seems to work fine now :)
Comment 34 Mamoru TASAKA 2007-10-07 01:53:56 EDT
Please close this bug when rebuild is done and 
request for bodhi is done.
Comment 35 Arindam Ghosh 2007-10-08 08:58:24 EDT
Koji Build Done for devel and F-7 branch.
http://koji.fedoraproject.org/koji/packageinfo?packageID=4658

Bodhi: I have put up sirius-0.8.0-11.fc7 in bodhi. I have pushed the put it up
for testing but it's still pending. Please check it:
https://admin.fedoraproject.org/updates/F7/pending/sirius-0.8.0-11.fc7



Comment 36 Arindam Ghosh 2007-10-08 14:59:19 EDT
Sirius is now pushed for updates-testing in Bodhi
https://admin.fedoraproject.org/updates/sirius
Comment 37 Arindam Ghosh 2007-10-14 16:27:59 EDT
Package Change Request
======================
Package Name: sirius
Updated Fedora CC: makghosh, mtasaka
Comment 38 Arindam Ghosh 2007-10-14 16:39:10 EDT
Kevin, debarshi told me about removing him from CC of this package. So I put up
the package change request :)
Comment 39 Kevin Fenzi 2007-10-15 11:53:23 EDT
Looks like this request was already done via the web interface?
Please reset if you need any cvsadmin action. 

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