Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow calls for systemd graphical target to fail #34

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

jokeyrhyme
Copy link
Contributor

@jokeyrhyme jokeyrhyme commented Jan 11, 2024

  • extracted out common std::process::Command work, and made that function swallow errors rather than return them or panic (because we want to keep systemd optional)
  • pass a null stdin to std::process::Command, because it's allegedly faster to skip inheriting the parent's stdin (and it's not our intention to allow this inheritance anyway)
  • switch from .spawn() to .status() so that we can at least find out if systemctl accepted our calls (mostly for diagnostic purposes)
  • pass --no-block to systemctl when starting/stopping the graphical target, because we care mostly that we called into systemd successfully, not what happens inside systemd after that

in all testing below, the session and compositor continue to launch as normal:

  • tested the happy path, and that systemctl --user status cosmic-session.target shows it is active
  • tested with systemctl-foo as the hardcoded command, to confirm that that the Err branch results in the expected logs, and that systemctl --user status cosmic-session.target shows it is inactive
  • tested with cosmic-session-foo.target as the hardcoded target, to confirm that that the non-zero exit status code branch results in the expected logs, and that systemctl --user status cosmic-session.target shows it is inactive

@jokeyrhyme
Copy link
Contributor Author

Another thought I had is that we could detect systemd the same way cosmic-comp does (using the libsystemd crate), and then change the behaviour of this PR (or in a follow-up PR) to not even attempt to call systemctl if systemd is not running

That would keep logs cleaner on non-systemd systems

@Drakulix
Copy link
Member

Another thought I had is that we could detect systemd the same way cosmic-comp does (using the libsystemd crate), and then change the behaviour of this PR (or in a follow-up PR) to not even attempt to call systemctl if systemd is not running

That would keep logs cleaner on non-systemd systems

It might, but we would pull another dbus api, which we don't really need. It think this is fine.

@Drakulix Drakulix merged commit efe4b58 into pop-os:master_jammy Jan 16, 2024
@jokeyrhyme jokeyrhyme deleted the optional-systemd-target branch January 16, 2024 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants