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

[JereBear]

> >
> > 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.

I'll switch to using the local schema. Is there some resource, such as a man page, which explains it?

"file:/" is intentional, and not a typo. From Wikipedia:

    file://path (i.e. two slashes, without a hostname) is never correct, but is often used.

See: https://en.wikipedia.org/wiki/File_URI_scheme#How_many_slashes?

> > # 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.

Shall I leave both of the variable assignments commented out, so that makepkg.conf is respected, and so that the user is aware of possible solutions? Or do you think it's better practice to omit such comments altogether?

I've not yet found a technique for configuring makepkg when building in a chroot with makechrootpkg. Here's the command I'm currently using:

    makechrootpkg \
        -c \
        -I /var/cache/pacman/pkg/binkplayer-bin-2.7J-1-x86_64.pkg.tar.xz \
        -r /var/lib/archbuild/extra-x86_64/

That's what prompted me to insert these variable assignments into PKGBUILD in the first place, rather than just advising the user to configure makepkg.conf. I'm happy to read a man page or wiki page if there's advice on this topic.

It's too bad that multi-threaded compression is incompatible with reproducible builds. :-(

> > prepare() {
> >   snip!
> > }
>
> 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.

Your intuition is correct: makeself can extract the blob that it prepends. This command will do so:

    pyre-${pkgver//./_}.sh --keep --target data

But GOG games consist of a makeself script, mojosetup installer, and a zip archive containing the game assets, concatenated in that order. As a result, the command above extracts the *mojosetup* installer, not the game itself. Meanwhile, the mojosetup installer is an interactive GUI application that installs the game into the calling user's home directory. Not what I want. As a fall-back solution, I'm stripping the makeself script and mojosetup installer with tail. In the simplest case, it looks like the following:

    tail --bytes=+757175 pyre-${pkver//./_}.sh > pyre.zip
    unzip -d pyre pyre.zip

However, I dislike hard-coding the number of bytes to strip, and I dislike calling unzip and hoping for the best, so most of prepare() is concerned with calculating the number of bytes to strip. I'm happy to explain the byte calculation logic in detail if you like.

I'd happily use pipes so as to avoid a large temporarily zip file:

    tail --bytes=+757175 pyre-${pkver//./_}.sh | unzip -d pyre

...but unzip can't read from stdin.

> > 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?

symlinking to start.sh is not an option here. :-( Here's the contents of gog-pyre:

    #!/usr/bin/env bash

    # Setting TERM prevents the application from crashing with "System.Exception:
    # Magic number is wrong: 542." See: https://github.com/mono/mono/issues/11557
    TERM=xterm /opt/gog-pyre/start.sh

I suspect that this workaround is only necessary because Pyre bundles dependencies.

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

I only glanced at the log. IIRC, it contains information like the current graphics card. I'll try symlinking to /dev/stdout. That seems like a much better idea than creating a world-writable file, and it's more useful than symlinking to /dev/null.

> Eli Schwartz
> Bug Wrangler and Trusted User

Thank you for your time!