You are not logged in.

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

BeChris
Member
Registered: 2020-05-23
Posts: 4

PKGBUILD review request: Domino chain

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')

Offline

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

loqs
Member
Registered: 2014-03-06
Posts: 11,000

Re: PKGBUILD review request: Domino chain

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

Offline

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

Trilby
Inspector Parrot
Registered: 2011-11-29
Posts: 23,727
Website

Re: PKGBUILD review request: Domino chain

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

"UNIX is simple and coherent..." - Dennis Ritchie, "GNU's Not UNIX" -  Richard Stallman

Offline

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

eschwartz
Trusted User/Bug Wrangler
Registered: 2014-08-08
Posts: 3,439

Re: PKGBUILD review request: Domino chain

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


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

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

BeChris
Member
Registered: 2020-05-23
Posts: 4

Re: PKGBUILD review request: Domino chain

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.

Offline

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

BeChris
Member
Registered: 2020-05-23
Posts: 4

Re: PKGBUILD review request: Domino chain

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.

Offline

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

BeChris
Member
Registered: 2020-05-23
Posts: 4

Re: PKGBUILD review request: Domino chain

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

Offline

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

eschwartz
Trusted User/Bug Wrangler
Registered: 2014-08-08
Posts: 3,439

Re: PKGBUILD review request: Domino chain

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.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

Board footer

Powered by FluxBB