Do not throw a warning if plymouth is not installed

[danimo]

Ideally, plymouth should only be referenced via dependencies,
not ExecStartPre's. This at least avoids the confusing error message
on minimal installations that do not carry plymouth.

[keszybz]

I agree it'd be nice to avoid the warning. This is ugly as hell ;), but since we already use /bin/sh in this unit file, there's no reason not to do this.

[keszybz]

One nitpick: on Debian/Ubuntu plymouth is installed as /bin/plymouth. For merged-usr distros this doesn't matter, but let's be nice to our friends in the other camp. So please change this to call /bin/plymouth like previously.

[danimo]

Changes implemented & force pushed for clean history.

[danimo]

Could someone have a look again and approve this patch?

[martinpitt]

Agreed about the ugliness, but shrug. Thanks for updating!

[mbiebl]

For consistencies sake, can we please apply the same to rescue.service which has the same ExecStartPre line as emergency.service

[danimo]

@mbiebl Good point, see above commit.

While grep'ing for more instances, I came across mentions of "plymouth-quit-wait.service". Isn't that what we are looking for in the first place?

[danimo]

/cc @fbuihuu

[fbuihuu]

Wouldn't it be better if it was the other way around: instead of making systemd referring to plymouth, we could make plymouth referring the emergency service.

For example plymouth could install a drop-in in /usr/lib/systemd/system/emergency.wants.d/plymouth-quit-wait.service.

[keszybz]

instead of making systemd referring to plymouth, we could make plymouth [refer to] the emergency service

Yeah, that sounds much nicer. But the downside is that coordination between the two projects would be required.

[mbiebl]

@keszybz don't we have that requirement already? What if plymouth one day changes its command line arguments?

That said, plymouth is not only referenced in the unit files but also in the systemd sources itself.

The plymouth calls in the unit files were just the most visible ones which confused a lot of users.
Recent example https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=858162#25

[keszybz]

Hm, after thinking about this some more, I think that we should keep the ugly shell work-around. Doing the same through dropins carried by plymouth would require a plymouth update each time we add or remove or rename units that interact with plymouth. This happens much more often than plymouth changing its option syntax.

[mbiebl]

Somehow I missed this: If plymouth is not installed

$ /bin/sh -c "[ -x /bin/plymouth ] && /bin/plymouth --wait quit"
$ echo $?
1

As a result, you get

● rescue.service - Rescue Shell
   Loaded: loaded (/lib/systemd/system/rescue.service; static; vendor preset: enabled)
   Active: active (running) since Wed 2017-03-22 08:09:48 CET; 2min 24s ago
     Docs: man:sulogin(8)
  Process: 447 ExecStartPre=/bin/echo -e You are in rescue mode. After logging in, type "journalctl -xb" to view\nsystem logs, "systemctl reboot" to reboot, "systemctl default" or ^D to\nboot into default mode. (code=exited, status=0/SUCCESS)
  Process: 424 ExecStartPre=/bin/sh -c [ -x /bin/plymouth ] && /bin/plymouth --wait quit (code=exited, status=1/FAILURE)

Notice the status=1/FAILURE

If we properly want to avoid an error, we should use

ExecStartPre=-/bin/sh -c "if [ -x /bin/plymouth ]; then /bin/plymouth --wait quit; fi"

[mbiebl]

I thought about this a bit more. Since we were already spawning two shells, and one echo, why not just use a shell script proper like /lib/systemd/systemd-rescue-shell

#!/bin/sh

if [ -x /bin/plymouth ]; then
    /bin/plymouth --wait quit
fi

cat <<EOF
You are in $1 mode. After logging in, type "journalctl -xb" to view
system logs, "systemctl reboot" to reboot, "systemctl default" or ^D to
boot into default mode.
EOF

/sbin/sulogin
/bin/systemctl --job-mode=fail --no-block default

This not only has the benefit that we only need to spawn a single shell, we can also use this for both rescue.service and emergency.service, which now simply have a

ExecStart=-/lib/systemd/systemd-rescue-shell rescue

or

ExecStart=-/lib/systemd/systemd-rescue-shell emergency

Much cleaner if you ask me. Should I prep a PR for that?

[martinpitt]

Indeed @mbiebl, that sounds much better!

[mbiebl]

@martinpitt I posted a PR at #5623 . Thoughts/review welcome