Bug 1006197 - Review Request: stockfish - Powerful open source chess engine
Review Request: stockfish - Powerful open source chess engine
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Raphael Groner
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-DEADREVIEW stockfish
  Show dependency treegraph
 
Reported: 2013-09-10 04:20 EDT by Dhiru Kholia
Modified: 2015-02-25 15:16 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: stockfish (view as bug list)
Environment:
Last Closed: 2014-12-10 07:12:58 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
projects.rg: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Dhiru Kholia 2013-09-10 04:20:05 EDT
Spec URL: https://bitbucket.org/dhiru/packages/raw/master/stockfish/stockfish.spec
SRPM URL: https://bitbucket.org/dhiru/packages/raw/master/stockfish/stockfish-4-1.fc19.src.rpm
Description: 

Stockfish is a free UCI chess engine derived from Glaurung 2.1. It is not a
complete chess program, but requires some UCI compatible GUI (like XBoard with
PolyGlot, eboard, Arena, Sigma Chess, Shredder, Chess Partner or Fritz) in
order to be used comfortably. Read the documentation for your GUI of choice for
information about how to use Stockfish with your GUI.

Fedora Account System Username: halfie

Original RFE: https://bugzilla.redhat.com/show_bug.cgi?id=820796

Koji Task: http://koji.fedoraproject.org/koji/taskinfo?taskID=5916924
Comment 1 Christopher Meng 2013-09-10 05:30:26 EDT
1. I've edited the common rpmlint error solutions page:

http://fedoraproject.org/wiki/Common_Rpmlint_issues#wrong-file-end-of-line-encoding

Please avoid using dos2unix if possible.

2. %{_var} should be %{_localstatedir}

3. 

# polyglot file
install -m 644 Book.bin $RPM_BUILD_ROOT%{_var}/lib/games/%{name}

# the opening book
install -m 644 polyglot.ini $RPM_BUILD_ROOT%{_var}/lib/games/%{name}

missing -p option to preserve the timestamp.

4. Source1 should be:

Source1:        http://cl.ly/3x333m0G173F/download/%{name}-%{book_version}-book.zip

5. I've sent an email to website maintainer of stockfish, hope they can add a sourcetarball download link on the website.

6. No %{optflags} during compiling?

I can see:

debug = no

optimize = yes (Research again contexts found:

# optimize = yes/no   --- (-O3/-fast etc.) --- Enable/Disable optimizations

?O.0?)

7. Why not use make install?

8. If one want to use your package on ppc(Peter Lemenkev), I think the installation way you've defined is not good. IMO upstream should use configure script to do that but actually not.
Comment 2 Michael Schwendt 2013-09-10 06:02:41 EDT
* Run rpmlint (or rpmlint -i for more helpful output) on the src.rpm and all
built rpms. Feel free to ignore obvious false positives in the report, but fix
anything else. Preferably add a comment here about whether/when you think what
rpmlint reports is correct or incorrect.


> %setup -q -n %{upstream_name}-%{upstream_short_name}_%{version}
> 
> unzip %{SOURCE1}

%setup can unzip, too. You don't need a second line for that. Use:

  %setup -q -a 1 -n %{upstream_name}-%{upstream_short_name}_%{version}


> 2. %{_var} should be %{_localstatedir}

Giving a rationale/explanation would be more helpful. We don't use macros just for fun. Nothing during the build uses %{_localstatedir}. The %configure macro isn't used either (it would pass --localstatedir=/var to the build framework).

So, using %{_var} is fine, but only provided that you use it consistently. The sed substitution uses a hardcoded /var, so I would either use /var or %{_var} everywhere, but not mix them.


> %build

That section currenly only supports a few architectures and doesn't call make for unknown archs. ExclusiveArch or ExcludeArch usage is missing:
https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Support


> install -m 644 Book.bin $RPM_BUILD_ROOT%{_var}/lib/games/%{name}

To be verified. Is that path correct?

The Polyglot .ini file contains a /usr/share/games/stockfish/Book.bin path.

Also consider contacting the Fedora Games SIG, since they tell that runtime configuration files should go in /var/games/%{name} instead.

  http://fedoraproject.org/wiki/SIGs/Games
   -> http://fedoraproject.org/wiki/SIGs/Games/Packaging
Comment 3 Dhiru Kholia 2013-09-10 14:09:36 EDT
(In reply to Christopher Meng from comment #1)
> 6. No %{optflags} during compiling?

Yes, we respect the upstream flags since this is a 100% CPU bound application and they are experts when it comes to "ricing" ;)

