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