install: follow unit file symlinks in /usr, but not /etc when looking for [Install] data

Some distributions use alias unit files via symlinks in /usr to cover
for legacy service names. With this change we'll allow "systemctl
enable" on such aliases.

Previously, our rule was that symlinks are user configuration that
"systemctl enable" + "systemctl disable" creates and removes, while unit
files is where the instructions to do so are store. As a result of the
rule we'd never read install information through symlinks, since that
would mix enablement state with installation instructions.

Now, the new rule is that only symlinks inside of /etc are
configuration. Unit files, and symlinks in /usr are now valid for
installation instructions.

This patch is quite a rework of the whole install logic, and makes the
following addional changes:

- Adds a complete test "test-instal-root" that tests the install logic
  pretty comprehensively.

- Never uses canonicalize_file_name(), because that's incompatible with
  operation relative to a specific root directory.

- unit_file_get_state() is reworked to return a proper error, and
  returns the state in a call-by-ref parameter. This cleans up confusion
  between the enum type and errno-like errors.

- The new logic puts a limit on how long to follow unit file symlinks:
  it will do so only for 64 steps at max.

- The InstallContext object's fields are renamed to will_process and
  has_processed (will_install and has_installed) since they are also
  used for deinstallation and all kinds of other operations.

- The root directory is always verified before use.

- install.c is reordered to place the exported functions together.

- Stricter rules are followed when traversing symlinks: the unit suffix
  must say identical, and it's not allowed to link between regular units
  and templated units.

- Various modernizations

- The "invalid" unit file state has been renamed to "bad", in order to
  avoid confusion between UNIT_FILE_INVALID and
  _UNIT_FILE_STATE_INVALID. Given that the state should normally not be
  seen and is not documented this should not be a problematic change.
  The new name is now documented however.

Fixes #1375, #1718, #1706

1 files changed, 50 insertions, 21 deletions

diff --git a/src/test/test-install.c b/src/test/test-install.c
index 5ee52e64cb..359b262347 100644
--- a/src/test/test-install.c
+++ b/src/test/test-install.c
@@ -46,17 +46,19 @@ int main(int argc, char* argv[]) {
         const char *const files2[] = { "/home/lennart/test.service", NULL };
         UnitFileChange *changes = NULL;
         unsigned n_changes = 0;
+        UnitFileState state = 0;
 
         h = hashmap_new(&string_hash_ops);
         r = unit_file_get_list(UNIT_FILE_SYSTEM, NULL, h);
         assert_se(r == 0);
 
         HASHMAP_FOREACH(p, h, i) {
-                UnitFileState s;
+                UnitFileState s = _UNIT_FILE_STATE_INVALID;
 
-                s = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(p->path));
+                r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(p->path), &s);
 
-                assert_se(p->state == s);
+                assert_se((r < 0 && p->state == UNIT_FILE_BAD) ||
+                          (p->state == s));
 
                 fprintf(stderr, "%s (%s)\n",
                         p->path,
@@ -78,7 +80,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_ENABLED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_ENABLED);
 
         log_error("disable");
 
@@ -91,7 +95,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_DISABLED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_DISABLED);
 
         log_error("mask");
         changes = NULL;
@@ -106,7 +112,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_MASKED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_MASKED);
 
         log_error("unmask");
         changes = NULL;
@@ -121,7 +129,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_DISABLED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_DISABLED);
 
         log_error("mask");
         changes = NULL;
@@ -133,7 +143,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_MASKED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_MASKED);
 
         log_error("disable");
         changes = NULL;
@@ -148,7 +160,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_MASKED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_MASKED);
 
         log_error("umask");
         changes = NULL;
@@ -160,7 +174,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == UNIT_FILE_DISABLED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0], &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_DISABLED);
 
         log_error("enable files2");
         changes = NULL;
@@ -172,19 +188,22 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == UNIT_FILE_ENABLED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_ENABLED);
 
         log_error("disable files2");
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == _UNIT_FILE_STATE_INVALID);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
+        assert_se(r < 0);
 
         log_error("link files2");
         changes = NULL;
@@ -196,19 +215,22 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == UNIT_FILE_LINKED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_LINKED);
 
         log_error("disable files2");
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == _UNIT_FILE_STATE_INVALID);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
+        assert_se(r < 0);
 
         log_error("link files2");
         changes = NULL;
@@ -220,7 +242,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == UNIT_FILE_LINKED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_LINKED);
 
         log_error("reenable files2");
         changes = NULL;
@@ -232,19 +256,22 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == UNIT_FILE_ENABLED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_ENABLED);
 
         log_error("disable files2");
         changes = NULL;
         n_changes = 0;
 
-        r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, &changes, &n_changes);
+        r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, STRV_MAKE(basename(files2[0])), &changes, &n_changes);
         assert_se(r >= 0);
 
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0])) == _UNIT_FILE_STATE_INVALID);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files2[0]), &state);
+        assert_se(r < 0);
         log_error("preset files");
         changes = NULL;
         n_changes = 0;
@@ -255,7 +282,9 @@ int main(int argc, char* argv[]) {
         dump_changes(changes, n_changes);
         unit_file_changes_free(changes, n_changes);
 
-        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files[0])) == UNIT_FILE_ENABLED);
+        r = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, basename(files[0]), &state);
+        assert_se(r >= 0);
+        assert_se(state == UNIT_FILE_ENABLED);
 
         return 0;
 }