all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Liliana Marie Prikler <liliana.prikler@ist.tugraz.at>
To: Attila Lendvai <attila@lendvai.name>, 54205@debbugs.gnu.org
Subject: [bug#54205] [PATCH v2] Factor out a public FORK-AND-CALL.
Date: Tue, 01 Mar 2022 13:01:52 +0100	[thread overview]
Message-ID: <ee29e53f40c550969d3a8a046d6f8dda64598a97.camel@ist.tugraz.at> (raw)
In-Reply-To: <20220301072927.26525-1-attila@lendvai.name>

Am Dienstag, dem 01.03.2022 um 08:29 +0100 schrieb Attila Lendvai:
> This enables service implementations to easily inject code that is
> run before their service is started.  One such example is calling
> setrlimit from a start action to set NOFILE (the open files limit),
> before the service is exec'ed and inherits this value from the parent
> process, i.e. from Shepherd.
In general, I think such capabilities should be added to exec-command,
rather than resorting to a lambda.  It takes a little while to realize
that call-in-fork, fork-and-call or whatever you want to name it is in
fact not pure evil; mainly because shepherd could in its stead already
invoke any lambda you throw at it.  That being said, one should always
be aware that this child process runs with the full permissions of
shepherd, which you normally don't want to do for a service.

> [...]
> +(define* (fork-and-call thunk)
> +  "Call THUNK in a fork."
>    ;; Install the SIGCHLD handler if this is the first fork+exec-
> command call.
This docstring, as well as the procedure name only describe what is
done with thunk in the crudest terms.  What's more, I don't think it
makes too much sense to restrict ourselves to thunks if we already run
arbitrary code anyway.

In my opinion, it ought to be 
> +(define* (fork+apply proc . args)
> +  "Spawn a process that calls PROC with ARGS and return its PID."
>    (unless %sigchld-handler-installed?
>      (sigaction SIGCHLD handle-SIGCHLD SA_NOCLDSTOP)
> @@ -916,17 +906,34 @@ its PID."
>              ;; process.
>              (unblock-signals %precious-signals)
>  
> -            (exec-command command
> -                          #:user user
> -                          #:group group
> -                          #:supplementary-groups supplementary-
> groups
> -                          #:log-file log-file
> -                          #:directory directory
> -                          #:file-creation-mask file-creation-mask
> -                          #:create-session? create-session?
> -                          #:environment-variables environment-
> variables))
> +            (apply proc args))
>            pid))))
WDYT?
 
> +(define* (fork+exec-command command
> +                            #:key
> +                            (user #f)
> +                            (group #f)
> +                            (supplementary-groups '())
> +                            (log-file #f)
> +                            (directory (default-service-directory))
> +                            (file-creation-mask #f)
> +                            (create-session? #t)
> +                            (environment-variables
> +                             (default-environment-variables)))
> +  "Spawn a process that executed COMMAND as per 'exec-command', and
> return
> +its PID."
This is just copypasta from a previous mistake, but
s/executed/executes/.

Cheers




  reply	other threads:[~2022-03-01 12:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-01  7:06 [bug#54205] [PATCH Shepherd] Factor out a public CALL-IN-FORK Attila Lendvai
2022-03-01  7:29 ` [bug#54205] [PATCH v2] Factor out a public FORK-AND-CALL Attila Lendvai
2022-03-01 12:01   ` Liliana Marie Prikler [this message]
2022-03-01 13:04     ` Attila Lendvai
2022-03-01 14:01       ` Liliana Marie Prikler
2022-03-01 17:14         ` Christine Lemmer-Webber
2022-03-01 12:47 ` [bug#54205] [PATCH Shepherd] Factor out a public CALL-IN-FORK Maxime Devos
2022-03-02 16:05   ` Ludovic Courtès
2022-03-02 18:21     ` Maxime Devos
2022-03-03  8:04     ` Attila Lendvai
2022-03-21 13:03       ` bug#54205: " Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ee29e53f40c550969d3a8a046d6f8dda64598a97.camel@ist.tugraz.at \
    --to=liliana.prikler@ist.tugraz.at \
    --cc=54205@debbugs.gnu.org \
    --cc=attila@lendvai.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.