PKGBUILD review request: Domino chain

BeChris · 2020-05-23 17:13:46

Hello everybody,
I wanted to install pushover game from aur : https://aur.archlinux.org/packages/pushover/

But this package can't be installed since the game has been renamed Domino chain and moved to another source repository.

Therefore I tried to write a PKGBUILD and it would be appreciated if someone can review it.

I already ran namcap on it and got some warnings about unecessary dependencies but the package is correctly generated by pkgbuild.

I can install the package and play the game.

Here's the PKGBUILD content:

# Maintainer: BeChris (gmail)

pkgname=domino-chain
pkgver=1.1
pkgrel=1
pkgdesc="Clone of the puzzle game Pushover (Amiga)."
arch=('i686' 'x86_64')
url="https://domino-chain.gitlab.io/"
license=('GPL')
depends=('boost-libs' 'fribidi' 'gcc-libs' 'glibc' 'hicolor-icon-theme' 'libpng' 'lua52' 'sdl2' 'sdl2_image' 'sdl2_mixer' 'sdl2_ttf' 'zlib')
makedepends=('gcc' 'gettext' 'git' 'gnu-free-fonts' 'imagemagick' 'pkgconf' 'povray' 'shellcheck')
source=("$pkgname::git+https://gitlab.com/domino-chain/domino-chain.gitlab.io.git#tag=${pkgver}"
        "path-to-fonts.patch"
        "$pkgname.desktop")

prepare() {
  patch -Np1 -i "$srcdir/path-to-fonts.patch"
}

build() {
  cd "$srcdir/$pkgname"
  make
}

package() {
  cd "$srcdir/$pkgname"
  make PREFIX="$pkgdir/usr" install

  install -D -m644 "$srcdir/$pkgname.desktop" "$pkgdir/usr/share/applications/$pkgname.desktop"
}

md5sums=('SKIP'
         'f02b1107acd16f962eee2d8aa8fb6875'
         '4ac36ca43ea455b5c251ec4558d83162')

loqs · 2020-05-23 18:12:58

license=('GPL')

Should it not be GPL3? https://gitlab.com/domino-chain/domino- … .1/LICENSE

makedepends=('gcc' 'gettext' 'git' 'gnu-free-fonts' 'imagemagick' 'pkgconf' 'povray' 'shellcheck')

gcc, gettext and pkgconf are in the base-devel group which is required to be installed before running makepkg so do not need to be listed. boost is not listed and a build in a clean-chroot fails without it.

path-to-fonts.patch fixes the following?

Unable to find FreeSans.ttf
make: *** [Makefile:188: data/fonts/FreeSans.ttf] Error 1
make: *** Waiting for unfinished jobs....

That is where the build failed so I can only check the rest by inspection.

Style criticism, very minor, to keep the same ordering as in man 5 PKGBUILD the cheksums should go just after the sources array.
Just as a note, not a bug, as the working directory is set to $srcdir before each function is called:

  cd "$srcdir/$pkgname"

is equivalent to:

  cd $pkgname

Trilby · 2020-05-23 19:02:16

Do not put $pkgdir in the prefix, that may break things in the binary. $pkgdir is for DESTDIR which the makefile properly uses. The prefix already defaults to /usr so there is no need to explicitly set that. In otherwords, the install line should be:

make DESTDIR="$pkgdir" install

eschwartz · 2020-05-24 03:55:16

shellcheck is a very weird thing to have as a makedepends, what kind of terrible software needs that to build itself?

BeChris · 2020-05-24 16:43:10

loqs wrote:

license=('GPL')
Should it not be GPL3? https://gitlab.com/domino-chain/domino- … .1/LICENSE
makedepends=('gcc' 'gettext' 'git' 'gnu-free-fonts' 'imagemagick' 'pkgconf' 'povray' 'shellcheck')
gcc, gettext and pkgconf are in the base-devel group which is required to be installed before running makepkg so do not need to be listed. boost is not listed and a build in a clean-chroot fails without it.
path-to-fonts.patch fixes the following?
Unable to find FreeSans.ttf
make: *** [Makefile:188: data/fonts/FreeSans.ttf] Error 1
make: *** Waiting for unfinished jobs....
That is where the build failed so I can only check the rest by inspection.
Style criticism, very minor, to keep the same ordering as in man 5 PKGBUILD the cheksums should go just after the sources array.
Just as a note, not a bug, as the working directory is set to $srcdir before each function is called:
  cd "$srcdir/$pkgname"
is equivalent to:
  cd $pkgname

Hi, applied all your suggestions and yes, path-to-fonts.patch fixes compilation error.

BeChris · 2020-05-24 16:43:58

Trilby wrote:

Do not put $pkgdir in the prefix, that may break things in the binary. $pkgdir is for DESTDIR which the makefile properly uses. The prefix already defaults to /usr so there is no need to explicitly set that. In otherwords, the install line should be:
make DESTDIR="$pkgdir" install

PKGBUILD modified accordingly.

BeChris · 2020-05-24 16:48:40

eschwartz wrote:

shellcheck is a very weird thing to have as a makedepends, what kind of terrible software needs that to build itself?

I only followed building instructions for Debian based systems indicated in the homepage:

sudo apt-get install fonts-freefont-ttf g++ gettext imagemagick libboost-filesystem-dev libboost-system-dev libfribidi-dev liblua5.2-dev libpng-dev libsdl2-dev libsdl2-image-dev libsdl2-mixer-dev libsdl2-ttf-dev make pkgconf povray shellcheck zlib1g-dev

eschwartz · 2020-05-24 16:54:55

So I went and looked at the project's git repo, and it defines this target:

README.md: src/doc/doc.sh $(DOC_INCLUDES)
	@mkdir -p $(dir $@)
	shellcheck -f gcc $<
	$< readme >$@

Upstream is insane if they think it's reasonable to force random users to install shellcheck in order to recreate the README.md file as part of building the "default" target.

It's not the only questionable thing they do, they also have an install target which depends on *file targets* e.g.

FILES_INSTALL += $(patsubst %,$(DESTDIR)$(BINDIR)/%,$(FILES_BINDIR))
$(DESTDIR)$(BINDIR)/%: %
	$(INSTALL) -m 755 -d $(dir $@)
	$(INSTALL) -m 755 $< $@

tl;dr Yes, this makedepends on shellcheck, and it's an upstream bug that it does.

Arch Linux

#1 2020-05-23 17:13:46

PKGBUILD review request: Domino chain

#2 2020-05-23 18:12:58

Re: PKGBUILD review request: Domino chain

#3 2020-05-23 19:02:16

Re: PKGBUILD review request: Domino chain

#4 2020-05-24 03:55:16

Re: PKGBUILD review request: Domino chain

#5 2020-05-24 16:43:10

Re: PKGBUILD review request: Domino chain

#6 2020-05-24 16:43:58

Re: PKGBUILD review request: Domino chain

#7 2020-05-24 16:48:40

Re: PKGBUILD review request: Domino chain

#8 2020-05-24 16:54:55

Re: PKGBUILD review request: Domino chain

Board footer