[bug#47849] [PATCH] Add a jami-daemon service.

[Leo Prikler <leo.prikler@student.tugraz.at>]

Hi Maxim,

Am Montag, den 02.08.2021, 15:17 -0400 schrieb Maxim Cournoyer:
> > > +                    ,@(if (or bus (%send-dbus-bus))
> > > +                          (list (string-append "--bus="
> > > +                                               (or bus (%send-
> > > dbus-
> > > bus))))
> > > +                          '())
> > You use this style here, but
> > > +        (let* ((uid (or (and=> (or user (%send-dbus-user))
> > > +                               (compose passwd:uid getpwnam))
> > > -1))
> > > +               (gid (or (and=> (or group (%send-dbus-group))
> > > +                               (compose group:gid getgrnam))
> > > -1)))
> > this style here.  I personally think using and=> everywhere would
> > make
> > things a little clearer.
> 
> I tried, but it seemed it had a bigger cognitive load to follow (or,
> and=>, compose, list, cut all in the same expression), so ended up
> keeping it as is.
I think the cognitive burden would in both cases be lessened if we
didn't need to write (or stuff (%send-dbus-stuff)) everywhere, which
see below.

> > More importantly, though
> > > +(define* (send-dbus #:key service path interface method
> > > +                    bus
> > > +                    dbus-send
> > > +                    user group
> > > +                    timeout
> > > +                    arguments)
> > If you e.g. write (bus (%send-dbus-bus)), I think the argument
> > should
> > already be correctly passed upon the function call, no?
> 
> the %send-dbus-bus is a parameter; if we were to provide it as a
> default
> value, it would be evaluated at the time the module is read (IIUC),
> but
> I want its value to be read at run time, when the procedure is called
> instead.
That's how define* works though, in my experience:

--8<---------------cut here---------------start------------->8---
;; foo.scm
(define-module (foo)
  #:export (*rand* rand))

(define *rand* (make-parameter 4))
(define* (rand #:optional (value (*rand*))) value)
--8<---------------cut here---------------end--------------->8---

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use (foo) ;; alternatively (load-compiled
"foo.go")
scheme@(guile-user)> (rand)
$1 = 4
scheme@(guile-user)> (parameterize ((*rand* 5)) (rand))
$2 = 5
scheme@(guile-user)> (rand 6)
$3 = 6
--8<---------------cut here---------------end--------------->8---

> > > +          (chown temp-port uid gid)))
> > > +      (lambda ()
> > > +        (let ((pid (fork+exec-command command
> > > +                                      #:user (or user (%send-
> > > dbus-
> > > user))
> > > +                                      #:group (or group (%send-
> > > dbus-
> > > group))
> > > +                                      #:log-file temp-file)))
> > > +          (match (waitpid pid)
> > > +            ((_ . status)
> > > +             (let ((exit-status (status:exit-val status))
> > > +                   (output (call-with-port temp-port get-string-
> > > all)))
> > > +               (if (= 0 exit-status)
> > > +                   output
> > > +                   (error "the send-dbus command exited with: "
> > > +                          command exit-status output)))))))
> > Since output is unused in the error case, I think you can move the
> > get-
> > string-all there.  It would make a difference if you were to e.g.
> > close
> > temp-port before that.
> 
> It's actually used as the last argument of the 'error' call :-).  I
> added it as it was helpful to debug stderr messages from dbus-send.
My bad, nvm then.

> > I did not look at the rest of this patch, but 1+2 LGTM.
> > 
> > Regards,
> 
> Thanks a lot for having a look!  It's been long in the making.
> 
> Pushed as commit 69dcc24c9f.
As I said, I only looked at parts of it, so this push was perhaps a bit
hasty.  I recently also hastily pushed something and had to fix it up
later.  Let's try to keep a cool head and not let the heat get to us :)

Regards,