shared/install: create relative symlinks for enablement and aliasing

This is a fairly noticable change, but I think it needs to be done.
So far we'd create an absolute symlink to the target unit file:
  .wants/foo.service → /usr/lib/systemd/system/foo.service
or
  alias.service → /etc/systemd/system/aliased.service.

This works reasonably well, except in one case: where the unit file
is linked. When we look at a file link, the name of the physical file
isn't used, and we only take the account the symlink source name.
(In fact, the destination filename may not even be a well-formed unit name,
so we couldn't use it, even if we wanted to.) But this means that if
a file is linked, and specifies aliases, we'd create absolute links for
those aliases, and systemd would consider each "alias" to be a separate
unit. This isn't checked by the tests here, because we don't have a running
systemd instance, but it is easy enough to check manually.

The most reasonable way to fix this is to create relative links to the
unit file:
  .wants/foo.service → ../foo.service
  alias.service → aliased.service.

I opted to use no prefix for aliases, both normal and 'default.target',
and to add "../" for .wants/ and .requires/. Note that the link that is
created doesn't necessarily point to the file. E.g. if we're enabling
a file under /usr/lib/systemd/system, and create a symlink in /etc/systemd/system,
it'll still be "../foo.service", not "../../usr/lib/systemd/system/foo.service".
For our unit loading logic this doesn't matter, and figuring out a path
that actually leads somewhere would be more work. Since the user is allowed
to move the unit file, or add a new unit file in a different location, and
we don't actually follow the symlink, I think it's OK to create a dangling
symlink. The prefix of "../" is useful to give a hint that the link points
to files that are conceptually "one level up" in the directory hierarchy.

With the relative symlinks, systemd knows that those are aliases.

The tests are adjusted to use the new forms. There were a few tests that
weren't really testing something useful: 'test -e x' fails if 'x' is a
a dangling symlink. Absolute links in the chroot would be dangling, even
though the target existed in the expected path, but become non-dangling
when made relative and the test fails.

This should be described in NEWS, but I'm not adding that here, because
it'd likely result in conflicts.

2 files changed, 43 insertions, 36 deletions

diff --git a/src/shared/install.c b/src/shared/install.c
index 3ec3772e01..bf110b9e56 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1837,7 +1837,7 @@ static int install_info_symlink_alias(
                 if (!alias_path)
                         return -ENOMEM;
 
-                q = create_symlink(lp, info->path, alias_path, force, changes, n_changes);
+                q = create_symlink(lp, info->name, alias_path, force, changes, n_changes);
                 r = r < 0 ? r : q;
         }
 
@@ -1906,7 +1906,7 @@ static int install_info_symlink_wants(
         }
 
         STRV_FOREACH(s, list) {
-                _cleanup_free_ char *path = NULL, *dst = NULL;
+                _cleanup_free_ char *dst = NULL;
 
                 q = install_name_printf(scope, info, *s, info->root, &dst);
                 if (q < 0) {
@@ -1936,11 +1936,15 @@ static int install_info_symlink_wants(
                         continue;
                 }
 
-                path = strjoin(config_path, "/", dst, suffix, n);
+                _cleanup_free_ char *path = strjoin(config_path, "/", dst, suffix, n);
                 if (!path)
                         return -ENOMEM;
 
-                q = create_symlink(lp, info->path, path, true, changes, n_changes);
+                _cleanup_free_ char *target = strjoin("../", info->name);
+                if (!target)
+                        return -ENOMEM;
+
+                q = create_symlink(lp, target, path, true, changes, n_changes);
                 if (r == 0)
                         r = q;
 
@@ -2848,7 +2852,7 @@ int unit_file_set_default(
                 return r;
 
         new_path = strjoina(lp.persistent_config, "/" SPECIAL_DEFAULT_TARGET);
-        return create_symlink(&lp, info->path, new_path, flags & UNIT_FILE_FORCE, changes, n_changes);
+        return create_symlink(&lp, info->name, new_path, flags & UNIT_FILE_FORCE, changes, n_changes);
 }
 
 int unit_file_get_default(
diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index 4f66c12655..dca695d124 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -88,7 +88,7 @@ TEST(basic_mask_and_enable) {
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) == 1);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/a.service"));
+        assert_se(streq(changes[0].source, "../a.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/a.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -128,7 +128,7 @@ TEST(basic_mask_and_enable) {
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("d.service"), &changes, &n_changes) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/a.service"));
+        assert_se(streq(changes[0].source, "../a.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/a.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -147,7 +147,7 @@ TEST(basic_mask_and_enable) {
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/a.service");
         assert_se(streq(changes[0].path, p));
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[1].source, "/usr/lib/systemd/system/a.service"));
+        assert_se(streq(changes[1].source, "../a.service"));
         assert_se(streq(changes[1].path, p));
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
@@ -186,7 +186,7 @@ TEST(basic_mask_and_enable) {
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("f.service"), &changes, &n_changes) == 1);
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/f.service"));
+        assert_se(streq(changes[0].source, "../f.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/x.target.wants/f.service");
         assert_se(streq(changes[0].path, p));
         assert_se(changes[1].type_or_errno == UNIT_FILE_DESTINATION_NOT_PRESENT);
@@ -280,7 +280,8 @@ TEST(linked_units) {
         q = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked.service");
         for (i = 0 ; i < n_changes; i++) {
                 assert_se(changes[i].type_or_errno == UNIT_FILE_SYMLINK);
-                assert_se(streq(changes[i].source, "/opt/linked.service"));
+                assert_se(STR_IN_SET(changes[i].source,
+                                     "../linked.service", "/opt/linked.service"));
 
                 if (p && streq(changes[i].path, p))
                         p = NULL;
@@ -322,7 +323,8 @@ TEST(linked_units) {
         q = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked2.service");
         for (i = 0 ; i < n_changes; i++) {
                 assert_se(changes[i].type_or_errno == UNIT_FILE_SYMLINK);
-                assert_se(streq(changes[i].source, "/opt/linked2.service"));
+                assert_se(STR_IN_SET(changes[i].source,
+                                     "../linked2.service", "/opt/linked2.service"));
 
                 if (p && streq(changes[i].path, p))
                         p = NULL;
@@ -340,7 +342,7 @@ TEST(linked_units) {
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(startswith(changes[0].path, root));
         assert_se(endswith(changes[0].path, "linked3.service"));
-        assert_se(streq(changes[0].source, "/opt/linked3.service"));
+        assert_se(streq(changes[0].source, "../linked3.service"));
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 }
@@ -371,7 +373,7 @@ TEST(default) {
         assert_se(unit_file_set_default(UNIT_FILE_SYSTEM, 0, root, "test-default.target", &changes, &n_changes) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/test-default-real.target"));
+        assert_se(streq(changes[0].source, "test-default-real.target"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR "/" SPECIAL_DEFAULT_TARGET);
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -401,7 +403,7 @@ TEST(add_dependency) {
         assert_se(unit_file_add_dependency(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("add-dependency-test-service.service"), "add-dependency-test-target.target", UNIT_WANTS, &changes, &n_changes) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/real-add-dependency-test-service.service"));
+        assert_se(streq(changes[0].source, "../real-add-dependency-test-service.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/real-add-dependency-test-target.target.wants/real-add-dependency-test-service.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -442,7 +444,7 @@ TEST(template_enable) {
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@.service"), &changes, &n_changes) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
+        assert_se(streq(changes[0].source, "../template@.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/template@def.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -473,13 +475,14 @@ TEST(template_enable) {
 
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@foo.service"), &changes, &n_changes) >= 0);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
+        assert_se(streq(changes[0].source, "../template@foo.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/template@foo.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0 && state == UNIT_FILE_INDIRECT);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0);
+        assert_se(state == UNIT_FILE_INDIRECT);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
@@ -506,7 +509,7 @@ TEST(template_enable) {
 
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template-symlink@quux.service"), &changes, &n_changes) >= 0);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service"));
+        assert_se(streq(changes[0].source, "../template@quux.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/template@quux.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -552,7 +555,7 @@ TEST(indirect) {
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("indirectc.service"), &changes, &n_changes) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/indirectb.service"));
+        assert_se(streq(changes[0].source, "../indirectb.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/indirectb.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -604,7 +607,7 @@ TEST(preset_and_list) {
         assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("preset-yes.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/preset-yes.service"));
+        assert_se(streq(changes[0].source, "../preset-yes.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/preset-yes.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -641,7 +644,7 @@ TEST(preset_and_list) {
         for (i = 0; i < n_changes; i++) {
 
                 if (changes[i].type_or_errno == UNIT_FILE_SYMLINK) {
-                        assert_se(streq(changes[i].source, "/usr/lib/systemd/system/preset-yes.service"));
+                        assert_se(streq(changes[i].source, "../preset-yes.service"));
                         assert_se(streq(changes[i].path, p));
                 } else
                         assert_se(changes[i].type_or_errno == UNIT_FILE_UNLINK);
@@ -757,7 +760,7 @@ TEST(preset_order) {
         assert_se(unit_file_preset(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("prefix-1.service"), UNIT_FILE_PRESET_FULL, &changes, &n_changes) >= 0);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/prefix-1.service"));
+        assert_se(streq(changes[0].source, "../prefix-1.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/prefix-1.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -866,8 +869,8 @@ TEST(with_dropin) {
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-1.service"));
-        assert_se(streq(changes[1].source, "/usr/lib/systemd/system/with-dropin-1.service"));
+        assert_se(streq(changes[0].source, "../with-dropin-1.service"));
+        assert_se(streq(changes[1].source, "../with-dropin-1.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-1.service");
         assert_se(streq(changes[0].path, p));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/graphical.target.wants/with-dropin-1.service");
@@ -880,8 +883,8 @@ TEST(with_dropin) {
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, SYSTEM_CONFIG_UNIT_DIR"/with-dropin-2.service"));
-        assert_se(streq(changes[1].source, SYSTEM_CONFIG_UNIT_DIR"/with-dropin-2.service"));
+        assert_se(streq(changes[0].source, "../with-dropin-2.service"));
+        assert_se(streq(changes[1].source, "../with-dropin-2.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-2.service");
         assert_se(streq(changes[0].path, p));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/graphical.target.wants/with-dropin-2.service");
@@ -894,8 +897,8 @@ TEST(with_dropin) {
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-3.service"));
-        assert_se(streq(changes[1].source, "/usr/lib/systemd/system/with-dropin-3.service"));
+        assert_se(streq(changes[0].source, "../with-dropin-3.service"));
+        assert_se(streq(changes[1].source, "../with-dropin-3.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-3.service");
         assert_se(streq(changes[0].path, p));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/graphical.target.wants/with-dropin-3.service");
@@ -908,8 +911,8 @@ TEST(with_dropin) {
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-4a.service"));
-        assert_se(streq(changes[1].source, "/usr/lib/systemd/system/with-dropin-4b.service"));
+        assert_se(streq(changes[0].source, "../with-dropin-4a.service"));
+        assert_se(streq(changes[1].source, "../with-dropin-4b.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-4a.service");
         assert_se(streq(changes[0].path, p));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-4b.service");
@@ -975,8 +978,8 @@ TEST(with_dropin_template) {
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-1@.service"));
-        assert_se(streq(changes[1].source, "/usr/lib/systemd/system/with-dropin-1@.service"));
+        assert_se(streq(changes[0].source, "../with-dropin-1@instance-1.service"));
+        assert_se(streq(changes[1].source, "../with-dropin-1@instance-1.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-1@instance-1.service");
         assert_se(streq(changes[0].path, p));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/graphical.target.wants/with-dropin-1@instance-1.service");
@@ -988,8 +991,8 @@ TEST(with_dropin_template) {
         assert_se(n_changes == 2);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
         assert_se(changes[1].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-2@.service"));
-        assert_se(streq(changes[1].source, "/usr/lib/systemd/system/with-dropin-2@.service"));
+        assert_se(streq(changes[0].source, "../with-dropin-2@instance-1.service"));
+        assert_se(streq(changes[1].source, "../with-dropin-2@instance-1.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-2@instance-1.service");
         assert_se(streq(changes[0].path, p));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/graphical.target.wants/with-dropin-2@instance-1.service");
@@ -1000,7 +1003,7 @@ TEST(with_dropin_template) {
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-2@instance-2.service"), &changes, &n_changes) == 1);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-2@.service"));
+        assert_se(streq(changes[0].source, "../with-dropin-2@instance-2.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-2@instance-2.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);
@@ -1009,7 +1012,7 @@ TEST(with_dropin_template) {
         assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-3@.service"), &changes, &n_changes) == 1);
         assert_se(n_changes == 1);
         assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK);
-        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/with-dropin-3@.service"));
+        assert_se(streq(changes[0].source, "../with-dropin-3@.service"));
         p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/with-dropin-3@instance-2.service");
         assert_se(streq(changes[0].path, p));
         unit_file_changes_free(changes, n_changes);