From eb79d39138d36f6f184eb30187646df1226cac67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 10 Jan 2022 12:02:03 +0100 Subject: [PATCH 1/3] xdg-autostart-generator: make parameter const --- src/xdg-autostart-generator/xdg-autostart-service.c | 2 +- src/xdg-autostart-generator/xdg-autostart-service.h | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/xdg-autostart-generator/xdg-autostart-service.c b/src/xdg-autostart-generator/xdg-autostart-service.c index 241a5b3cfd..3fd1f4bbf2 100644 --- a/src/xdg-autostart-generator/xdg-autostart-service.c +++ b/src/xdg-autostart-generator/xdg-autostart-service.c @@ -502,7 +502,7 @@ static int xdg_autostart_generate_desktop_condition( } int xdg_autostart_service_generate_unit( - XdgAutostartService *service, + const XdgAutostartService *service, const char *dest) { _cleanup_free_ char *path_escaped = NULL, *exec_start = NULL, *unit = NULL; diff --git a/src/xdg-autostart-generator/xdg-autostart-service.h b/src/xdg-autostart-generator/xdg-autostart-service.h index 2641718899..61a4a7304d 100644 --- a/src/xdg-autostart-generator/xdg-autostart-service.h +++ b/src/xdg-autostart-generator/xdg-autostart-service.h @@ -26,7 +26,6 @@ typedef struct XdgAutostartService { } XdgAutostartService; - XdgAutostartService * xdg_autostart_service_free(XdgAutostartService *s); DEFINE_TRIVIAL_CLEANUP_FUNC(XdgAutostartService*, xdg_autostart_service_free); @@ -34,4 +33,4 @@ char *xdg_autostart_service_translate_name(const char *name); int xdg_autostart_format_exec_start(const char *exec, char **ret_exec_start); XdgAutostartService *xdg_autostart_service_parse_desktop(const char *path); -int xdg_autostart_service_generate_unit(XdgAutostartService *service, const char *dest); +int xdg_autostart_service_generate_unit(const XdgAutostartService *service, const char *dest); From 0d1610c9c93c10da737d0741d327d3f78bd1119b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 10 Jan 2022 12:06:38 +0100 Subject: [PATCH 2/3] xdg-autostart-generator: rework debug logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The logs used the service name as the primary log key. But the service name often needs to contain escape symbols, and the logs are rather hard to read because of this. Thus the logs are changed to use the path to the source desktop file. I think this is much more useful because the user will want to look at the source file too and maybe change it if something goes wrong. A bit more logging to show which directories we are looking at and why we skip certain units is added too. $ rm -rf /tmp/out && mkdir /tmp/out && SYSTEMD_LOG_LEVEL=debug SYSTEMD_LOG_TARGET=console build/systemd-xdg-autostart-generator /tmp/{out,out,out} Scanning autostart directory "/home/zbyszek/.config/autostart"… Scanning autostart directory "/etc/xdg/autostart"… /etc/xdg/autostart/tracker-miner-rss-3.desktop: not generating unit, marked as skipped by generator. /etc/xdg/autostart/gnome-initial-setup-first-login.desktop: ExecCondition executable gnome-systemd-autostart-condition not found, unit will not be started automatically: No such file or directory /etc/xdg/autostart/geoclue-demo-agent.desktop: symlinking app-geoclue\x2ddemo\x2dagent@autostart.service in xdg-desktop-autostart.target/.wants… SELinux enabled state cached to: disabled Directory "/tmp" already exists, but has mode 0777 that is too permissive (0755 was requested), refusing. /etc/xdg/autostart/polkit-mate-authentication-agent-1.desktop: symlinking app-polkit\x2dmate\x2dauthentication\x2dagent\x2d1@autostart.service in xdg-desktop-autostart.target/.wants… /etc/xdg/autostart/mate-settings-daemon.desktop: symlinking app-mate\x2dsettings\x2ddaemon@autostart.service in xdg-desktop-autostart.target/.wants… /etc/xdg/autostart/user-dirs-update-gtk.desktop: symlinking app-user\x2ddirs\x2dupdate\x2dgtk@autostart.service in xdg-desktop-autostart.target/.wants… /etc/xdg/autostart/org.freedesktop.problems.applet.desktop: symlinking app-org.freedesktop.problems.applet@autostart.service in xdg-desktop-autostart.target/.wants… /etc/xdg/autostart/org.gnome.SettingsDaemon.Datetime.desktop: not generating unit, startup phases are not supported. /etc/xdg/autostart/org.gnome.SettingsDaemon.XSettings.desktop: not generating unit, startup phases are not supported. /etc/xdg/autostart/org.gnome.SettingsDaemon.DiskUtilityNotify.desktop: symlinking app-org.gnome.SettingsDaemon.DiskUtilityNotify@autostart.service in xdg-desktop-autostart.target/.wants… /etc/xdg/autostart/gnome-initial-setup-copy-worker.desktop: not generating unit, startup phases are not supported. /etc/xdg/autostart/org.gnome.Evolution-alarm-notify.desktop: symlinking app-org.gnome.Evolution\x2dalarm\x2dnotify@autostart.service in xdg-desktop-autostart.target/.wants… /etc/xdg/autostart/tracker-miner-fs-3.desktop: not generating unit, marked as skipped by generator. /etc/xdg/autostart/orca-autostart.desktop: ExecCondition executable gnome-systemd-autostart-condition not found, unit will not be started automatically: No such file or directory ... Inspired by https://bugzilla.redhat.com/show_bug.cgi?id=2038750. The return value from xdg_autostart_service_generate_unit() is ignored by the caller, so we can do a shortcut return without functional change. This is nicer because we're now consistently always returning an error if something failed. --- .../xdg-autostart-generator.c | 26 +++++++------ .../xdg-autostart-service.c | 37 ++++++++++--------- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/xdg-autostart-generator/xdg-autostart-generator.c b/src/xdg-autostart-generator/xdg-autostart-generator.c index c5c6b54fdc..8bc16831e7 100644 --- a/src/xdg-autostart-generator/xdg-autostart-generator.c +++ b/src/xdg-autostart-generator/xdg-autostart-generator.c @@ -44,38 +44,42 @@ static int enumerate_xdg_autostart(Hashmap *all_services) { STRV_FOREACH(path, autostart_dirs) { _cleanup_closedir_ DIR *d = NULL; + log_debug("Scanning autostart directory \"%s\"…", *path); d = opendir(*path); if (!d) { - if (errno != ENOENT) - log_warning_errno(errno, "Opening %s failed, ignoring: %m", *path); + log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, errno, + "Opening %s failed, ignoring: %m", *path); continue; } FOREACH_DIRENT(de, d, log_warning_errno(errno, "Failed to enumerate directory %s, ignoring: %m", *path)) { - _cleanup_free_ char *fpath = NULL, *name = NULL; - _cleanup_(xdg_autostart_service_freep) XdgAutostartService *service = NULL; struct stat st; - if (fstatat(dirfd(d), de->d_name, &st, 0) < 0) { - log_warning_errno(errno, "stat() failed on %s/%s, ignoring: %m", *path, de->d_name); + log_warning_errno(errno, "%s/%s: stat() failed, ignoring: %m", *path, de->d_name); continue; } - if (!S_ISREG(st.st_mode)) + if (!S_ISREG(st.st_mode)) { + log_debug("%s/%s: not a regular file, ignoring.", *path, de->d_name); continue; + } - name = xdg_autostart_service_translate_name(de->d_name); + _cleanup_free_ char *name = xdg_autostart_service_translate_name(de->d_name); if (!name) return log_oom(); - if (hashmap_contains(all_services, name)) + if (hashmap_contains(all_services, name)) { + log_debug("%s/%s: we have already seen \"%s\", ignoring.", + *path, de->d_name, name); continue; + } - fpath = path_join(*path, de->d_name); + _cleanup_free_ char *fpath = path_join(*path, de->d_name); if (!fpath) return log_oom(); - service = xdg_autostart_service_parse_desktop(fpath); + _cleanup_(xdg_autostart_service_freep) XdgAutostartService *service = + xdg_autostart_service_parse_desktop(fpath); if (!service) return log_oom(); service->name = TAKE_PTR(name); diff --git a/src/xdg-autostart-generator/xdg-autostart-service.c b/src/xdg-autostart-generator/xdg-autostart-service.c index 3fd1f4bbf2..32a4958aed 100644 --- a/src/xdg-autostart-generator/xdg-autostart-service.c +++ b/src/xdg-autostart-generator/xdg-autostart-service.c @@ -470,6 +470,7 @@ int xdg_autostart_format_exec_start( } static int xdg_autostart_generate_desktop_condition( + const XdgAutostartService *service, FILE *f, const char *test_binary, const char *condition) { @@ -483,7 +484,8 @@ static int xdg_autostart_generate_desktop_condition( r = find_executable(test_binary, &gnome_autostart_condition_path); if (r < 0) { log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r, - "%s not found: %m", test_binary); + "%s: ExecCondition executable %s not found, unit will not be started automatically: %m", + service->path, test_binary); fprintf(f, "# ExecCondition using %s skipped due to missing binary.\n", test_binary); return r; } @@ -492,6 +494,9 @@ static int xdg_autostart_generate_desktop_condition( if (!e_autostart_condition) return log_oom(); + log_debug("%s: ExecCondition converted to %s --condition \"%s\"…", + service->path, gnome_autostart_condition_path, e_autostart_condition); + fprintf(f, "ExecCondition=%s --condition \"%s\"\n", gnome_autostart_condition_path, @@ -513,23 +518,23 @@ int xdg_autostart_service_generate_unit( /* Nothing to do for hidden services. */ if (service->hidden) { - log_debug("Not generating service for XDG autostart %s, it is hidden.", service->name); + log_debug("%s: not generating unit, entry is hidden.", service->path); return 0; } if (service->systemd_skip) { - log_debug("Not generating service for XDG autostart %s, should be skipped by generator.", service->name); + log_debug("%s: not generating unit, marked as skipped by generator.", service->path); return 0; } /* Nothing to do if type is not Application. */ if (!streq_ptr(service->type, "Application")) { - log_debug("Not generating service for XDG autostart %s, only Type=Application is supported.", service->name); + log_debug("%s: not generating unit, Type=%s is not supported.", service->path, service->type); return 0; } if (!service->exec_string) { - log_warning("Not generating service for XDG autostart %s, it is has no Exec= line.", service->name); + log_warning("%s: not generating unit, no Exec= line.", service->path); return 0; } @@ -539,24 +544,21 @@ int xdg_autostart_service_generate_unit( r = find_executable(service->try_exec, NULL); if (r < 0) { log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r, - "Not generating service for XDG autostart %s, could not find TryExec= binary %s: %m", - service->name, service->try_exec); + "%s: not generating unit, could not find TryExec= binary %s: %m", + service->path, service->try_exec); return 0; } } r = xdg_autostart_format_exec_start(service->exec_string, &exec_start); if (r < 0) { - log_warning_errno(r, - "Not generating service for XDG autostart %s, error parsing Exec= line: %m", - service->name); + log_warning_errno(r, "%s: not generating unit, error parsing Exec= line: %m", service->path); return 0; } if (service->gnome_autostart_phase) { /* There is no explicit value for the "Application" phase. */ - log_debug("Not generating service for XDG autostart %s, startup phases are not supported.", - service->name); + log_debug("%s: not generating unit, startup phases are not supported.", service->path); return 0; } @@ -570,7 +572,7 @@ int xdg_autostart_service_generate_unit( f = fopen(unit, "wxe"); if (!f) - return log_error_errno(errno, "Failed to create unit file %s: %m", unit); + return log_error_errno(errno, "%s: failed to create unit file %s: %m", service->path, unit); fprintf(f, "# Automatically generated by systemd-xdg-autostart-generator\n\n" @@ -635,19 +637,18 @@ int xdg_autostart_service_generate_unit( e_not_show_in); } - r = xdg_autostart_generate_desktop_condition(f, + r = xdg_autostart_generate_desktop_condition(service, f, "gnome-systemd-autostart-condition", service->autostart_condition); if (r < 0) return r; - r = xdg_autostart_generate_desktop_condition(f, + r = xdg_autostart_generate_desktop_condition(service, f, "kde-systemd-start-condition", service->kde_autostart_condition); if (r < 0) return r; - (void) generator_add_symlink(dest, "xdg-desktop-autostart.target", "wants", service->name); - - return 0; + log_debug("%s: symlinking %s in xdg-desktop-autostart.target/.wants…", service->path, service->name); + return generator_add_symlink(dest, "xdg-desktop-autostart.target", "wants", service->name); } From 8c19f79e40cfd4f825185d835fce51e82a758f19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 10 Jan 2022 12:07:20 +0100 Subject: [PATCH 3/3] docs: fix typo in key name and adjust grammar a bit --- docs/DESKTOP_ENVIRONMENTS.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/DESKTOP_ENVIRONMENTS.md b/docs/DESKTOP_ENVIRONMENTS.md index b5195da26c..0a0eff6539 100644 --- a/docs/DESKTOP_ENVIRONMENTS.md +++ b/docs/DESKTOP_ENVIRONMENTS.md @@ -97,13 +97,13 @@ to create appropriate units for the autostart directory (`systemd-xdg-autostart-generator`). Desktop Environments can opt-in to using this by starting `xdg-desktop-autostart.target`. The systemd generator correctly handles -`OnlyShowIn=` and `NotShowin=`. It also handles the KDE and GNOME specific -`X-KDE-autostart-condition=` and `AutostartCondition=` by using desktop -environment provided binaries in an `ExecCondition=` line. +`OnlyShowIn=` and `NotShowIn=`. It also handles the KDE and GNOME specific +`X-KDE-autostart-condition=` and `AutostartCondition=` by using desktop-environment-provided +binaries in an `ExecCondition=` line. However, this generator is somewhat limited in what it supports. For example, all generated units will have `After=graphical-session.target` set on them, -it may therefore not be useful to start session services. +and therefore may not be useful to start session services. Desktop files can be marked to be explicitly excluded from the generator using the line `X-systemd-skip=true`. This should be set if an application provides its own