bug#54786: Installation tests are failing

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2386 bytes --]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>
> [...]
>
>>> Before going further, I’d like to understand: this does more than just
>>> fix the Jami system tests, right?
>>>
>>> It would have been nice to have surgical changes to “just” fix the
>>> tests, along the lines of <https://issues.guix.gnu.org/54786#9>,
>>> possibly followed by a rework of the whole machinery, if that’s
>>> possible.
>>
>> It's not really possible unfortunately, because the rework from talking
>> to the D-Bus API via the 'dbus-send' binary to using Guile AC/D-bus was
>> needed or at least simplified fixing the issues.
>
> So am I right that the “issues” were not specifically related to the
> Shepherd 0.9.0 switch, or at least not just to that?  (Just to make sure
> I understand the context.)

I tried capturing the issue in the commit message, but I'll provide
another more hands-on view: the Jami service was broken due to changes
in Shepherd 0.9.0 that caused the blocking sleeps + concurrent
make+forkexec-constructor/container and fork+exec-command combination
used to not work anymore.

This problem can be manually observed by spawning a VM with the Jami
service:

$(guix system vm --no-graphic -e '(@@ (gnu tests telephony) %jami-os)') -m 512

Then you'll see the service doesn't even start:

--8<---------------cut here---------------start------------->8---
root@jami ~# herd status
[...]
Stopped:
 - jami
[...]

root@jami ~# pgrep jamid
192

root@jami ~# killall jamid

root@jami ~# herd start jami
Jami Daemon 11.0.0, by Savoir-faire Linux 2004-2019
https://jami.net/
[Video support enabled]
[Plugins support enabled]

12:53:47.144         os_core_unix.c !pjlib 2.11 for POSIX initialized

herd: exception caught while executing 'start' on service 'jami':
Throw to key `match-error' with args `("match" "no matching pattern" #f)'.
--8<---------------cut here---------------end--------------->8---

I've ran this: herd start jami& strace -p1 -f -s800 -o strace.out

Attached is the last 10% of the gzip'd file.  I couldn't explain this
failure very clearly, but when I tried investigating it was failing on
the '(dbus-service-available? "cx.ring.Ring")' call, if I recall
correctly.


[-- Attachment #2: shepherd pid1 strace --]
[-- Type: application/octet-stream, Size: 31449 bytes --]

[-- Attachment #3: Type: text/plain, Size: 3874 bytes --]


[...]

>>> What do you think is missing upstream so that starting Jami is
>>> simpler?
>>
>> 1) Lack of D-Bus support in Shepherd to easily start D-Bus services.
>> The upstream systemd service definition for the Jami daemon (jamid) is
>> this:
>>
>> # net.jami.daemon.service
>> [D-BUS Service]
>> Name=cx.ring.Ring
>> Exec=@LIBDIR@/jamid
>>
>> But that's nearly not where the complexity of our jami-service-type
>> lies.
>
> But that should be fine: we have dozens of D-Bus services that happily
> get started by dbus-daemon.

I guess that works (minus races like we've had with elogind) if the
other services are also D-Bus services sharing the same bus.  But here
the code talking with Jami are in the Shepherd service actions and more
critically in the start slot itself -- so it's important the D-Bus
service has been acquired and ready to service D-Bus calls otherwise
they'd fail (that's what the loop polling for (jami-service-available?)
ensures).

>> Rather, it's in the following:
>>
>> 2) The lack of a way to declaratively configure Jami and the need to use
>> D-Bus API to issue commands to Jami non-interactively.  For example, to
>> have Jami import an account it's necessary to go via either
>>
>> a) the GUI or
>> b) the D-Bus API
>>
>> The Jami service in Guix makes use of b), which introduces the need for
>> some Scheme bindings wrapping the low-level D-Bus interface.  Perhaps
>> such bindings could live in Jami itself.
>>
>> The second point (2) could be addressed upstream, but since it's a
>> rather niche use case (the common use case is simply running the client
>> GUI), is already achievable via D-Bus, and would probably require a
>> considerable amount of work in Jami itself, I think we can keep it as is
>> for now, as a Guix System exclusive feature ;-).  Note that even if Jami
>> could be configured via configuration files, we'd still want to be able
>> to communicate with it via D-Bus to maintain the possible actions
>> currently available in our Shepherd service (listing/enabling/disable
>> accounts, etc.), so it'd only really help to reduce the start slot, and
>> that's it.  We'd still need most of the D-Bus bindings, so it wouldn't
>> help that much anyway.
>
> Ah I see.
>
>> I hope that clarifies how our jami-service-type is both complex but also
>> unique.
>
> Sure, the ability to configure Jami in a declarative and stateless
> fashion is a plus, that’s really cool.
>
> Longer-term I think this should go in Jami proper though.  It’s great
> that Guix has an edge over the competition :-), but having to maintain
> it is less nice.

Perhaps with the Scheme bindings introduced by Olivier for the Jami
tests (that work via an embedded libguile), it could be possible to add
the ability to pass an init script to 'jamid' at launch time, which
would automate importing the account.  Proper 'Scheme' bindings would be
nice though, and I'd like to look into the feasibility to add these via
Swig.  Food for thought.

> Also, in more concrete terms: one goal of the least-authority work at
> <https://issues.guix.gnu.org/54997> is to remove
> ‘make-forkexec-constructor/container’ and the whole (gnu build shepherd)
> module.  Jami is one of its last remaining users (adjusting it felt like
> beyond my abilities, precisely because it’s much more complex than the
> other services I adjusted).
>
> Could you take a look at that eventually, once this patch has been
> reviewed?

I reviewed how that works, and it'd be easy; I just didn't see the
incentive yet (there's no composition needed for the service, and it'd
make the definition slightly less readable).  If you tell me
mark+forkexec-constructor/container is going the way of the Dodo though,
that's a good enough incentive :-).

Thanks for having a look!

Maxim