PKGBUILD review request - toga2

ds77 · 2014-01-02 23:35:54

Hi,

as this is my first PKGBUILD, I would like to ask you for a review.

pkgdesc="Toga II: Fruit chess engine derivative (including opening book and egbb)"

note: I have used the same path patching pattern as stockfish PKGBUILD is using, the patch is inlined so I can use path variable in the script.
note2: egbb (end game bitboards) package has around 200 MB

Thanks for your comments.

 
# Maintainer: Dusan Saiko <dusan.saiko ..at.. gmail ..dot.. com>
pkgname=toga2
pkgver=3.0
pkgrel=1
pkgdesc="Toga II: Fruit chess engine derivative (including opening book and egbb)"
arch=('i686' 'x86_64')
url="https://chessprogramming.wikispaces.com/Toga"
license=('GPL')
depends=('gcc-libs')
makedepends=('unzip')

source=(
    "$pkgname-$pkgver.zip::http://www.computerchess.info/tdbb/phpBB3/download/file.php?id=1402"
    "egbbdll.zip::http://sites.google.com/site/dshawul/egbbdll_new.zip?attredirects=0"
    "http://cl.ly/3x333m0G173F/download/stockfish-231-book.zip"
    "http://www.olympuschess.com/bitbases/egbb-3.0.zip"
)

noextract=("egbbdll.zip")

md5sums=('24656863a79e2765ec7ec778bbee3005'
         '5e0865f7690a666feb3d7a8c24dce4f5'
         '9e51c2e57d8b55bbc588150033e4b133'
         '7356d517f428a0fc85f58058248494ea')

_toga2shared=/usr/share/toga2
_egbbpath=${_toga2shared}/egbb

prepare() {
	cd $srcdir
	#RENAME EXTRACTED FOLDER
	mv toga-30-ja-release $pkgname-$pkgver
	#REMOVE BINARY BUILDS
	rm -rf $pkgname-$pkgver/Android
	rm -rf $pkgname-$pkgver/Linux
	rm -rf $pkgname-$pkgver/Windows
	cd $pkgname-$pkgver/src
	dos2unix *
	#PATCH END GAME BITBOARDs PATH
	patch protocol.cpp <<EOF
55c55
< static char * dirptr = "C:/egbb";
---
> static char * dirptr = "${_egbbpath}/";
EOF
	patch main.cpp <<EOF
25c25
< static char * egbb_path = "c:/egbb/";
---
> static char * egbb_path = "${_egbbpath}/";
EOF
	patch option.cpp <<EOF
40,41c40,41
<    { "BookFile", true, "performance.bin", "string", "", NULL },
<    { "Bitbases Path", true, "c:/egbb/", "string", "", NULL },
---
>    { "BookFile", true, "${_toga2shared}/performance.bin", "string", "", NULL },
>    { "Bitbases Path", true, "${_egbbpath}/", "string", "", NULL },
EOF

	
	#UNZIP EGBBDLL
	cd $srcdir
	unzip egbbdll.zip -d egbbdll
	cd egbbdll
	dos2unix *
	#PATCH EGBBDLL
	patch cache.h <<EOF
4a5
> #include <cstring>
EOF
}

build() {
	#GET ARCHITECTURE
	if [[ "$CARCH" == "i686" ]];
	then
	    _arch=x86-32
	elif grep popcnt /proc/cpuinfo 2>&1
	then
	    _arch=x86-64-modern
	else
	    _arch=x86-64
	fi

	#BUILD TOGA2
	cd "$srcdir/$pkgname-$pkgver/src"
	make ARCH=$_arch
	
	#BUILD EGBBDLL
	cd "$srcdir/egbbdll"
	make ARCH=$_arch
	
}


package()
{
	install -D -m755 "$srcdir/$pkgname-$pkgver/src/toga2"		"${pkgdir}/usr/bin/toga2"
	install -D -m755 "$srcdir/egbbdll/egbbso.so"			"${pkgdir}${_egbbpath}/egbbso.so"
	install -D -m644 Book.bin $pkgdir${_toga2shared}/performance.bin
	cd "$srcdir/egbb"
	for f in *
	do
	    install -D -m644 $f $pkgdir${_egbbpath}/$f
	done
}

progandy · 2014-01-03 00:15:19

Why do you have egccdll in the noextract-array? Edit: OK, it extracts in the source-root without a subdirectory.
For the patches, maybe some sed-lines would be easier to read.

Last edited by progandy (2014-01-03 00:23:00)

ds77 · 2014-01-03 00:21:02

Why have you egccdll in the noextract-array?

because it would unzip into root. I unzip it manually in prepare() into a subfolder

unzip egbbdll.zip -d egbbdll

Last edited by ds77 (2014-01-03 00:22:14)

