shared/install: also check for self-aliases during installation and ignore them

We had a check that was done in unit_file_resolve_symlink(). Let's move
the check to unit_validate_alias_symlink_or_warn(), which makes it available
to the code in install.c.

With this, unit_file_resolve_symlink() behaves almost the same. The warning
about "suspicious symlink" is done a bit later. I think this should be OK.

diff --git a/src/basic/unit-file.c b/src/basic/unit-file.c
index 0e9a556268c7df4b666936cb1daf8df783200285..6cf66b45cf61dbc3b99adcef5d715df4f8baadab 100644 (file)
--- a/src/basic/unit-file.c
+++ b/src/basic/unit-file.c
@@ -82,7 +82,8 @@ int unit_validate_alias_symlink_or_warn(int log_level, const char *filename, con
          *
          * -EINVAL is returned if the something is wrong with the source filename or the source unit type is
          *         not allowed to symlink,
-         * -EXDEV if the target filename is not a valid unit name or doesn't match the source.
+         * -EXDEV if the target filename is not a valid unit name or doesn't match the source,
+         * -ELOOP for an alias to self.
          */
 
         src = basename(filename);
@@ -111,6 +112,11 @@ int unit_validate_alias_symlink_or_warn(int log_level, const char *filename, con
 
         /* dst checks */
 
+        if (streq(src, dst))
+                return log_debug_errno(SYNTHETIC_ERRNO(ELOOP),
+                                       "%s: unit self-alias: %s → %s, ignoring.",
+                                       filename, src, dst);
+
         dst_name_type = unit_name_to_instance(dst, &dst_instance);
         if (dst_name_type < 0)
                 return log_full_errno(log_level, dst_name_type == -EINVAL ? SYNTHETIC_ERRNO(EXDEV) : dst_name_type,
@@ -347,24 +353,12 @@ int unit_file_resolve_symlink(
                 if (r < 0)
                         return r;
 
-                bool self_alias = streq(target_name, filename);
-
-                if (is_path(tail))
-                        log_full(self_alias ? LOG_DEBUG : LOG_WARNING,
-                                 "Suspicious symlink %s/%s→%s, treating as alias.",
-                                 dir, filename, simplified);
-
                 r = unit_validate_alias_symlink_or_warn(LOG_NOTICE, filename, simplified);
                 if (r < 0)
                         return r;
-
-                if (self_alias && !resolve_destination_target)
-                        /* A self-alias that has no effect when loading, let's just ignore it. */
-                        return log_debug_errno(SYNTHETIC_ERRNO(ELOOP),
-                                               "Unit file self-alias: %s/%s → %s, ignoring.",
-                                               dir, filename, target_name);
-
-                log_debug("Unit file alias: %s/%s → %s", dir, filename, target_name);
+                if (is_path(tail))
+                        log_warning("Suspicious symlink %s/%s→%s, treating as alias.",
+                                    dir, filename, simplified);
 
                 dst = resolve_destination_target ? TAKE_PTR(simplified) : TAKE_PTR(target_name);
         }

diff --git a/src/shared/install.c b/src/shared/install.c
index cfada1344240f1286b37359991a1b6732976c101..bc15b0e9d203ba305700d13a04b72937aba13b87 100644 (file)
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1695,6 +1695,11 @@ int unit_file_verify_alias(
          * ret_dst is set in cases where "instance propagation" happens, i.e. when the instance part is
          * inserted into dst. It is not normally set, even on success, so that the caller can easily
          * distinguish the case where instance propagation occurred.
+         *
+         * Returns:
+         * -EXDEV when the alias doesn't match the unit,
+         * -EUCLEAN when the name is invalid,
+         * -ELOOP when the alias it to the unit itself.
          */
 
         const char *path_alias = strrchr(dst, '/');
@@ -1760,6 +1765,8 @@ int unit_file_verify_alias(
                 }
 
                 r = unit_validate_alias_symlink_or_warn(LOG_DEBUG, dst_updated ?: dst, info->name);
+                if (r == -ELOOP)  /* -ELOOP means self-alias, which we (quietly) ignore */
+                        return r;
                 if (r < 0) {
                         unit_file_changes_add(changes, n_changes,
                                               r == -EINVAL ? -EXDEV : r,
@@ -1799,6 +1806,8 @@ static int install_info_symlink_alias(
                 }
 
                 q = unit_file_verify_alias(info, dst, &dst_updated, changes, n_changes);
+                if (q == -ELOOP)
+                        continue;
                 if (q < 0) {
                         r = r < 0 ? r : q;
                         continue;

diff --git a/test/test-systemctl-enable.sh b/test/test-systemctl-enable.sh
index 32bc6e5ef7892866553fb2bf240563a7ebf51cd7..4117436462d79e6f3881f23edab4e7aab2f8d410 100644 (file)
--- a/test/test-systemctl-enable.sh
+++ b/test/test-systemctl-enable.sh
@@ -328,8 +328,7 @@ test ! -h "$root/etc/systemd/system/services.target.wants/templ1@two.service"
 test ! -e "$root/etc/systemd/system/link4.service"
 cat >"$root/etc/systemd/system/link4.service" <<EOF
 [Install]
-# FIXME: self-alias should be ignored
-# Alias=link4.service
+Alias=link4.service
 Alias=link4@.service
 Alias=link4@inst.service
 Alias=link4alias.service
@@ -372,8 +371,7 @@ test ! -h "$root/etc/systemd/system/link4alias2.service"
 test ! -e "$root/etc/systemd/system/link5.service"
 cat >"$root/etc/systemd/system/link5.service" <<EOF
 [Install]
-# FIXME: self-alias should be ignored
-# Alias=link5.service
+Alias=link5.service
 Alias=link5alias.service
 Alias=link5alias2.service
 EOF