5fd553eceec8325f829089d13e5828e002393438 was supposed to allow
multiple instances, but given that it defines `procname`, SIGTERM
gets delivered to all running instances. Instead, don't daemonize
darkhttpd and let start-stop-daemon(8) handle process forking and
pidfile creation. This was chosen over using `--pidfile` flag of
darkhttpd because pidfile is created inside the chroot in such
case.

Signed-off-by: Milliardo Peacecraft <milliardo@firemail.cc>
---
 main/darkhttpd/darkhttpd.initd | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/main/darkhttpd/darkhttpd.initd b/main/darkhttpd/darkhttpd.initd
index b16244b25c..38d33f3a69 100644
--- a/main/darkhttpd/darkhttpd.initd
+++ b/main/darkhttpd/darkhttpd.initd
@@ -2,10 +2,9 @@
 
 description="darkhttpd web server"
 command="/usr/bin/darkhttpd"
-command_args="${document_root:-/var/www/localhost/htdocs} --chroot --daemon --uid darkhttpd --gid www-data $darkhttpd_args"
-procname="darkhttpd"
-pidfile=""
-stopsig="SIGTERM"
+command_args="${document_root:-/var/www/localhost/htdocs} --chroot --uid darkhttpd --gid www-data $darkhttpd_args"
+command_background="true"
+pidfile="/run/$SVCNAME.pid"
 start_stop_daemon_args="-q"
 
 optional_arg() {
-- 
2.19.1




---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---

Hi,

thank you for your patch. Please look at some comments below.

On Thu, Oct 25, 2018 at 5:15 PM Milliardo Peacecraft <milliardo@firemail.cc>
wrote:

> 5fd553eceec8325f829089d13e5828e002393438 was supposed to allow
> multiple instances, but given that it defines `procname`, SIGTERM
> gets delivered to all running instances. Instead, don't daemonize
> darkhttpd and let start-stop-daemon(8) handle process forking and
> pidfile creation. This was chosen over using `--pidfile` flag of
> darkhttpd because pidfile is created inside the chroot in such
> case.
>
> Signed-off-by: Milliardo Peacecraft <milliardo@firemail.cc>
> ---
>  main/darkhttpd/darkhttpd.initd | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/main/darkhttpd/darkhttpd.initd
> b/main/darkhttpd/darkhttpd.initd
> index b16244b25c..38d33f3a69 100644
> --- a/main/darkhttpd/darkhttpd.initd
> +++ b/main/darkhttpd/darkhttpd.initd
> @@ -2,10 +2,9 @@
>
>  description="darkhttpd web server"
>  command="/usr/bin/darkhttpd"
> -command_args="${document_root:-/var/www/localhost/htdocs} --chroot
> --daemon --uid darkhttpd --gid www-data $darkhttpd_args"
> -procname="darkhttpd"
> -pidfile=""
> -stopsig="SIGTERM"
> +command_args="${document_root:-/var/www/localhost/htdocs} --chroot --uid
> darkhttpd --gid www-data $darkhttpd_args"
> +command_background="true"
> +pidfile="/run/$SVCNAME.pid"
>

Looking at sources and man page I think $SVCNAME is deprecated, $RC_SVCNAME
should be used instead.
Also, since it does not run as root it would be preferrable to place the
pidfile under "/run/darkhttpd" and use "checkpath" from init in order to
create $rundir with the right uid/gid. Since the pidfile is created by
openrc, the $pidfile creation is successful anyway in this case.

Also please remember to bump $pkgrel in APKBUILD for any change.

Thanks!

/eo

> Looking at sources and man page I think $SVCNAME is deprecated, $RC_SVCNAME
> should be used instead.

I used $SVCNAME because it was what was in use, but I'll change it to
$RC_SVCNAME.

> Also, since it does not run as root it would be preferrable to place the
> pidfile under "/run/darkhttpd" and use "checkpath" from init in order to
> create $rundir with the right uid/gid. Since the pidfile is created by
> openrc, the $pidfile creation is successful anyway in this case.

Should this go in the same patch? It doesn't affects the problem I'm
trying to solve. I can make the other patch if wanted.

> Also please remember to bump $pkgrel in APKBUILD for any change.

Going to. First time contributing so I missed that.

> Thanks!

Thank you for taking the time to review it! I'll send an updated
patch once I hear back from you regarding to make 2 patches or one.

Milliardo


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---