Re: wrong exit code for sv status in lsb mode

2019-01-08 Thread David Mountney via supervision
The error was introduced in commit 5fe1bc773c2d979093fe4b1f3ecbbed5e6acdaf0
"sv.c: properly format status command's output on failure cases."

Where the log status is being saved to the same variable as the service
status. The quick fix would be to not record the log status for now:

$ git diff src/sv.c
diff --git a/src/sv.c b/src/sv.c
index 9003142..1676227 100644
--- a/src/sv.c
+++ b/src/sv.c
@@ -167,7 +167,7 @@ int status(char *unused) {
   }
   else {
 outs("; ");
-if (svstatus_get()) { rc =svstatus_print("log"); outs("\n"); }
+if (svstatus_get()) { svstatus_print("log"); outs("\n"); }
   }
   islog =0;
   flush("");
$

On Tue, Jan 8, 2019 at 12:18 AM Gerrit Pape  wrote:

> Hi,
>
> donations for my free software increased recently, not sure why, but
> thanks.
>
> runit upstream sources with history still are available through
>
>  $ git clone http://smarden.org/git/runit.git
>
> Below is the diff for sv.c from 2.1.1 to 2.1.2.  Possibly anyone can
> spot the error and create a patch if that issue is annoying.
>
> Regards, Gerrit.
>
>
> $ git diff v2.1.1..v2.1.2 src/sv.c
> diff --git a/src/sv.c b/src/sv.c
> index d126cee..9003142 100644
> --- a/src/sv.c
> +++ b/src/sv.c
> @@ -41,6 +41,7 @@ unsigned int lsb;
>  unsigned int verbose =0;
>  unsigned long wait =7;
>  unsigned int kll =0;
> +unsigned int islog =0;
>  struct taia tstart, tnow, tdiff;
>  struct tai tstatus;
>
> @@ -67,6 +68,7 @@ void fatal2(char *m1, char *m2) {
>  void out(char *p, char *m1) {
>buffer_puts(buffer_1, p);
>buffer_puts(buffer_1, *service);
> +  if (islog) buffer_puts(buffer_1, "/log");
>buffer_puts(buffer_1, ": ");
>buffer_puts(buffer_1, m1);
>if (errno) {
> @@ -153,20 +155,22 @@ int status(char *unused) {
>int rc;
>
>rc =svstatus_get();
> -  switch(r) { case -1: if (lsb) done(4); case 0: return(0); }
> +  switch(rc) { case -1: if (lsb) done(4); case 0: return(0); }
>rc =svstatus_print(*service);
> +  islog =1;
>if (chdir("log") == -1) {
>  if (errno != error_noent) {
> -  outs("; log: "); outs(WARN);
> -  outs("unable to change to log service directory: ");
> -  outs(error_str(errno));
> +  outs("; ");
> +  warn("unable to change directory");
>  }
> +else outs("\n");
>}
> -  else
> -if (svstatus_get()) {
> -  outs("; "); svstatus_print("log");
> -}
> -  flush("\n");
> +  else {
> +outs("; ");
> +if (svstatus_get()) { rc =svstatus_print("log"); outs("\n"); }
> +  }
> +  islog =0;
> +  flush("");
>if (lsb) switch(rc) { case 1: done(0); case 2: done(3); case 0:
> done(4); }
>return(rc);
>  }
> @@ -305,9 +309,11 @@ int main(int argc, char **argv) {
>  acts ="d"; kll =1; cbk = break;
>case 'T':
>  acts ="tc"; kll =1; cbk = break;
> +  case 't':
> +if (!str_diff(action, "try-restart")) { acts ="tc"; cbk =
> break; }
>case 'c':
>  if (!str_diff(action, "check")) { act =0; acts ="C"; cbk =
> break; }
> -  case 'u': case 'd': case 'o': case 't': case 'p': case 'h':
> +  case 'u': case 'd': case 'o': case 'p': case 'h':
>case 'a': case 'i': case 'k': case 'q': case '1': case '2':
>  action[1] =0; acts =action; break;
>case 's':
> @@ -318,6 +324,7 @@ int main(int argc, char **argv) {
>  act = cbk =0; break;
>case 'r':
>  if (!str_diff(action, "restart")) { acts ="tcu"; cbk = break; }
> +if (!str_diff(action, "reload")) { acts ="h"; cbk = break; }
>  usage();
>case 'f':
>  if (!str_diff(action, "force-reload"))
> $
>
>
> On Fri, Jan 04, 2019 at 05:10:13PM -0800, David Mountney via supervision
> wrote:
> > This looks like it's still a problem introduced in runit 2.1.2
> >
> > If you have defined a log handler, only the status of the log handler is
> > returned by sv status.
> >
> > Its not clear to me from the changelog why this change was made.
> >
> > I guess, ideally if the service status was successful, you could then
> also
> > check the log status, but as it is now, if the service is down, but the
> log
> > is up, sv status reports all is well for its exit codes.
> >
> > I've seen other references to this bug, where projects have reverted to
> the
> > previous runit version:
> https://github.com/chef/omnibus-software/pull/793
> >
> > It would be ideal if we could just get this fixed.
> >
> >
> > > From: Fabian Ruff
> > > Date: Tue, 18 Aug 2015 23:33:50 +0200
> > >  Tue, 18 Aug 2015 23:33:50 +0200
> > > Hi,
> > > I just noticed that the exit code of sv status in "lsb" mode is not 3
> in
> > > all cases when
> > > the service is "down".
> > > The problem arises when a service defines a log handler. In that case
> the
> > > return code
> > > of svstatus print for the log process overwrites the return code of
> > > svstatus print of
> > > the actual supervised process (line 170 of sv.c).
> > > Is this intentional? Looks like a bug to me.
> > > Kind regards,
>


Re: Can s6 be enough?: was s6-ps

2019-01-08 Thread Laurent Bercot

When Debian acquires a properly working s6-rc package, the answer to my
question degenerates to "why not?" But for now, for the Debian person
who only installs via package, s6-rc is out of the question, so my
question was, isn't s6 itself good enough?


 Then maybe you should ask Debian instead, "why aren't you packaging
s6-rc?"

 It's literally a distribution's *job* to package software that may
be useful to users. And instead, you are bending your mind to try
and find justifications for users not to use some software, for the
simple reason that it's not packaged by a distribution. That logic
is 100% backwards.

 You want to help users? Package s6-rc for Debian. Also, call out
Debian on their prejudice and disingenuous use of policies that
prevent people from making correct execline and s6 packages.

--
 Laurent



Re: wrong exit code for sv status in lsb mode

2019-01-08 Thread Gerrit Pape
Hi,

donations for my free software increased recently, not sure why, but
thanks.

runit upstream sources with history still are available through

 $ git clone http://smarden.org/git/runit.git

Below is the diff for sv.c from 2.1.1 to 2.1.2.  Possibly anyone can
spot the error and create a patch if that issue is annoying.

Regards, Gerrit.


$ git diff v2.1.1..v2.1.2 src/sv.c
diff --git a/src/sv.c b/src/sv.c
index d126cee..9003142 100644
--- a/src/sv.c
+++ b/src/sv.c
@@ -41,6 +41,7 @@ unsigned int lsb;
 unsigned int verbose =0;
 unsigned long wait =7;
 unsigned int kll =0;
+unsigned int islog =0;
 struct taia tstart, tnow, tdiff;
 struct tai tstatus;
 
@@ -67,6 +68,7 @@ void fatal2(char *m1, char *m2) {
 void out(char *p, char *m1) {
   buffer_puts(buffer_1, p);
   buffer_puts(buffer_1, *service);
+  if (islog) buffer_puts(buffer_1, "/log");
   buffer_puts(buffer_1, ": ");
   buffer_puts(buffer_1, m1);
   if (errno) {
@@ -153,20 +155,22 @@ int status(char *unused) {
   int rc;
 
   rc =svstatus_get();
-  switch(r) { case -1: if (lsb) done(4); case 0: return(0); }
+  switch(rc) { case -1: if (lsb) done(4); case 0: return(0); }
   rc =svstatus_print(*service);
+  islog =1;
   if (chdir("log") == -1) {
 if (errno != error_noent) {
-  outs("; log: "); outs(WARN);
-  outs("unable to change to log service directory: ");
-  outs(error_str(errno));
+  outs("; ");
+  warn("unable to change directory");
 }
+else outs("\n");
   }
-  else
-if (svstatus_get()) {
-  outs("; "); svstatus_print("log");
-}
-  flush("\n");
+  else {
+outs("; ");
+if (svstatus_get()) { rc =svstatus_print("log"); outs("\n"); }
+  }
+  islog =0;
+  flush("");
   if (lsb) switch(rc) { case 1: done(0); case 2: done(3); case 0: done(4); }
   return(rc);
 }
@@ -305,9 +309,11 @@ int main(int argc, char **argv) {
 acts ="d"; kll =1; cbk = break;
   case 'T':
 acts ="tc"; kll =1; cbk = break;
+  case 't':
+if (!str_diff(action, "try-restart")) { acts ="tc"; cbk = break; }
   case 'c':
 if (!str_diff(action, "check")) { act =0; acts ="C"; cbk = break; }
-  case 'u': case 'd': case 'o': case 't': case 'p': case 'h':
+  case 'u': case 'd': case 'o': case 'p': case 'h':
   case 'a': case 'i': case 'k': case 'q': case '1': case '2':
 action[1] =0; acts =action; break;
   case 's':
@@ -318,6 +324,7 @@ int main(int argc, char **argv) {
 act = cbk =0; break;
   case 'r':
 if (!str_diff(action, "restart")) { acts ="tcu"; cbk = break; }
+if (!str_diff(action, "reload")) { acts ="h"; cbk = break; }
 usage();
   case 'f':
 if (!str_diff(action, "force-reload"))
$ 


On Fri, Jan 04, 2019 at 05:10:13PM -0800, David Mountney via supervision wrote:
> This looks like it's still a problem introduced in runit 2.1.2
> 
> If you have defined a log handler, only the status of the log handler is
> returned by sv status.
> 
> Its not clear to me from the changelog why this change was made.
> 
> I guess, ideally if the service status was successful, you could then also
> check the log status, but as it is now, if the service is down, but the log
> is up, sv status reports all is well for its exit codes.
> 
> I've seen other references to this bug, where projects have reverted to the
> previous runit version: https://github.com/chef/omnibus-software/pull/793
> 
> It would be ideal if we could just get this fixed.
> 
> 
> > From: Fabian Ruff
> > Date: Tue, 18 Aug 2015 23:33:50 +0200
> >  Tue, 18 Aug 2015 23:33:50 +0200
> > Hi,
> > I just noticed that the exit code of sv status in "lsb" mode is not 3 in
> > all cases when
> > the service is "down".
> > The problem arises when a service defines a log handler. In that case the
> > return code
> > of svstatus print for the log process overwrites the return code of
> > svstatus print of
> > the actual supervised process (line 170 of sv.c).
> > Is this intentional? Looks like a bug to me.
> > Kind regards,