xdg-autostart-generator: rework debug logging

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.

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);
 }