Date: Fri, 25 Nov 2016 13:00:44 -0200
From: Natanael Copa <ncopa@alpinelinux.org>
To: Valery Kartel <valery.kartel@gmail.com>
Cc: alpine-aports@lists.alpinelinux.org
Subject: Re: [alpine-aports] [PATCH v3] testing/rspamd: upgrade to 1.4.0 +
 some fixups
Message-ID: <20161125130044.009c9518@ncopa-macbook.copa.dup.pw>
In-Reply-To: <20161125132727.10281-1-valery.kartel@gmail.com>
References: <20161125132727.10281-1-valery.kartel@gmail.com>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

Thank you for working on this. Please see comments below.

On Fri, 25 Nov 2016 15:27:27 +0200
Valery Kartel <valery.kartel@gmail.com> wrote:

> subpackage rename: www -> controller
> subpackage add: fuzzy, utils
> fix /var/lib/rspamd permissions
> fix run in lxc container (create and mount /dev/shm)
> 
> ---
>  testing/rspamd/APKBUILD                 | 95 ++++++++++++++++++++++-----------
>  testing/rspamd/rspamd.conf              | 21 ++++++--
>  testing/rspamd/rspamd.initd             | 13 +++--
>  testing/rspamd/rspamd.worker_controller |  9 +---
>  testing/rspamd/rspamd.worker_fuzzy      |  6 +++
>  testing/rspamd/rspamd.worker_normal     |  6 +--
>  6 files changed, 100 insertions(+), 50 deletions(-)
>  create mode 100644 testing/rspamd/rspamd.worker_fuzzy

...

> @@ -12,15 +12,18 @@ pkgusers="rspamd"
>  pkggroups="rspamd"
>  depends=""
>  depends_dev=""
> -makedepends="$depends_dev cmake libressl-dev libevent-dev glib-dev gmime-dev
> -	lua5.1-dev lua5.1 sqlite-dev hiredis-dev file-dev pcre-dev ragel"
> +makedepends="$depends_dev cmake ragel perl
> +	luajit-dev glib-dev pcre-dev gmime-dev libevent-dev sqlite-dev
> +	libressl-dev file-dev curl-dev"
>  install="$pkgname.pre-install"

Why do we use luajit instead of "normal" lua? Does this mean that we
need to disable PaX protections?

> -subpackages="$pkgname-doc $pkgname-web $pkgname-client"
> +subpackages="$pkgname-doc $pkgname-controller::noarch $pkgname-client
> +	$pkgname-fuzzy::noarch $pkgname-utils::noarch"
>  source="https://rspamd.com/downloads/$pkgname-$pkgver.tar.xz
>  	$pkgname.logrotated
>  	$pkgname.initd
>  	$pkgname.confd
>  	$pkgname.conf
> +	$pkgname.worker_fuzzy
>  	$pkgname.worker_normal
>  	$pkgname.worker_controller
>  "

...

> @@ -84,24 +96,43 @@ client() {
>  	mv "$pkgdir"/usr/bin/rspamc-$pkgver "$subpkgdir"/usr/bin/rspamc
>  }
>  
> -md5sums="ab81d063861d68b1e2260d1c599bf29c  rspamd-1.3.4.tar.xz
> +fuzzy() {
> +	depends="$pkgname"
> +	pkgdesc="$pkgdesc (local fuzzy storage)"
> +	mkdir -p "$subpkgdir"/etc/$pkgname/modules.d
> +	mv "$pkgdir"/etc/$pkgname/worker-fuzzy.* "$subpkgdir"/etc/$pkgname
> +	mv "$pkgdir"/etc/$pkgname/modules.d/fuzzy_* "$subpkgdir"/etc/$pkgname/modules.d
> +}

Is the fuzzy subpackage just a configuration file that we provide? is
the fuzzy config file available from upstream?


> +
> +utils() {
> +	depends="perl"
> +	pkgdesc="$pkgdesc (utils)"
> +	mkdir -p "$subpkgdir"/usr/bin
> +	mv "$pkgdir"/usr/bin/${pkgname}_stats "$subpkgdir"/usr/bin/${pkgname}-stats
> +	mv "$pkgdir"/usr/bin/${pkgname}-redirector "$subpkgdir"/usr/bin
> +}
> +

+1 We want utils that depends on perl to be in subpackage.

...

> diff --git a/testing/rspamd/rspamd.conf b/testing/rspamd/rspamd.conf
> index fe737f9..1e37cb7 100644
> --- a/testing/rspamd/rspamd.conf
> +++ b/testing/rspamd/rspamd.conf
> @@ -1,9 +1,24 @@
> +# Please don't modify this file as your changes might be overwritten with
> +# the next update.

I believe this comment is false, in fact if you you don't modify the
config file it will be replaced with newer version. If you modify it,
the new updated config file will be installed as .apk-new

...

> diff --git a/testing/rspamd/rspamd.initd b/testing/rspamd/rspamd.initd
> index a20f784..b77a431 100644
> --- a/testing/rspamd/rspamd.initd
> +++ b/testing/rspamd/rspamd.initd
> @@ -14,24 +14,29 @@ description_reopen="Reopen log files"
>  
>  depend() {
>  	need localmount net
> -	before exim postfix
> +	before mta
>  }
>  
>  start_pre() {
>  	ebegin
> +	# shm fix for lxc-containers
> +	if ! grep -q ^shm /proc/mounts; then
> +		checkpath --directory /dev/shm
> +		mount -t tmpfs -o noexec,nosuid,nodev,mode=1777 shm /dev/shm
> +	fi

mounting /dev/shm from rspamd init script does not make sense. If you
need /dev/shm mounted, then you should not need install and run rspamd.
This needs to be fixed other place, like lxc config.

IIRC also python build needs /dev/shm

>  	checkpath --directory --mode 750 --owner $user:$group ${pidfile%/*}
>  	$command $command_args -t >/dev/null 2>>${startuplog:-/dev/null}
>  	eend $?
>  }
>  
>  reload() {
> -	ebegin "Reloading ${SVCNAME}"
> +	ebegin Reloading $SVCNAME

why?

>  	start-stop-daemon --signal HUP --pidfile $pidfile
>  	eend $?
>  }
>  
>  reopen() {
> -	ebegin "Reopening ${SVCNAME} log files"
> +	ebegin Reopening $SVCNAME log files

why?

>  	start-stop-daemon --signal USR1 --pidfile $pidfile
>  	eend $?
> -}
> \ No newline at end of file
> +}

...


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---