> I can see:
> 
> debug = no
> optimize = yes (Research again contexts found:

These settings are fine and are desirable for good performance.

> 8. If one want to use your package on ppc(Peter Lemenkev), I think the
> installation way you've defined is not good. IMO upstream should use
> configure script to do that but actually not.

I don't have a good solution for this problem (is there one you know?).

Also, I don't think that the upstream will consider using configure stuff any time soon.
Comment 4 Michael Schwendt 2013-09-10 14:15:05 EDT
That's a surprising response, given that the -debuginfo package is empty, and the guidelines have been ignored:
https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
Comment 5 Dhiru Kholia 2013-09-10 15:07:28 EDT
Using "debug = no" was a mistake on my part, which is fixed now. Michael, thanks for pointing it out. The -debuginfo package is fine now.

I still need to think about using (or fixing) those upstream provided compiler flags.

Spec URL: https://bitbucket.org/dhiru/packages/raw/master/stockfish/stockfish.spec
SRPM URL: https://bitbucket.org/dhiru/packages/raw/master/stockfish/stockfish-4-2.fc19.src.rpm

http://koji.fedoraproject.org/koji/taskinfo?taskID=5920402
Comment 6 Ralf Corsepius 2013-09-11 02:54:41 EDT
(In reply to Dhiru Kholia from comment #5)

> I still need to think about using (or fixing) those upstream provided
> compiler flags.
Well, you MUST use $RPM_OPT_FLAGS/%optflags to make sure the binaries are compatible to all current and futures architectures and features supported by Fedora and its build infrastructure.

That said,
* "-g -O3" doesn't provide substantial advantages over "-g -O2" 
* "-msse" is implied by Fedora's gcc's implicit default set of cflags on architectures it is applicatible.
Comment 7 matthew gatto 2014-01-22 12:58:40 EST
Is there a downloadable stockfish RPM build on koji.fedoraproject.org? I was linking to http://kojipkgs.fedoraproject.org//work/tasks/6926/5916926/stockfish-4-1.fc20.x86_64.rpm on the PyChess engines wiki at http://code.google.com/p/pychess/wiki/ChessEngines , but that link isn't working anymore.
Comment 8 Christopher Meng 2014-01-23 02:01:46 EST
(In reply to matthew gatto from comment #7)
> Is there a downloadable stockfish RPM build on koji.fedoraproject.org? I was
> linking to
> http://kojipkgs.fedoraproject.org//work/tasks/6926/5916926/stockfish-4-1.
> fc20.x86_64.rpm on the PyChess engines wiki at
> http://code.google.com/p/pychess/wiki/ChessEngines , but that link isn't
> working anymore.

That was a build only available in 7 days after it got built, now it's unavailable.

You can use rpmbuild --rebuild to build the SRPM linked in comment 5, but I strongly recommend that you should wait for a while before this get approval of the review.

The RPM is not good still.
Comment 10 Raphael Groner 2014-10-05 10:56:14 EDT
(In reply to Dhiru Kholia from comment #5)
…
> Spec URL:
> https://bitbucket.org/dhiru/packages/raw/master/stockfish/stockfish.spec
> SRPM URL:
> https://bitbucket.org/dhiru/packages/raw/master/stockfish/stockfish-4-2.fc19.
> src.rpm

% rpmlint stockfish.spec stockfish-4-2.fc22.x86_64.rpm stockfish-4-2.fc19.src.rpm 
stockfish.spec: W: invalid-url Source1: stockfish-231-book.zip
Should be OK, since you're providing the file in your SRPM.

stockfish.x86_64: W: spelling-error %description -l en_US eboard -> board, aboard, e board
Maybe easily fixable.

stockfish.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/stockfish/Copying.txt
You may use the tool dos2unix to fix this.

stockfish.x86_64: W: no-manual-page-for-binary stockfish
Not needed if upstream does not provide.

stockfish.src: W: spelling-error %description -l en_US eboard -> board, aboard, e board
Ignored. False positive.


Remove ExclusiveArch cause those are now the default primary ones in rawhide.

You should use macros where possible:
- sed -i "/Book File/s:book.bin:/usr/share/stockfish/Book.bin:" src/ucioption.cpp
+ sed -i "/Book File/s:book.bin:%{_datadir}/%{name}/Book.bin:" src/ucioption.cpp

The folder /var/games is marked as deprecated in FHS. What is /var/games/stockfish/polyglot.ini about, configuration? If so, consider to move the file into folder /etc/stockfish/ instead and patch the source to find it there.
Comment 11 Raphael Groner 2014-12-10 03:13:50 EST
Any progress here?
Comment 12 Dhiru Kholia 2014-12-10 04:00:20 EST
Hi,

Feel free to take over (adopt) this package. I am currently short on free time.

Dhiru
Comment 13 Raphael Groner 2014-12-10 07:12:58 EST
(In reply to Dhiru Kholia from comment #12)
> Feel free to take over (adopt) this package. I am currently short on free
> time.

As you wish … closing.

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