Merge pull request #3183 from crawford/preset-array

install: cache the presets before evaluating

2 files changed, 172 insertions, 26 deletions

diff --git a/src/shared/install.c b/src/shared/install.c
index f02d81504f..cc36da1853 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -66,6 +66,35 @@ typedef struct {
         OrderedHashmap *have_processed;
 } InstallContext;
 
+typedef enum {
+        PRESET_UNKNOWN,
+        PRESET_ENABLE,
+        PRESET_DISABLE,
+} PresetAction;
+
+typedef struct {
+        char *pattern;
+        PresetAction action;
+} PresetRule;
+
+typedef struct {
+        PresetRule *rules;
+        size_t n_rules;
+} Presets;
+
+static inline void presets_freep(Presets *p) {
+        size_t i;
+
+        if (!p)
+                return;
+
+        for (i = 0; i < p->n_rules; i++)
+                free(p->rules[i].pattern);
+
+        free(p->rules);
+        p->n_rules = 0;
+}
+
 static int unit_file_lookup_state(UnitFileScope scope, const LookupPaths *paths, const char *name, UnitFileState *ret);
 
 bool unit_type_may_alias(UnitType type) {
@@ -2434,17 +2463,16 @@ int unit_file_exists(UnitFileScope scope, const LookupPaths *paths, const char *
         return 1;
 }
 
-int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name) {
+static int read_presets(UnitFileScope scope, const char *root_dir, Presets *presets) {
+        _cleanup_(presets_freep) Presets ps = {};
+        size_t n_allocated = 0;
         _cleanup_strv_free_ char **files = NULL;
         char **p;
         int r;
 
         assert(scope >= 0);
         assert(scope < _UNIT_FILE_SCOPE_MAX);
-        assert(name);
-
-        if (!unit_name_is_valid(name, UNIT_NAME_ANY))
-                return -EINVAL;
+        assert(presets);
 
         if (scope == UNIT_FILE_SYSTEM)
                 r = conf_files_list(&files, ".preset", root_dir,
@@ -2461,8 +2489,11 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
                                     "/usr/local/lib/systemd/user-preset",
                                     "/usr/lib/systemd/user-preset",
                                     NULL);
-        else
-                return 1; /* Default is "enable" */
+        else {
+                *presets = (Presets){};
+
+                return 0;
+        }
 
         if (r < 0)
                 return r;
@@ -2470,6 +2501,7 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
         STRV_FOREACH(p, files) {
                 _cleanup_fclose_ FILE *f;
                 char line[LINE_MAX];
+                int n = 0;
 
                 f = fopen(*p, "re");
                 if (!f) {
@@ -2480,10 +2512,12 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
                 }
 
                 FOREACH_LINE(line, f, return -errno) {
+                        PresetRule rule = {};
                         const char *parameter;
                         char *l;
 
                         l = strstrip(line);
+                        n++;
 
                         if (isempty(l))
                                 continue;
@@ -2492,31 +2526,87 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
 
                         parameter = first_word(l, "enable");
                         if (parameter) {
-                                if (fnmatch(parameter, name, FNM_NOESCAPE) == 0) {
-                                        log_debug("Preset file says enable %s.", name);
-                                        return 1;
-                                }
+                                char *pattern;
 
-                                continue;
+                                pattern = strdup(parameter);
+                                if (!pattern)
+                                        return -ENOMEM;
+
+                                rule = (PresetRule) {
+                                        .pattern = pattern,
+                                        .action = PRESET_ENABLE,
+                                };
                         }
 
                         parameter = first_word(l, "disable");
                         if (parameter) {
-                                if (fnmatch(parameter, name, FNM_NOESCAPE) == 0) {
-                                        log_debug("Preset file says disable %s.", name);
-                                        return 0;
-                                }
+                                char *pattern;
 
+                                pattern = strdup(parameter);
+                                if (!pattern)
+                                        return -ENOMEM;
+
+                                rule = (PresetRule) {
+                                        .pattern = pattern,
+                                        .action = PRESET_DISABLE,
+                                };
+                        }
+
+                        if (rule.action) {
+                                if (!GREEDY_REALLOC(ps.rules, n_allocated, ps.n_rules + 1))
+                                        return -ENOMEM;
+
+                                ps.rules[ps.n_rules++] = rule;
                                 continue;
                         }
 
-                        log_debug("Couldn't parse line '%s'", l);
+                        log_syntax(NULL, LOG_WARNING, *p, n, 0, "Couldn't parse line '%s'. Ignoring.", line);
                 }
         }
 
-        /* Default is "enable" */
-        log_debug("Preset file doesn't say anything about %s, enabling.", name);
-        return 1;
+        *presets = ps;
+        ps = (Presets){};
+
+        return 0;
+}
+
+static int query_presets(const char *name, const Presets presets) {
+        PresetAction action = PRESET_UNKNOWN;
+        size_t i;
+
+        if (!unit_name_is_valid(name, UNIT_NAME_ANY))
+                return -EINVAL;
+
+        for (i = 0; i < presets.n_rules; i++)
+                if (fnmatch(presets.rules[i].pattern, name, FNM_NOESCAPE) == 0) {
+                        action = presets.rules[i].action;
+                        break;
+                }
+
+        switch (action) {
+        case PRESET_UNKNOWN:
+                log_debug("Preset files don't specify rule for %s. Enabling.", name);
+                return 1;
+        case PRESET_ENABLE:
+                log_debug("Preset files say enable %s.", name);
+                return 1;
+        case PRESET_DISABLE:
+                log_debug("Preset files say disable %s.", name);
+                return 0;
+        default:
+                assert_not_reached("invalid preset action");
+        }
+}
+
+int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name) {
+        _cleanup_(presets_freep) Presets presets = {};
+        int r;
+
+        r = read_presets(scope, root_dir, &presets);
+        if (r < 0)
+                return r;
+
+        return query_presets(name, presets);
 }
 
 static int execute_preset(
@@ -2572,6 +2662,7 @@ static int preset_prepare_one(
                 LookupPaths *paths,
                 UnitFilePresetMode mode,
                 const char *name,
+                Presets presets,
                 UnitFileChange **changes,
                 unsigned *n_changes) {
 
@@ -2582,7 +2673,7 @@ static int preset_prepare_one(
             install_info_find(minus, name))
                 return 0;
 
-        r = unit_file_query_preset(scope, paths->root_dir, name);
+        r = query_presets(name, presets);
         if (r < 0)
                 return r;
 
@@ -2612,6 +2703,7 @@ int unit_file_preset(
 
         _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
         _cleanup_lookup_paths_free_ LookupPaths paths = {};
+        _cleanup_(presets_freep) Presets presets = {};
         const char *config_path;
         char **i;
         int r;
@@ -2626,11 +2718,12 @@ int unit_file_preset(
 
         config_path = runtime ? paths.runtime_config : paths.persistent_config;
 
-        STRV_FOREACH(i, files) {
-                if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
-                        return -EINVAL;
+        r = read_presets(scope, root_dir, &presets);
+        if (r < 0)
+                return r;
 
-                r = preset_prepare_one(scope, &plus, &minus, &paths, mode, *i, changes, n_changes);
+        STRV_FOREACH(i, files) {
+                r = preset_prepare_one(scope, &plus, &minus, &paths, mode, *i, presets, changes, n_changes);
                 if (r < 0)
                         return r;
         }
@@ -2649,6 +2742,7 @@ int unit_file_preset_all(
 
         _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
         _cleanup_lookup_paths_free_ LookupPaths paths = {};
+        _cleanup_(presets_freep) Presets presets = {};
         const char *config_path = NULL;
         char **i;
         int r;
@@ -2663,6 +2757,10 @@ int unit_file_preset_all(
 
         config_path = runtime ? paths.runtime_config : paths.persistent_config;
 
+        r = read_presets(scope, root_dir, &presets);
+        if (r < 0)
+                return r;
+
         STRV_FOREACH(i, paths.search_path) {
                 _cleanup_closedir_ DIR *d = NULL;
                 struct dirent *de;
@@ -2686,7 +2784,7 @@ int unit_file_preset_all(
                                 continue;
 
                         /* we don't pass changes[] in, because we want to handle errors on our own */
-                        r = preset_prepare_one(scope, &plus, &minus, &paths, mode, de->d_name, NULL, 0);
+                        r = preset_prepare_one(scope, &plus, &minus, &paths, mode, de->d_name, presets, NULL, 0);
                         if (r == -ERFKILL)
                                 r = unit_file_changes_add(changes, n_changes,
                                                           UNIT_FILE_IS_MASKED, de->d_name, NULL);
diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index 4680b0336d..4b9a74fca4 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -681,6 +681,53 @@ static void test_revert(const char *root) {
         changes = NULL; n_changes = 0;
 }
 
+static void test_preset_order(const char *root) {
+        UnitFileChange *changes = NULL;
+        unsigned n_changes = 0;
+        const char *p;
+        UnitFileState state;
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-1.service", &state) == -ENOENT);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-2.service", &state) == -ENOENT);
+
+        p = strjoina(root, "/usr/lib/systemd/system/prefix-1.service");
+        assert_se(write_string_file(p,
+                                    "[Install]\n"
+                                    "WantedBy=multi-user.target\n", WRITE_STRING_FILE_CREATE) >= 0);
+
+        p = strjoina(root, "/usr/lib/systemd/system/prefix-2.service");
+        assert_se(write_string_file(p,
+                                    "[Install]\n"
+                                    "WantedBy=multi-user.target\n", WRITE_STRING_FILE_CREATE) >= 0);
+
+        p = strjoina(root, "/usr/lib/systemd/system-preset/test.preset");
+        assert_se(write_string_file(p,
+                                    "enable prefix-1.service\n"
+                                    "disable prefix-*.service\n"
+                                    "enable prefix-2.service\n", WRITE_STRING_FILE_CREATE) >= 0);
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-1.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-2.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+
+        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, false, root, STRV_MAKE("prefix-1.service"), UNIT_FILE_PRESET_FULL, false, &changes, &n_changes) >= 0);
+        assert_se(n_changes == 1);
+        assert_se(changes[0].type == UNIT_FILE_SYMLINK);
+        assert_se(streq(changes[0].source, "/usr/lib/systemd/system/prefix-1.service"));
+        p = strjoina(root, SYSTEM_CONFIG_UNIT_PATH"/multi-user.target.wants/prefix-1.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, "prefix-1.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-2.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+
+        assert_se(unit_file_preset(UNIT_FILE_SYSTEM, false, root, STRV_MAKE("prefix-2.service"), UNIT_FILE_PRESET_FULL, false, &changes, &n_changes) >= 0);
+        assert_se(n_changes == 0);
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-1.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "prefix-2.service", &state) >= 0 && state == UNIT_FILE_DISABLED);
+}
+
 int main(int argc, char *argv[]) {
         char root[] = "/tmp/rootXXXXXX";
         const char *p;
@@ -709,6 +756,7 @@ int main(int argc, char *argv[]) {
         test_template_enable(root);
         test_indirect(root);
         test_preset_and_list(root);
+        test_preset_order(root);
         test_revert(root);
 
         assert_se(rm_rf(root, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0);