Re: [announce] utmps-0.0.1.0

2018-07-07 Thread Laurent Bercot

So, I noticed the package includes example OpenRC service scripts
(wow!) for utmps-utmpd and utmps-wtmpd. Probably not the kind of
testing you hoped for, but for the sake of correctness: they won't
work :-P


 On the contrary, that's exactly the kind of testing I appreciate. :)
I've managed to do some decent testing of the code itself, and
0.0.1.2 seems to be working as intended. But, as you've noticed, the
OpenRC scripts don't get as much coverage. :P

 Note that even on distributions using OpenRC, I tend to start utmps
under a s6 supervision tree: since utmps depends on s6 anyway (for
s6-ipcserver) unless the distro provides an alternative Unix domain
super-server, there's no drawback in taking full advantage of s6's
supervision capabilities.



 The arguments supplied to the program named in the assignment
to 'command' must go in an assignment to variable 'command_args', and
in these particular examples, the end of options marker ("--") before
s6-ipcserver's arguments interacts badly with OpenRC's mechanism for
invoking start-stop-daemon: arguments stored in variable
'start_stop_daemon_args', including -b, end up getting passed to
s6-ipcserver instead, and the script hangs waiting for this program to
exit.


 Blah. start-stop-daemon must die anyway. What's that people said
to me? "The problem with s6 is the unintuitive interface"? Yeah
buddy, at least it understands what a Unix command line is. >.>



Also, as an improvement, the script doesn't need to assume directory
/run/utmps exists: it can check that using OpenRC's 'checkpath'
command, and create it with the correct owner and mode if it does not
exist, just like service utmps-prepare does in the s6-rc examples (is
its definition directory wrongly named 'sutmp-prepare'?).


 Thanks, that's a useful idiom.
 And yes, the package was originally named "sutmp" and I was rightly
told it was a confusing name. So I renamed everything, but forgot that
definition directory. Fixed now.



Patch at the end of the message with those changes, plus some
additional ones for better readability.


 Applied, modulo gmail wrapping long lines and messing up your patch.
 Thanks!

--
 Laurent



Re: [announce] utmps-0.0.1.0

2018-07-07 Thread Guillermo
Hello,

2018-06-07 9:47 GMT-03:00 Laurent Bercot:
>
>  Hello,
>  It's been there for some time but there's now a release number.
>  utmps-0.0.1.0 is out.
> […]
>  It still needs a lot of testing.

So, I noticed the package includes example OpenRC service scripts
(wow!) for utmps-utmpd and utmps-wtmpd. Probably not the kind of
testing you hoped for, but for the sake of correctness: they won't
work :-P The arguments supplied to the program named in the assignment
to 'command' must go in an assignment to variable 'command_args', and
in these particular examples, the end of options marker ("--") before
s6-ipcserver's arguments interacts badly with OpenRC's mechanism for
invoking start-stop-daemon: arguments stored in variable
'start_stop_daemon_args', including -b, end up getting passed to
s6-ipcserver instead, and the script hangs waiting for this program to
exit.

Also, as an improvement, the script doesn't need to assume directory
/run/utmps exists: it can check that using OpenRC's 'checkpath'
command, and create it with the correct owner and mode if it does not
exist, just like service utmps-prepare does in the s6-rc examples (is
its definition directory wrongly named 'sutmp-prepare'?).

Patch at the end of the message with those changes, plus some
additional ones for better readability. Patched scripts tested as
working on Gentoo:

# rc-service utmpd start
 * /run/utmps: creating directory
 * /run/utmps: correcting owner
 * Starting utmpd ... [ ok ]

# rc-service wtmpd start
 * /run/utmps: creating directory
 * Starting wtmpd ... [ ok ]

$ rc-status
...
Dynamic Runlevel: manual
 utmpd[  started  ]
 wtmpd[  started  ]

$ ps -eo pid,ppid,euser,egroup,args
  PID  PPID EUSEREGROUP   COMMAND
…
 3588 1 utmp utmp s6-ipcserverd -- utmps-utmpd
 3620 1 utmp utmp s6-ipcserverd -- utmps-wtmpd

$ ls -ld /run/utmps
drwxr-xr-x 2 utmp utmp 120 Jul  5 23:14 /run/utmps

$ ls -l /run/utmps
total 8
-rw-r--r-- 1 root root 5 Jul  6 23:14 utmpd.pid
srwxrwxrwx 1 utmp utmp 0 Jul  6 23:14 utmpd-socket
-rw-r--r-- 1 root root 5 Jul  6 23:14 wtmpd.pid
srwxrwxrwx 1 utmp utmp 0 Jul  6 23:14 wtmpd-socket

G.

--- original/utmps-0.0.1.2/examples/openrc/utmpd 2017-11-20
11:42:03.0 -0300
+++ patched/utmps-0.0.1.2/examples/openrc/utmpd  2018-07-05
23:26:06.490884765 -0300
@@ -1,8 +1,13 @@
 #!/sbin/openrc-run

-# Assumes the /run/utmps directory already exists and belongs to utmp
-
 name="utmpd"
-command="s6-ipcserver -- /run/utmps/utmpd-socket utmps-utmpd"
+command="s6-ipcserver"
+command_args="/run/utmps/utmpd-socket utmps-utmpd"
+command_background=yes
+command_user=utmp
 pidfile="/run/utmps/utmpd.pid"
-start_stop_daemon_args="-b -m -c utmp -d /run/utmps"
+start_stop_daemon_args="-d /run/utmps"
+
+start_pre() {
+   checkpath -D -d -o utmp:utmp -m 0755 /run/utmps
+}
--- original/utmps-0.0.1.2/examples/openrc/wtmpd 2017-11-20
11:42:03.0 -0300
+++ patched/utmps-0.0.1.2/examples/openrc/wtmpd  2018-07-05
23:26:29.349884765 -0300
@@ -1,8 +1,13 @@
 #!/sbin/openrc-run

-# Assumes the /run/utmps directory already exists and belongs to utmp
-
 name="wtmpd"
-command="s6-ipcserver -- /run/utmps/wtmpd-socket utmps-wtmpd"
+command="s6-ipcserver"
+command_args="/run/utmps/wtmpd-socket utmps-wtmpd"
+command_background=yes
+command_user=utmp
 pidfile="/run/utmps/wtmpd.pid"
-start_stop_daemon_args="-b -m -c utmp -d /run/utmps"
+start_stop_daemon_args="-d /run/utmps"
+
+start_pre() {
+   checkpath -D -d -o utmp:utmp -m 0755 /run/utmps
+}