From cccf570355c9ffcf34cea5f70e5714765ab002fe Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Fri, 14 Feb 2020 22:43:38 +0100 Subject: [PATCH 1/5] core: Move environment generator path lookup into path-lookup.c --- src/core/manager.c | 26 ++++++-------------------- src/shared/path-lookup.c | 14 ++++++++++++++ src/shared/path-lookup.h | 1 + 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 25afdbea04..38f7ba1eb8 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3826,25 +3826,9 @@ static bool generator_path_any(const char* const* paths) { return found; } -static const char *const system_env_generator_binary_paths[] = { - "/run/systemd/system-environment-generators", - "/etc/systemd/system-environment-generators", - "/usr/local/lib/systemd/system-environment-generators", - SYSTEM_ENV_GENERATOR_PATH, - NULL -}; - -static const char *const user_env_generator_binary_paths[] = { - "/run/systemd/user-environment-generators", - "/etc/systemd/user-environment-generators", - "/usr/local/lib/systemd/user-environment-generators", - USER_ENV_GENERATOR_PATH, - NULL -}; - static int manager_run_environment_generators(Manager *m) { char **tmp = NULL; /* this is only used in the forked process, no cleanup here */ - const char *const *paths; + _cleanup_strv_free_ char **paths = NULL; void* args[] = { [STDOUT_GENERATE] = &tmp, [STDOUT_COLLECT] = &tmp, @@ -3855,13 +3839,15 @@ static int manager_run_environment_generators(Manager *m) { if (MANAGER_IS_TEST_RUN(m) && !(m->test_run_flags & MANAGER_TEST_RUN_ENV_GENERATORS)) return 0; - paths = MANAGER_IS_SYSTEM(m) ? system_env_generator_binary_paths : user_env_generator_binary_paths; + paths = env_generator_binary_paths(MANAGER_IS_SYSTEM(m)); + if (!paths) + return log_oom(); - if (!generator_path_any(paths)) + if (!generator_path_any((const char* const*) paths)) return 0; RUN_WITH_UMASK(0022) - r = execute_directories(paths, DEFAULT_TIMEOUT_USEC, gather_environment, + r = execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, m->transient_environment, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); return r; } diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index 5b16209745..c98d699424 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -837,3 +837,17 @@ char **generator_binary_paths(UnitFileScope scope) { assert_not_reached("Hmm, unexpected scope."); } } + +char **env_generator_binary_paths(bool is_system) { + + if (is_system) + return strv_new("/run/systemd/system-environment-generators", + "/etc/systemd/system-environment-generators", + "/usr/local/lib/systemd/system-environment-generators", + SYSTEM_ENV_GENERATOR_PATH); + else + return strv_new("/run/systemd/user-environment-generators", + "/etc/systemd/user-environment-generators", + "/usr/local/lib/systemd/user-environment-generators", + USER_ENV_GENERATOR_PATH); +} diff --git a/src/shared/path-lookup.h b/src/shared/path-lookup.h index f0762d248a..b99e918144 100644 --- a/src/shared/path-lookup.h +++ b/src/shared/path-lookup.h @@ -72,3 +72,4 @@ void lookup_paths_flush_generator(LookupPaths *p); void lookup_paths_free(LookupPaths *p); char **generator_binary_paths(UnitFileScope scope); +char **env_generator_binary_paths(bool is_system); From 2909f4dd28ddb8d4bf6ec57dae990253031cc53c Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Mon, 17 Feb 2020 11:53:26 +0100 Subject: [PATCH 2/5] path-lookup: Split out helper to get paths from environment This is so that we can use the same pattern for other sets of paths. --- src/shared/path-lookup.c | 50 ++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index c98d699424..0dd6c831ed 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -479,6 +479,36 @@ static int patch_root_prefix_strv(char **l, const char *root_dir) { return 0; } +static int get_paths_from_environ(const char *var, char ***paths, bool *append) { + const char *e; + int r; + + assert(var); + assert(paths); + assert(append); + + *append = false; + + e = getenv(var); + if (e) { + const char *k; + + k = endswith(e, ":"); + if (k) { + e = strndupa(e, k - e); + *append = true; + } + + /* FIXME: empty components in other places should be rejected. */ + + r = path_split_and_make_absolute(e, paths); + if (r < 0) + return r; + } + + return 0; +} + int lookup_paths_init( LookupPaths *p, UnitFileScope scope, @@ -496,7 +526,6 @@ int lookup_paths_init( *persistent_attached = NULL, *runtime_attached = NULL; bool append = false; /* Add items from SYSTEMD_UNIT_PATH before normal directories */ _cleanup_strv_free_ char **paths = NULL; - const char *e; int r; assert(p); @@ -562,22 +591,9 @@ int lookup_paths_init( return r; /* First priority is whatever has been passed to us via env vars */ - e = getenv("SYSTEMD_UNIT_PATH"); - if (e) { - const char *k; - - k = endswith(e, ":"); - if (k) { - e = strndupa(e, k - e); - append = true; - } - - /* FIXME: empty components in other places should be rejected. */ - - r = path_split_and_make_absolute(e, &paths); - if (r < 0) - return r; - } + r = get_paths_from_environ("SYSTEMD_UNIT_PATH", &paths, &append); + if (r < 0) + return r; if (!paths || append) { /* Let's figure something out. */ From 39f7d10c2423db0ada965e4163c2abca217c8b36 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Mon, 17 Feb 2020 12:51:23 +0100 Subject: [PATCH 3/5] path-lookup: Allow setting generator paths via environment This adds SYSTEMD_GENERATOR_PATH and SYSTEMD_ENVIRONMENT_GENERATOR_PATH environment variables that will be read in the same manner as SYSTEMD_UNIT_PATH is. i.e. if set, these paths will be used and a trailing empty entry means that the usual paths will be appended, while no trailing entry means that solely the given paths are used. --- src/shared/path-lookup.c | 101 +++++++++++++++++++++++++++++---------- 1 file changed, 77 insertions(+), 24 deletions(-) diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index 0dd6c831ed..20d6c03a9a 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -833,37 +833,90 @@ void lookup_paths_flush_generator(LookupPaths *p) { } char **generator_binary_paths(UnitFileScope scope) { + bool append = false; /* Add items from SYSTEMD_GENERATOR_PATH before normal directories */ + _cleanup_strv_free_ char **paths = NULL; + int r; - switch (scope) { + /* First priority is whatever has been passed to us via env vars */ + r = get_paths_from_environ("SYSTEMD_GENERATOR_PATH", &paths, &append); + if (r < 0) + return NULL; - case UNIT_FILE_SYSTEM: - return strv_new("/run/systemd/system-generators", - "/etc/systemd/system-generators", - "/usr/local/lib/systemd/system-generators", - SYSTEM_GENERATOR_PATH); + if (!paths || append) { + _cleanup_strv_free_ char **add = NULL; - case UNIT_FILE_GLOBAL: - case UNIT_FILE_USER: - return strv_new("/run/systemd/user-generators", - "/etc/systemd/user-generators", - "/usr/local/lib/systemd/user-generators", - USER_GENERATOR_PATH); + switch (scope) { - default: - assert_not_reached("Hmm, unexpected scope."); + case UNIT_FILE_SYSTEM: + add = strv_new("/run/systemd/system-generators", + "/etc/systemd/system-generators", + "/usr/local/lib/systemd/system-generators", + SYSTEM_GENERATOR_PATH); + break; + + case UNIT_FILE_GLOBAL: + case UNIT_FILE_USER: + add = strv_new("/run/systemd/user-generators", + "/etc/systemd/user-generators", + "/usr/local/lib/systemd/user-generators", + USER_GENERATOR_PATH); + break; + + default: + assert_not_reached("Hmm, unexpected scope."); + } + + if (!add) + return NULL; + + if (paths) { + r = strv_extend_strv(&paths, add, true); + if (r < 0) + return NULL; + } else + /* Small optimization: if paths is NULL (and it usually is), we can simply assign 'add' to it, + * and don't have to copy anything */ + paths = TAKE_PTR(add); } + + return TAKE_PTR(paths); } char **env_generator_binary_paths(bool is_system) { + bool append = false; /* Add items from SYSTEMD_ENVIRONMENT_GENERATOR_PATH before normal directories */ + _cleanup_strv_free_ char **paths = NULL; + _cleanup_strv_free_ char **add = NULL; + int r; - if (is_system) - return strv_new("/run/systemd/system-environment-generators", - "/etc/systemd/system-environment-generators", - "/usr/local/lib/systemd/system-environment-generators", - SYSTEM_ENV_GENERATOR_PATH); - else - return strv_new("/run/systemd/user-environment-generators", - "/etc/systemd/user-environment-generators", - "/usr/local/lib/systemd/user-environment-generators", - USER_ENV_GENERATOR_PATH); + /* First priority is whatever has been passed to us via env vars */ + r = get_paths_from_environ("SYSTEMD_ENVIRONMENT_GENERATOR_PATH", &paths, &append); + if (r < 0) + return NULL; + + if (!paths || append) { + if (is_system) + add = strv_new("/run/systemd/system-environment-generators", + "/etc/systemd/system-environment-generators", + "/usr/local/lib/systemd/system-environment-generators", + SYSTEM_ENV_GENERATOR_PATH); + else + add = strv_new("/run/systemd/user-environment-generators", + "/etc/systemd/user-environment-generators", + "/usr/local/lib/systemd/user-environment-generators", + USER_ENV_GENERATOR_PATH); + + if (!add) + return NULL; + } + + if (paths) { + r = strv_extend_strv(&paths, add, true); + if (r < 0) + return NULL; + } else + /* Small optimization: if paths is NULL (and it usually is), we can simply assign 'add' to it, + * and don't have to copy anything */ + paths = TAKE_PTR(add); + + return TAKE_PTR(paths); } From 96b10a13f2a2bc2c63187944c6e5475031ac16b3 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Mon, 17 Feb 2020 13:30:26 +0100 Subject: [PATCH 4/5] test: Add test for setting generator paths via environment --- src/test/test-path-lookup.c | 53 ++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/src/test/test-path-lookup.c b/src/test/test-path-lookup.c index 62ebc9c923..b9111e9259 100644 --- a/src/test/test-path-lookup.c +++ b/src/test/test-path-lookup.c @@ -67,15 +67,54 @@ static void test_user_and_global_paths(void) { log_info("+ %s", *p); } -static void print_generator_binary_paths(UnitFileScope scope) { - _cleanup_strv_free_ char **paths; +static void test_generator_binary_paths(UnitFileScope scope) { + char template[] = "/tmp/test-path-lookup.XXXXXXX"; + + _cleanup_strv_free_ char **gp_without_env = NULL; + _cleanup_strv_free_ char **env_gp_without_env = NULL; + _cleanup_strv_free_ char **gp_with_env = NULL; + _cleanup_strv_free_ char **env_gp_with_env = NULL; + char *systemd_generator_path = NULL; + char *systemd_env_generator_path = NULL; char **dir; - log_info("Generators dirs (%s):", scope == UNIT_FILE_SYSTEM ? "system" : "user"); + assert_se(mkdtemp(template)); - paths = generator_binary_paths(scope); - STRV_FOREACH(dir, paths) + assert_se(unsetenv("SYSTEMD_GENERATOR_PATH") == 0); + assert_se(unsetenv("SYSTEMD_ENVIRONMENT_GENERATOR_PATH") == 0); + + gp_without_env = generator_binary_paths(scope); + env_gp_without_env = env_generator_binary_paths(scope == UNIT_FILE_SYSTEM ? true : false); + + log_info("Generators dirs (%s):", scope == UNIT_FILE_SYSTEM ? "system" : "user"); + STRV_FOREACH(dir, gp_without_env) log_info(" %s", *dir); + + log_info("Environment generators dirs (%s):", scope == UNIT_FILE_SYSTEM ? "system" : "user"); + STRV_FOREACH(dir, env_gp_without_env) + log_info(" %s", *dir); + + assert_se(!strv_isempty(gp_without_env)); + assert_se(!strv_isempty(env_gp_without_env)); + + systemd_generator_path = strjoina(template, "/systemd-generator-path"); + systemd_env_generator_path = strjoina(template, "/systemd-environment-generator-path"); + assert_se(setenv("SYSTEMD_GENERATOR_PATH", systemd_generator_path, 1) == 0); + assert_se(setenv("SYSTEMD_ENVIRONMENT_GENERATOR_PATH", systemd_env_generator_path, 1) == 0); + + gp_with_env = generator_binary_paths(scope); + env_gp_with_env = env_generator_binary_paths(scope == UNIT_FILE_SYSTEM ? true : false); + + log_info("Generators dirs (%s):", scope == UNIT_FILE_SYSTEM ? "system" : "user"); + STRV_FOREACH(dir, gp_with_env) + log_info(" %s", *dir); + + log_info("Environment generators dirs (%s):", scope == UNIT_FILE_SYSTEM ? "system" : "user"); + STRV_FOREACH(dir, env_gp_with_env) + log_info(" %s", *dir); + + assert_se(strv_equal(gp_with_env, STRV_MAKE(systemd_generator_path))); + assert_se(strv_equal(env_gp_with_env, STRV_MAKE(systemd_env_generator_path))); } int main(int argc, char **argv) { @@ -87,8 +126,8 @@ int main(int argc, char **argv) { test_user_and_global_paths(); - print_generator_binary_paths(UNIT_FILE_SYSTEM); - print_generator_binary_paths(UNIT_FILE_USER); + test_generator_binary_paths(UNIT_FILE_SYSTEM); + test_generator_binary_paths(UNIT_FILE_USER); return EXIT_SUCCESS; } From 5b058473fed3a99d57f143032ce1f9784f4289ce Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Wed, 4 Mar 2020 11:30:31 +0100 Subject: [PATCH 5/5] man: Add documentation for generator path environment variables This adds documentation for the SYSTEMD_GENERATOR_PATH and SYSTEMD_ENVIRONMENT_GENERATOR_PATH variables to the systemd man page grouped with the existing SYSTEMD_UNIT_PATH. Also added is a description about how these variables work, i.e. that a trailing : can be used to prepend paths to the usual set. --- man/systemd.xml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/man/systemd.xml b/man/systemd.xml index 28bf49e131..8b1b6e6c6b 100644 --- a/man/systemd.xml +++ b/man/systemd.xml @@ -649,9 +649,17 @@ $SYSTEMD_UNIT_PATH + $SYSTEMD_GENERATOR_PATH + $SYSTEMD_ENVIRONMENT_GENERATOR_PATH - Controls where systemd looks for unit - files. + Controls where systemd looks for unit files and + generators. + These variables may contain a list of paths, separated by colons + (:). When set, if the list ends with an empty + component (...:), this list is prepended to the + usual set of of paths. Otherwise, the specified list replaces the usual + set of paths. +