Date: Sat, 30 May 2020 08:11:21 +0300
From: Konstantin Kulikov <k.kulikov2@gmail.com>
To: Miles Alan <m@milesalan.com>
Cc: ~alpine/aports@lists.alpinelinux.org
Subject: Re: [PATCH] testing/lf: new aport
Message-Id: <20200530081121.83fb92a61b46617ca6284934@gmail.com>
In-Reply-To: <20200530021355.20187-1-m@milesalan.com>
References: <20200530021355.20187-1-m@milesalan.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

Many devs ignore aports ML, consider sending merge request at https://gitlab.alpinelinux.org/alpine/aports

Review comments inline below.

> https://github.com/gokcehan/lf
> Terminal filemanager written in Go with vim-style keybindings
> ---
>  testing/lf/APKBUILD | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 testing/lf/APKBUILD
> 
> diff --git a/testing/lf/APKBUILD b/testing/lf/APKBUILD
> new file mode 100644
> index 00000000..f79c70ae
> --- /dev/null
> +++ b/testing/lf/APKBUILD
> @@ -0,0 +1,36 @@
> +# Maintainer: Miles Alan <m@milesalan.com>
> +pkgname=lf
> +pkgver=14
> +pkgrel=0
> +pkgdesc="Terminal filemanager written in Go with vim-style keybindings"
> +url="https://github.com/gokcehan/lf"
> +arch="all"
> +license="MIT"
> +makedepends="go"
> +subpackages="$pkgname-doc"
> +source="$pkgname-$pkgver.tar.gz::https://github.com/gokcehan/lf/archive/r$pkgver.tar.gz"
> +builddir="$srcdir/src/github.com/gokcehan/$pkgname"

builddir is unneccesary with go modules. 

> +options="chmod-clean"

Add "net" to options as well. This may become a requirement in future.

Also export a few variables so go doesn't put stuff in /home or /tmp like this:

export GOPATH="$srcdir/go"
export GOCACHE="$srcdir/go-build"
export GOTMPDIR="$srcdir"

> +
> +prepare() {
> +	mkdir -p ${builddir%/*}
> +	mv $srcdir/$pkgname-r$pkgver "$builddir"/
> +	default_prepare
> +	go mod vendor
> +}

If you vendor stuff you need to also pass -mod=vendor to "go build" and "go test",
because alpine is still on go1.13.
remove prepare() function and vendoring completely instead, it is unneccesary here.

> +
> +build() {
> +	go build -v -o bin/$pkgname
> +}
> +
> +check() {
> +	go test

Prefer "go test ./..." just in case upstream adds more packages to repo in future.

> +}
> +
> +package() {
> +	install -Dm755 "$builddir"/bin/$pkgname "$pkgdir"/usr/bin/$pkgname
> +	install -Dm644 LICENSE "$pkgdir"/usr/share/licenses/$pkgname/LICENSE

Do not install license file, unless package uses "license=custom".

> +	install -Dm644 lf.1 "$pkgdir"/usr/share/man/man1/lf.1
> +}
> +
> +sha512sums="cd2e268d551a0a83a32563424675455288e09b28d60940143eff4d24cf5b98ad0d7bd9ab100ea5a456e37460c318001ad5b444d252a986e9ddcfff274adf7720  lf-14.tar.gz"
> -- 
> 2.26.2