[aur-general] RFC: PKGBUILD for nixnote2-release-bin

[Eli Schwartz]

On 11/5/18 7:07 AM, Tom Hale wrote:
> Could I please have comments on the below (and attached) PKGBUILD for
> nixnote2-release-bin.
> 
> To pre-empt some comments:
> 
> 1) This will download the latest binary from GitHub. I'm open to a
> different package name to denote that it's not a '-git' or
> build-from-latest-source-code, but rather a
> build-from-latest-binary-release.
> 
> You may say that building from latest binary release against the
> packaging guidelines, but I can't see where. Please see prior art
> regarding this at:
> 
> https://bbs.archlinux.org/viewtopic.php?pid=1811590#p1811590

Prior art? That would be my prior art where I stated that I feel this is
strongly against the spirit of the AUR at an absolute minimum?

Where it was mentioned that this breaks the concept of providing useful
metadata in the .SRCINFO?

If you're not interested in listening to advice, why bother asking us to
give it? Just do whatever you want (as long as you don't upload it to
the AUR).

> 2) There is already a '-git' version of this package, but I want to
> provide another option which:
> * Is more stable - based on versioned releases rather than the
> 'continuous' branch.
> * Has a quicker install - doesn't require waiting for compiling

The purpose of an AppImage is to circumvent package managers in order to
provide rapid binary releases. I see zero benefit in repackaging this,
and several reasons why an AUR package is inferior to the original AppImage.

For example, the AppImage does not require root, does the same thing
regardless, provides automatic update notifications, provides delta
updates which conserve bandwidth...

> Question:
> 
> I've found that even with pointing the .desktop file explicitly at the
> pathname of the icon, xfce4 still doesn't pick it up. Why not?
> 
> ===============================================================
> 
> # Maintainer: Tom Hale <tom[noodle]hale[point]ee>
> # Contributor: twa022 <twa022 at gmail dot com>
> 
> OPTIONS=(!strip)  # Stripping an AppImage renders it non-functional

OPTIONS is meant for makepkg.conf alone. For PKGBUILDs we provide an
override, which is lowercased.

> _pkgname=nixnote2
> pkgname=${_pkgname}-release-bin
> pkgver=2.1.0.beta4
> pkgrel=1
> pkgdesc='Evernote clone (formerly Nevernote) - latest binary release'
> url="https://github.com/robert7/$_pkgname"
> arch=(x86_64)
> license=(GPL3)
> depends=(hicolor-icon-theme zlib)
> makedepends=(git)

This needs git because why???

> conflicts=(nixnote2-git)

This should only conflict with the base nixnote2 package, and all
repackaged versions should provide and conflict the base package --
rather than pinning specific repackaged versions. In its current form,
this will not conflict with nixnote2 at all, but attempting to install
both will cause pacman to error out with:

 error: failed to commit transaction (conflicting files)
nixnote2-release-bin: /path/to/file exists in filesystem (owned by nixnote2)

> provides=("nixnote=${pkgver%.r*}" "$_pkgname=${pkgver%.r*}")
> replaces=(nevernote nixnote nixnote-beta)
> source=("$pkgname:desktop.patch")

Why does this filename contain an extremely confusing ":"? It is bound
to trip people up by making them think it is a double "::" -- which is
PKGBUILD syntax for renaming a downloaded source.

Why haven't you uploaded this file for review?

> sha256sums=(4f12cddc49e0c694f41e344acebc0d8a3bce07a3496098eb2fae258ba9044967)
> 
> noextract=("${source[@]%%::*}")  # Don't extract anything

There is nothing to extract, since your only source file is a .patch
which is not an archive to begin with.

> _version_file=$pkgname-PKGBUILD-version  # For version from the download
> URL

This does absolutely nothing and is uselessly useless.

> _appimage_path=$(curl -s 'https://github.com/robert7/nixnote2/releases' |
>                   sed -En
> 's:.*(/releases/download/v[^"/]+/[^"/]+[Aa]pp[Ii]mage)".*:\1:p' |
>                   head -n1)

This accesses the internet every time you even try to *parse* the PKGBUILD.

> _appimage=${_appimage_path##*/}  # basename of AppImage
> 
> prepare() {
>    wget -N "$url/$_appimage_path"  # Only download again if server
> version is newer

You make git a makedepends which you don't need, then use wget which is
not a makedepends and is not on peoples' systems by default. And makepkg
uses a configurable DLAGENTS for this, which defaults to a downloader
that is actually installed as a makedepends of pacman/makepkg.

As for the actual source download, it circumvents makepkg's source
handling and avoids caching. The executable is totally untrusted
content, downloaded without checksums, so makepkg cannot even know
whether the file downloaded correctly, let alone whether it is the same
one you (hopefully) verified as accurate and maybe even safe.

Furthermore, the server should *never* have a newer version of a
versioned release artifact, and if it does, that should raise alarm
bells indicating the author's github account was hacked and malware was
uploaded.

>    # Make download executable
>    chmod 755 "$_appimage"
> 
>    # Extract user resources from inside the AppImage
>    # All files in the directories specified are extracted
>    # Note that --appimage-extract currently doesn't support recursive
> extraction:
>    # https://github.com/AppImage/AppImageKit/issues/370
>    local extract_dirs=(usr/share/applications
>                        usr/share/man/man1
>                        usr/share/icons/hicolor/48x48/apps
>                        usr/share/nixnote2
>                        usr/share/nixnote2/help
>                        usr/share/nixnote2/translations)
> 
>    # Due to this issue, all --appimage-extract target directories must
> already exist
>    # https://github.com/AppImage/AppImageKit/issues/363
>    local dir
>    for dir in "${extract_dirs[@]}"; do
>      mkdir -p squashfs-root/"$dir"
>      "./$_appimage" --appimage-extract "$dir/*"
>    done
> 
>    # Save version of extracted files for pkgver() in `makepkg -e`
>    [[ $_appimage_path =~ /releases/download/v([^/]+).* ]]
>    local version=${BASH_REMATCH[1]}
>    echo ${version//-/.} >| "$_version_file"

This is not the version of extracted files... it is the sub-portion of
the download url you calculated outside of any function at all. It
certainly doesn't need to be saved to a file in one function in order to
be used in another function.

>    rm -r squashfs-root/usr/share/"$_pkgname"/{java,images}  # Remove
> non-human resources
> 
>    # Directories are extracted with no permissions for group and other.
> Fix:
>    find squashfs-root -type d -exec chmod go+rx -- {} +
> 
>    # Have the .desktop point explicitly at the icon
>    patch -d squashfs-root/usr/share/applications < "$pkgname:desktop.patch"
> }
> 
> 
> pkgver() {
>      cat "$_version_file"
> }
> 
> 
> package() {
>    cd "$pkgdir"
> 
>    # User resources
>    cp -pr "$srcdir"/squashfs-root/usr .
> 
>    # AppImage as /usr/bin/nixnote2 for .desktop file
>    install -Dm755 -T "$srcdir/$_appimage" usr/bin/"$_pkgname"
> }
> 

After all this effort installing resource files, you just copy the
actual appimage over and have the user run it directly? That means no
resources will actually be used...

This PKGBUILD is nothing but a recipe to download an appimage file using
mkepkg.

-- 
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/20181105/d1b62074/attachment.asc>