Scimmia · 2014-01-03 00:56:04

So you inlined the patches to use the variables, why? What's the advantage of having the variable over just hardcoding the paths? Personally I would just be using sed instead of patch anyway.

Looks like dos2unix is a makedep

You have instances of $srcdir and $pkgdir throughout that aren't quoted.

You don't really need a for loop in the package function. install -m644 * "$pkgdir${_egbbpath}/" should work as long as you make the dir first.

Last edited by Scimmia (2014-01-03 00:57:37)

fsckd · 2014-01-03 14:33:35

Scimmia wrote:

What's the advantage of having the variable over just hardcoding the paths?

Variables expand in heredocs.

progandy · 2014-01-03 14:38:00

fsckd wrote:

Scimmia wrote:
What's the advantage of having the variable over just hardcoding the paths?
Variables expand in heredocs.

Why do you need variables at all? The paths are really static, so just create one big patch that fixes all paths and apply it to your source tree

ds77 · 2014-01-03 15:02:50

thanks for your response, I have cleaned up the PKGBUILD to following (probably) final form:

# Maintainer: Dusan Saiko <dusan.saiko ..at.. gmail ..dot.. com>
pkgname=toga2
pkgver=3.0
pkgrel=1
pkgdesc="Toga II: Fruit chess engine derivative (including opening book and egbb)"
arch=('i686' 'x86_64')
url="http://chessprogramming.wikispaces.com/Toga"
license=('GPL')
depends=('gcc-libs')
makedepends=('unzip')

#info: egbb-3.0.zip has approximatelly 220 MB
source=(
    "${pkgname}-${pkgver}.zip::http://www.computerchess.info/tdbb/phpBB3/download/file.php?id=1402"
    "egbbdll.zip::http://sites.google.com/site/dshawul/egbbdll_new.zip?attredirects=0"
    "http://cl.ly/3x333m0G173F/download/stockfish-231-book.zip"
    "http://www.olympuschess.com/bitbases/egbb-3.0.zip"
    "toga2.patch"
    "egbbdll.patch"
)

md5sums=('24656863a79e2765ec7ec778bbee3005'
         '5e0865f7690a666feb3d7a8c24dce4f5'
         '9e51c2e57d8b55bbc588150033e4b133'
         '7356d517f428a0fc85f58058248494ea'
         'fe4ecbe4edba72ae2ea439bfb75cf94a'
         'c17842d6f1359f7737a7b6133ce34510')

#would extract source files into root, will be unzipped later on into subfolder
noextract=("egbbdll.zip")

prepare() {
	cd "${srcdir}"
	#RENAME EXTRACTED FOLDER
	mv toga-30-ja-release "${pkgname}-${pkgver}"
	cd "${pkgname}-${pkgver}"
	#APPLY PATCH
	patch -s -p0 < "${srcdir}/toga2.patch"
	
	#UNZIP EGBBDLL INTO SUBDIRECTORY
	cd "${srcdir}"
	unzip egbbdll.zip -d egbbdll
	#PATCH EGBBDLL
	patch -s -p0 < "${srcdir}/egbbdll.patch"
}

build() {
	#GET ARCHITECTURE
	if [[ "${CARCH}" == "i686" ]];
	then
	    _arch=x86-32
	elif grep popcnt /proc/cpuinfo 2>&1
	then
	    _arch=x86-64-modern
	else
	    _arch=x86-64
	fi

	#BUILD TOGA2
	cd "${srcdir}/${pkgname}-${pkgver}/src"
	make ARCH=$_arch
	
	#BUILD EGBBDLL
	cd "${srcdir}/egbbdll"
	make ARCH=$_arch
}

package()
{
	install -D 	-m755 "${srcdir}/${pkgname}-${pkgver}/src/toga2"     "${pkgdir}/usr/bin/toga2"
	install -D 	-m755 "${srcdir}/egbbdll/egbbso.so"                         "${pkgdir}/usr/share/toga2/egbb/egbbso.so"
	install -D 	-m644 Book.bin                                                      "${pkgdir}/usr/share/toga2/performance.bin"
	
	cd "${srcdir}/egbb"
	install 	-m644 *                                                                 "${pkgdir}/usr/share/toga2/egbb/"
}

Last edited by ds77 (2014-01-03 15:03:26)

ds77 · 2014-01-03 15:23:23

one more question:
I have posted the package, installation works fine, just downloading end game bitboards from olympuschess.com is very very slow.
Is it legal to upload the file to some other place? There is no licensing information for the bitboards.

thanks

fsckd · 2014-01-03 17:34:12

@ds77: Ouch, that is slow. I suggest contacting upstream and asking them. Are you sure there's no license in the archive?

progandy wrote:

