[aur-general] package review request: gog-pyre

[Eli Schwartz]

On 1/4/19 11:21 AM, JereBear via aur-general wrote:
> Would someone be willing to review the gog-pyre package I've put together? I've included just the PKGBUILD below, but I'm happy to include the entire enchilada with git-format-patch. IIRC, this mailing list strips attachments, so I've included the PKGBUILD inline. I've successfully compiled and installed the package, and can verify that the application works. (At least, the first five minutes of gameplay work.)
> 
> 
> 
> 
> 
> # Maintainer: Jeremy Audet <jerebear at protonmail.com>
> # shellcheck shell=bash
> # shellcheck disable=SC2034,SC2154
> 
> pkgname='gog-pyre'
> pkgver='1.50427.11957.23366'
> pkgrel=1
> pkgdesc='A party-based RPG in which you lead a band of exiles to freedom. (GOG version)'
> url='https://www.gog.com/game/pyre'
> license=('custom')
> groups=('games' 'gog')
> # May also work on i686, but untested. Most 64-bit dependencies (e.g. sdl2) are
> # bundled with the game.
> arch=('x86_64')
> depends=('binkplayer-bin')
> makedepends=('unzip')
> # Copy the game file into the current directory before building.
> source=("file:/pyre_${pkgver//./_}.sh"

This is not file:// because it is typoed.

But you should use local:// since file:// is an actual makepkg.conf
protocol and will be downloaded using curl from an on-disk location.

>         "${pkgname}.desktop"
>         "${pkgname}")
> sha256sums=('f42b4c55975df69e8d98069dea72178320485f6bb8b8a1573490e10331fa17d2'
>             'f66a8ad19f05d826afbe2a9375d1b6317f9166dc304ba1be2b92913064bf6971'
>             '5b91e71101efe303986851df828d2e3934715232bfc12bb3634d1ec49cf70e42')
> 
> # Single-threaded compression of a multi-gigabyte package is time-consuming.
> # Possible solutions are to skip compression or throw more threads at it.
> # PKGEXT='.pkg.tar'
> COMPRESSXZ=(xz --to-stdout --compress --threads 0 -)

Users who want to avoid compressing packages because they either don't
need the storage savings or aren't hosting them over a network, should
set PKGEXT in their makepkg.conf -- it's not the job of the package to
enforce this.

Meanwhile, single-threaded compression is the default in makepkg because
multithreaded compression results in packages that cannot be
reproducibly built. Even decompressing and recompressing the tarball
will result in changes, so it would also break delta packages.

> prepare() {
>   # GOG games for linux appear to consist of a concatenated makeself shell
>   # script, mojosetup gzip archive, and game zip archive. I've not found a way
>   # to make the scripts strip themselves, and use tail as a fall-back. A hacky
>   # way of verifying $header_bytes is to call unzip on the game file, and to see
>   # how many bytes it skips while searching for the start of the zip archive.
>   bash "pyre_${pkgver//./_}.sh" --dumpconf > conf
> 
>   local makeself_lines
>   # shellcheck disable=SC1091
>   makeself_lines="$(( "$(source conf && echo "${OLDSKIP}")" - 1 ))"
> 
>   local makeself_bytes
>   makeself_bytes="$(
>     head "pyre_${pkgver//./_}.sh" --lines "${makeself_lines}" \
>     | wc --bytes
>   )"
> 
>   local mojosetup_bytes
>   # shellcheck disable=SC1091
>   mojosetup_bytes="$(source conf && echo "${filesizes}")"
> 
>   local header_bytes
>   header_bytes="$((makeself_bytes + mojosetup_bytes))"
> 
>   tail --bytes=+"$((header_bytes + 1))" "pyre_${pkgver//./_}.sh" > pyre.zip
>   unzip -qd pyre pyre.zip
> }

This looks very complicated, shouldn't there be a way to tell it to
extract to a temporary directory? I'm not overly familiar with makeself
installers though.

> package() {
>   # game files and launcher
>   install -d "${pkgdir}/opt/${pkgname}"
>   cp -rt "${pkgdir}/opt/${pkgname}" "${srcdir}/pyre/data/noarch/"*
>   chmod 755 "${pkgdir}/opt/${pkgname}/start.sh"
>   install -Dm755 "${srcdir}/${pkgname}" "${pkgdir}/usr/bin/${pkgname}"

What does this script do? Most such script launchers (start.sh) should
in theory work okay if you symlink them to /usr/bin, is this not an
option here?

>   # The game writes to DebugLog.txt, which results in a permission denied error.
>   # AFAICT, this is a bug, and it should write to a location suited for the
>   # purpose, such as a subdirectory of the calling user's $XDG_CACHE_HOME. A
>   # simple solution is to symlink DebugLog.txt to /dev/null. Making DebugLog.txt
>   # world-writable is less secure but more useful.
>   chmod ugo+w "${pkgdir}/opt/${pkgname}/game/DebugLog.txt"

What is this debuglog needed for? I wonder if it would be better to
symlink it to /dev/stdout

>   # desktop environment integration
>   install -Dm644 \
>     "${srcdir}/pyre/data/noarch/support/icon.png" \
>     "${pkgdir}/usr/share/pixmaps/${pkgname}.png"
>   install -Dm644 \
>     "${srcdir}/${pkgname}.desktop" \
>     "${pkgdir}/usr/share/applications/${pkgname}.desktop"
> 
>   # license
>   install -Dm644 \
>     "${srcdir}/pyre/data/noarch/docs/End User License Agreement.txt" \
>     "${pkgdir}/usr/share/licenses/${pkgname}/LICENSE"
> }
> 
> # vim: ts=2 sw=2 et:
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/aur-general/attachments/20190104/58c39444/attachment.asc>