Re: [shepherd] several patches that i deem ready

[Ludovic Courtès]

Hi Attila,

Attila Lendvai <attila@lendvai.name> skribis:

> i have prepared the rest of my commits that were needed to hunt down the 
> shepherd hanging bug. you can find them at:
>
> https://codeberg.org/attila-lendvai-patches/shepherd/commits/branch/attila
>
> there's some dependency among the commits, so sending them to debbugs would 
> be either as one big series of commits, or a hopeless labirinth of patches 
> otherwise.

Yes, but OTOH, piecemeal, focused changes sent to Debbugs are easier to
review for me.  (There are 34 commits in this branch touching different
aspects.)

> therefore i recommend the following workflow instead (assuming that Ludo is 
> pretty much the only one hacking on shepherd):
>
> Ludo, please take a look at my branch, and cherry-pick whatever you are happy 
> with. then based on your feedback, and the new main branch, i'll rebase and 
> refine my commits and give you a head's up when it's ready for another 
> merge/review.
>
> the commits are more or less ordered in least controversial order, modulo 
> dependencies.
>
> the main additions are:
>
> - a multi-layered error handler that got employed at various points in
>   the codebase. this makes shepherd much more resilient, even in case
>   of nested errors, and much more communicative in the log when errors
>   end up happening.
>
> - a lightweight logging infrastructure together with plenty of log
>   lines throughout the codebase, and some hints in the README on how
>   to turn log lines gray in emacs (i.e. easily ignorable).

I cherry-picked a couple of patches.

Some notes:

+ 94c1143 shepherd: Add tests/startup-error.sh

  Redundant with ‘tests/startup-failure.sh’ I think?

+ e802761 service: Add custom printer for <service> records.

  Good idea, but the goal is to remove GOOPS, so put aside for now.

+ af2ebec service: respawn-limit: make #f mean no limit.

  I’d rather not do that: one can use +inf.0 when needed.

+ 095e930 shepherd: Do not respawn disabled services.

  That’s already the case (see commit
  7c88d67076a0bb1d9014b3bc23ed9c68f1c702ab; maybe we hacked it
  independently in parallel).

+ dbc9150 shepherd: Increase the time range for the default respawn limit.

  This arbitrary and thus debatable, but I think the current setting
  works well, doesn’t it?

+ e03b958 support: Add logging operators.
+ 39c2e14 shepherd: add call-with-error-handling

  I like the idea: we really need those backtraces to be logged!
  There are mostly-stylistic issues that would need to be discussed
  though.  I’d like logging to be less baroque; I’m not convinced by:

    + 7183c9c shepherd: Populate the code with some log lines.

  This is exactly what I’d like to avoid—adding logging statements all
  around the code base, possibly redundant with existing logging
  statements that target users.

  What I do want though is to have “first-class logs”, pretty much like
  what we see with ‘herd log’ etc.  To me, that is much more useful than
  writing the arguments passed each and every ‘fork+exec-command’ call.

I’ll have to look further that branch.  I admit I have limited bandwidth
available and, perhaps selfishly, I like to use my free-time computing
to hack myself.

Regardless, I’d like to thank you for your continued efforts on the
Shepherd.  In one way or another, it contributes to shaping it.

Ludo’.