fsckd wrote:
Scimmia wrote:
What's the advantage of having the variable over just hardcoding the paths?
Variables expand in heredocs.
Why do you need variables at all? The paths are really static, so just create one big patch that fixes all paths and apply it to your source tree

It is gone in the revised PKGBUILD. I still want to make this point. If there's a value which is used in more than one place, such as the prepare() and the package() functions, it is perfectly fine to assign it to a variable.

Scimmia · 2014-01-03 18:14:13

fsckd wrote:

@ds77: Ouch, that is slow. I suggest contacting upstream and asking them. Are you sure there's no license in the archive?
progandy wrote:
fsckd wrote:
Variables expand in heredocs.
Why do you need variables at all? The paths are really static, so just create one big patch that fixes all paths and apply it to your source tree
It is gone in the revised PKGBUILD. I still want to make this point. If there's a value which is used in more than one place, such as the prepare() and the package() functions, it is perfectly fine to assign it to a variable.

Nobody said it was wrong, but when it complicates things for no apparent benefit, why would you?

Last edited by Scimmia (2014-01-03 18:14:28)

ds77 · 2014-01-03 19:16:18

according to http://chessprogramming.wikispaces.com/Scorpio+Bitbases
respective http://www.talkchess.com/forum/viewtopi … 72&start=7

"Scorpio Bitbases are open source, since October 2013 distributed under the BSD license"

so I think it should be fine to upload it to my server ?

fsckd · 2014-01-03 19:36:30

@ds77: BSD license would mean you can mirror it (unless they modified the license to prohibit that). If it is BSD licensed, the license file should be included in the source archive. Confirm it is there.

Scimmia wrote:

Nobody said it was wrong, but when it complicates things for no apparent benefit, why would you?

I was answering your earlier question, not saying your are wrong. I don't understand how those variables complicate things.

ds77 · 2014-01-03 19:46:20

no, there is no license file inside the archive.

But as Bitboard tables are just data structure with all combinations of a chess board (of some criteria), i think there is nothing to copyright for the content, only for the format. And that's BSD ..

progandy · 2014-01-03 22:23:37

fsckd wrote:

@ds77: BSD license would mean you can mirror it (unless they modified the license to prohibit that). If it is BSD licensed, the license file should be included in the source archive. Confirm it is there.
Scimmia wrote:
Nobody said it was wrong, but when it complicates things for no apparent benefit, why would you?
I was answering your earlier question, not saying your are wrong. I don't understand how those variables complicate things.

The variables require the patch to be part of the PKGBUILD, and that just looks ugly and is hard to read.

Or wait, I just had an idea:

sed 's:pathvariable:/usr/share/:g' paths.patch | patch -p0

Last edited by progandy (2014-01-03 22:24:44)

fsckd · 2014-01-04 03:22:11

progandy wrote:

The variables require the patch to be part of the PKGBUILD, and that just looks ugly and is hard to read.

Fair point.

sed 's:pathvariable:/usr/share/:g' paths.patch | patch -p0

Trilby · 2014-01-04 04:04:48

Why does the PKGBUILD list the license as "GPL"? It seems there is some question about whether it is BSD or not, but it sounds like it is not GPL.

ds77 · 2014-01-04 22:33:19

toga2 - the main part, is GPL
so is stockfish opening book
just the end game bit base part is under BSD

final PKGBUILD states all three licences and includes them under /usr/share/licences/toga2/

Arch Linux

#1 2014-01-02 23:35:54

PKGBUILD review request - toga2

#2 2014-01-03 00:15:19

Re: PKGBUILD review request - toga2

#3 2014-01-03 00:21:02

Re: PKGBUILD review request - toga2

#4 2014-01-03 00:56:04

Re: PKGBUILD review request - toga2

#5 2014-01-03 14:33:35

Re: PKGBUILD review request - toga2

#6 2014-01-03 14:38:00

Re: PKGBUILD review request - toga2

#7 2014-01-03 15:02:50

Re: PKGBUILD review request - toga2

#8 2014-01-03 15:23:23

Re: PKGBUILD review request - toga2

#9 2014-01-03 17:34:12

Re: PKGBUILD review request - toga2

#10 2014-01-03 18:14:13

Re: PKGBUILD review request - toga2

#11 2014-01-03 19:16:18

Re: PKGBUILD review request - toga2

#12 2014-01-03 19:36:30

Re: PKGBUILD review request - toga2

#13 2014-01-03 19:46:20

Re: PKGBUILD review request - toga2

#14 2014-01-03 22:23:37

Re: PKGBUILD review request - toga2

#15 2014-01-04 03:22:11

Re: PKGBUILD review request - toga2

#16 2014-01-04 04:04:48

Re: PKGBUILD review request - toga2

#17 2014-01-04 22:33:19

Re: PKGBUILD review request - toga2

Board footer