From d9daae55d5d7d7cfe4edcc70fb120f28d5ba5dc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 22 Nov 2017 14:13:32 +0100 Subject: [PATCH 01/20] tmpfiles: add a special return code for syntax failures In this way, individual errors in files can be treated differently than a failure of the whole service. A test is added to check that the expected value is returned. Some parts are commented out, because it is not. This will be fixed in a subsequent commit. --- man/systemd-tmpfiles.xml | 10 ++++- meson.build | 5 +++ src/test/meson.build | 4 ++ src/test/test-systemd-tmpfiles.py | 51 ++++++++++++++++++++++++ src/tmpfiles/tmpfiles.c | 66 ++++++++++++++++++++++++------- 5 files changed, 120 insertions(+), 16 deletions(-) create mode 100755 src/test/test-systemd-tmpfiles.py diff --git a/man/systemd-tmpfiles.xml b/man/systemd-tmpfiles.xml index 4dd5b3da6c..596bbfd88d 100644 --- a/man/systemd-tmpfiles.xml +++ b/man/systemd-tmpfiles.xml @@ -133,11 +133,13 @@ marked with r or R are removed. + Also execute lines with an exclamation mark. + Only apply rules with paths that start with @@ -191,8 +193,12 @@ Exit status - On success, 0 is returned, a non-zero failure code - otherwise. + On success, 0 is returned. If the configuration was invalid (invalid syntax, missing + arguments, …), so some lines had to be ignored, but no other errors occurred, + 65 is returned (EX_DATAERR from + /usr/include/sysexits.h). Otherwise, 1 is returned + (EXIT_FAILURE from /usr/include/stdlib.h). + diff --git a/meson.build b/meson.build index 6eff25f45c..6288b5697b 100644 --- a/meson.build +++ b/meson.build @@ -2146,6 +2146,11 @@ if conf.get('ENABLE_TMPFILES') == 1 install : true, install_dir : rootbindir) public_programs += [exe] + + test('test-systemd-tmpfiles', + test_systemd_tmpfiles_py, + args : exe.full_path()) + # https://github.com/mesonbuild/meson/issues/2681 endif if conf.get('ENABLE_HWDB') == 1 diff --git a/src/test/meson.build b/src/test/meson.build index efaa259d76..45d5b8d05e 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -54,6 +54,10 @@ test_dlopen_c = files('test-dlopen.c') ############################################################ +test_systemd_tmpfiles_py = find_program('test-systemd-tmpfiles.py') + +############################################################ + tests += [ [['src/test/test-device-nodes.c'], [], diff --git a/src/test/test-systemd-tmpfiles.py b/src/test/test-systemd-tmpfiles.py new file mode 100755 index 0000000000..0f45bd4f76 --- /dev/null +++ b/src/test/test-systemd-tmpfiles.py @@ -0,0 +1,51 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +import sys +import subprocess + +EX_DATAERR = 65 # from sysexits.h + +exe = sys.argv[1] + +def test_invalid_line(line): + print('Running {} on {!r}'.format(exe, line)) + c = subprocess.run([exe, '--create', '-'], + input=line, stdout=subprocess.PIPE, universal_newlines=True) + assert c.returncode == EX_DATAERR, c + +if __name__ == '__main__': + test_invalid_line('asdfa') + test_invalid_line('f "open quote') + test_invalid_line('f closed quote""') + test_invalid_line('Y /unknown/letter') + test_invalid_line('w non/absolute/path') + test_invalid_line('s') # s is for short + test_invalid_line('f!! /too/many/bangs') + test_invalid_line('f++ /too/many/plusses') + test_invalid_line('f+!+ /too/many/plusses') + test_invalid_line('f!+! /too/many/bangs') + #test_invalid_line('w /unresolved/argument - - - - "%Y"') + #test_invalid_line('w /unresolved/argument/sandwich - - - - "%v%Y%v"') + #test_invalid_line('w /unresolved/filename/%Y - - - - "whatever"') + #test_invalid_line('w /unresolved/filename/sandwich/%v%Y%v - - - - "whatever"') + test_invalid_line('w - - - - - "no file specfied"') + test_invalid_line('C - - - - - "no file specfied"') + test_invalid_line('C non/absolute/path - - - - -') + test_invalid_line('b - - - - - -') + test_invalid_line('b 1234 - - - - -') + test_invalid_line('c - - - - - -') + test_invalid_line('c 1234 - - - - -') + test_invalid_line('t - - -') + test_invalid_line('T - - -') + test_invalid_line('a - - -') + test_invalid_line('A - - -') + test_invalid_line('h - - -') + test_invalid_line('H - - -') diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index c29087b9d0..e0d96d612d 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -1841,7 +1842,7 @@ static int specifier_expansion_from_arg(Item *i) { return 0; } -static int parse_line(const char *fname, unsigned line, const char *buffer) { +static int parse_line(const char *fname, unsigned line, const char *buffer, bool *invalid_config) { _cleanup_free_ char *action = NULL, *mode = NULL, *user = NULL, *group = NULL, *age = NULL, *path = NULL; _cleanup_(item_free_contents) Item i = {}; @@ -1865,9 +1866,14 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { &group, &age, NULL); - if (r < 0) + if (r < 0) { + if (r == -EINVAL) /* invalid quoting and such */ + *invalid_config = true; return log_error_errno(r, "[%s:%u] Failed to parse line: %m", fname, line); + } + else if (r < 2) { + *invalid_config = true; log_error("[%s:%u] Syntax error.", fname, line); return -EIO; } @@ -1879,6 +1885,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { } if (isempty(action)) { + *invalid_config = true; log_error("[%s:%u] Command too short '%s'.", fname, line, action); return -EINVAL; } @@ -1889,6 +1896,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { else if (action[pos] == '+' && !force) force = true; else { + *invalid_config = true; log_error("[%s:%u] Unknown modifiers in command '%s'", fname, line, action); return -EINVAL; @@ -1907,8 +1915,11 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { r = specifier_printf(path, specifier_table, NULL, &i.path); if (r == -ENOKEY) return log_unresolvable_specifier(fname, line); - if (r < 0) + if (r < 0) { + /* ENOMEM is the only return value left after ENOKEY, + * so *invalid_config should not be set. */ return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", fname, line, path); + } switch (i.type) { @@ -1945,6 +1956,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { case WRITE_FILE: if (!i.argument) { + *invalid_config = true; log_error("[%s:%u] Write file requires argument.", fname, line); return -EBADMSG; } @@ -1956,6 +1968,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (!i.argument) return log_oom(); } else if (!path_is_absolute(i.argument)) { + *invalid_config = true; log_error("[%s:%u] Source path is not absolute.", fname, line); return -EBADMSG; } @@ -1968,11 +1981,13 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { unsigned major, minor; if (!i.argument) { + *invalid_config = true; log_error("[%s:%u] Device file requires argument.", fname, line); return -EBADMSG; } if (sscanf(i.argument, "%u:%u", &major, &minor) != 2) { + *invalid_config = true; log_error("[%s:%u] Can't parse device file major/minor '%s'.", fname, line, i.argument); return -EBADMSG; } @@ -1984,6 +1999,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { case SET_XATTR: case RECURSIVE_SET_XATTR: if (!i.argument) { + *invalid_config = true; log_error("[%s:%u] Set extended attribute requires argument.", fname, line); return -EBADMSG; } @@ -1995,6 +2011,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { case SET_ACL: case RECURSIVE_SET_ACL: if (!i.argument) { + *invalid_config = true; log_error("[%s:%u] Set ACLs requires argument.", fname, line); return -EBADMSG; } @@ -2006,21 +2023,26 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { case SET_ATTRIBUTE: case RECURSIVE_SET_ATTRIBUTE: if (!i.argument) { + *invalid_config = true; log_error("[%s:%u] Set file attribute requires argument.", fname, line); return -EBADMSG; } r = parse_attribute_from_arg(&i); + if (r == -EINVAL) + *invalid_config = true; if (r < 0) return r; break; default: log_error("[%s:%u] Unknown command type '%c'.", fname, line, (char) i.type); + *invalid_config = true; return -EBADMSG; } if (!path_is_absolute(i.path)) { log_error("[%s:%u] Path '%s' not absolute.", fname, line, i.path); + *invalid_config = true; return -EBADMSG; } @@ -2052,8 +2074,8 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { r = get_user_creds(&u, &i.uid, NULL, NULL, NULL); if (r < 0) { - log_error("[%s:%u] Unknown user '%s'.", fname, line, user); - return r; + *invalid_config = true; + return log_error_errno(r, "[%s:%u] Unknown user '%s'.", fname, line, user); } i.uid_set = true; @@ -2064,6 +2086,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { r = get_group_creds(&g, &i.gid); if (r < 0) { + *invalid_config = true; log_error("[%s:%u] Unknown group '%s'.", fname, line, group); return r; } @@ -2081,6 +2104,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { } if (parse_mode(mm, &m) < 0) { + *invalid_config = true; log_error("[%s:%u] Invalid mode '%s'.", fname, line, mode); return -EBADMSG; } @@ -2099,6 +2123,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { } if (parse_sec(a, &i.age) < 0) { + *invalid_config = true; log_error("[%s:%u] Invalid age '%s'.", fname, line, age); return -EBADMSG; } @@ -2149,8 +2174,8 @@ static void help(void) { " --boot Execute actions only safe at boot\n" " --prefix=PATH Only apply rules with the specified prefix\n" " --exclude-prefix=PATH Ignore rules with the specified prefix\n" - " --root=PATH Operate on an alternate filesystem root\n", - program_invocation_short_name); + " --root=PATH Operate on an alternate filesystem root\n" + , program_invocation_short_name); } static int parse_argv(int argc, char *argv[]) { @@ -2242,7 +2267,7 @@ static int parse_argv(int argc, char *argv[]) { return 1; } -static int read_config_file(const char *fn, bool ignore_enoent) { +static int read_config_file(const char *fn, bool ignore_enoent, bool *invalid_config) { _cleanup_fclose_ FILE *_f = NULL; FILE *f; char line[LINE_MAX]; @@ -2274,6 +2299,7 @@ static int read_config_file(const char *fn, bool ignore_enoent) { FOREACH_LINE(line, f, break) { char *l; int k; + bool invalid_line = false; v++; @@ -2281,9 +2307,15 @@ static int read_config_file(const char *fn, bool ignore_enoent) { if (IN_SET(*l, 0, '#')) continue; - k = parse_line(fn, v, l); - if (k < 0 && r == 0) - r = k; + k = parse_line(fn, v, l, &invalid_line); + if (k < 0) { + if (invalid_line) + /* Allow reporting with a special code if the caller requested this */ + *invalid_config = true; + else if (r == 0) + /* The first error becomes our return value */ + r = k; + } } /* we have to determine age parameter for each entry of type X */ @@ -2327,6 +2359,7 @@ int main(int argc, char *argv[]) { int r, k; ItemArray *a; Iterator iterator; + bool invalid_config = false; r = parse_argv(argc, argv); if (r <= 0) @@ -2354,7 +2387,7 @@ int main(int argc, char *argv[]) { int j; for (j = optind; j < argc; j++) { - k = read_config_file(argv[j], false); + k = read_config_file(argv[j], false, &invalid_config); if (k < 0 && r == 0) r = k; } @@ -2370,7 +2403,7 @@ int main(int argc, char *argv[]) { } STRV_FOREACH(f, files) { - k = read_config_file(*f, true); + k = read_config_file(*f, true, &invalid_config); if (k < 0 && r == 0) r = k; } @@ -2404,5 +2437,10 @@ finish: mac_selinux_finish(); - return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; + if (r < 0) + return EXIT_FAILURE; + else if (invalid_config) + return EX_DATAERR; + else + return EXIT_SUCCESS; } From cd9f5b68ce08375eb1d68a4ddaa7a24a5092d7ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 22 Nov 2017 14:18:25 +0100 Subject: [PATCH 02/20] units: use SuccessExitStatus to ignore syntax errors in tmpfiles This makes sense from the point of view of the whole distribution: if there are some specific files that have syntax problems, or unknown users or groups, or use unsupported features, failing the whole service is not useful. In particular, services with tmpfiles --boot should not be started after boot. The premise of --boot is that there are actions which are only safe to do once during boot, because the state evolves later through other means and re-running the boot-time setup would destroy it. If services with --boot fail in the initial transaction, they would be re-run later on when a unit which (indirectly) depends on them is started, causing problems. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1507501. (If we had a mode where a service would at most run once, and would not be started in subsequent transactions, that'd be a good additional safeguard. Using ExecStart=-... is a bit like that, but it causes all failure to be ignored, which is too big of a hammer.) --- units/systemd-tmpfiles-clean.service.in | 1 + units/systemd-tmpfiles-setup-dev.service.in | 1 + units/systemd-tmpfiles-setup.service.in | 1 + 3 files changed, 3 insertions(+) diff --git a/units/systemd-tmpfiles-clean.service.in b/units/systemd-tmpfiles-clean.service.in index 5963cf27fb..d1025afadb 100644 --- a/units/systemd-tmpfiles-clean.service.in +++ b/units/systemd-tmpfiles-clean.service.in @@ -18,4 +18,5 @@ Before=shutdown.target [Service] Type=oneshot ExecStart=@rootbindir@/systemd-tmpfiles --clean +SuccessExitStatus=65 IOSchedulingClass=idle diff --git a/units/systemd-tmpfiles-setup-dev.service.in b/units/systemd-tmpfiles-setup-dev.service.in index 29b4f0bec0..6a6ebed955 100644 --- a/units/systemd-tmpfiles-setup-dev.service.in +++ b/units/systemd-tmpfiles-setup-dev.service.in @@ -20,3 +20,4 @@ ConditionCapability=CAP_SYS_MODULE Type=oneshot RemainAfterExit=yes ExecStart=@rootbindir@/systemd-tmpfiles --prefix=/dev --create --boot +SuccessExitStatus=65 diff --git a/units/systemd-tmpfiles-setup.service.in b/units/systemd-tmpfiles-setup.service.in index ba24b8d823..0410d0bfd8 100644 --- a/units/systemd-tmpfiles-setup.service.in +++ b/units/systemd-tmpfiles-setup.service.in @@ -20,3 +20,4 @@ RefuseManualStop=yes Type=oneshot RemainAfterExit=yes ExecStart=@rootbindir@/systemd-tmpfiles --create --remove --boot --exclude-prefix=/dev +SuccessExitStatus=65 From 2df36d096d7b2691ac11f415b3bb8da2a26ac8b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 24 Nov 2017 12:05:03 +0100 Subject: [PATCH 03/20] man: specifiers are allow for argument field in tmpfiles --- man/tmpfiles.d.xml | 99 +++++++++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 46 deletions(-) diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 793c124007..d983984fd6 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -482,51 +482,8 @@ r! /tmp/.X[0-9]*-lock Path The file system path specification supports simple - specifier expansion. The following expansions are - understood: - - - Specifiers available - - - - - - - Specifier - Meaning - Details - - - - - %m - Machine ID - The machine ID of the running system, formatted as string. See machine-id5 for more information. - - - %b - Boot ID - The boot ID of the running system, formatted as string. See random4 for more information. - - - %H - Host name - The hostname of the running system. - - - %v - Kernel release - Identical to uname -r output. - - - %% - Escaped % - Single percent sign. - - - -
+ specifier expansion, see below. The path (after expansion) must be + absolute. @@ -628,8 +585,58 @@ r! /tmp/.X[0-9]*-lock attributes to be set. For h and H, determines the file attributes to set. Ignored for all other lines. - + This field can contain specifiers, see below. + + + + + Specifiers + + Specifiers can be used in the "path" and "argument" fields. + The following expansions are understood: + + Specifiers available + + + + + + + Specifier + Meaning + Details + + + + + %m + Machine ID + The machine ID of the running system, formatted as string. See machine-id5 for more information. + + + %b + Boot ID + The boot ID of the running system, formatted as string. See random4 for more information. + + + %H + Host name + The hostname of the running system. + + + %v + Kernel release + Identical to uname -r output. + + + %% + Escaped % + Single percent sign. + + + +
From 751223fecf56c6865080c2dda760a47a9958e49c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 24 Nov 2017 12:19:40 +0100 Subject: [PATCH 04/20] Fail on unknown (alphanumerical) specifiers The code intentionally ignored unknown specifiers, treating them as text. This needs to change because otherwise we can never add a new specifier in a backwards compatible way. So just treat an unknown (potential) specifier as an error. In principle this is a break of backwards compatibility, but the previous behaviour was pretty much useless, since the expanded value could change every time we add new specifiers, which we do all the time. As a compromise for backwards compatibility, only fail on alphanumerical characters. This should cover the most cases where an unescaped percent character is used, like size=5% and such, which behave the same as before with this patch. OTOH, this means that we will not be able to use non-alphanumerical specifiers without breaking backwards compatibility again. I think that's an acceptable compromise. v2: - add NEWS entry v3: - only fail on alphanumerical --- NEWS | 7 +++++++ man/systemd.unit.xml | 3 ++- man/tmpfiles.d.xml | 1 + src/shared/specifier.c | 9 ++++++++- src/test/test-systemd-tmpfiles.py | 8 ++++---- src/tmpfiles/tmpfiles.c | 14 +++++++++----- 6 files changed, 31 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index 77b38f5d6c..de3d28d668 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,13 @@ CHANGES WITH 236 in spe: numdummies=0, preventing the kernel from automatically creating dummy0. All dummy interfaces must now be explicitly created. + * Unknown specifiers are now rejected. This applies to units and + tmpfiles.d configuration. Any percent characters that are followed by + a letter or digit that are not supposed to be interpreted as the + beginning of a specifier should be escaped by doubling ("%%"). + (So "size=5%" is still accepted, as well as "size=5%,foo=bar", but + not "LABEL=x%y%z" since %y and %z are not valid specifiers today.) + * systemd-resolved now maintains a new dynamic /run/systemd/resolve/stub-resolv.conf compatibility file. It is recommended to make /etc/resolv.conf a symlink to it. This file diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 9c40562e08..06a7bbd3ba 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1297,7 +1297,8 @@ Many settings resolve specifiers which may be used to write generic unit files referring to runtime or unit parameters that - are replaced when the unit files are loaded. The following + are replaced when the unit files are loaded. Specifiers must be known + and resolvable for the setting to be valid. The following specifiers are understood: diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index d983984fd6..642a124eac 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -594,6 +594,7 @@ r! /tmp/.X[0-9]*-lockSpecifiersSpecifiers can be used in the "path" and "argument" fields. + An unknown or unresolvable specifier is treated as invalid configuration. The following expansions are understood:
Specifiers available diff --git a/src/shared/specifier.c b/src/shared/specifier.c index 9bef890346..9034cc20d7 100644 --- a/src/shared/specifier.c +++ b/src/shared/specifier.c @@ -41,6 +41,10 @@ * */ +/* Any ASCII character or digit: our pool of potential specifiers, + * and "%" used for escaping. */ +#define POSSIBLE_SPECIFIERS ALPHANUMERICAL "%" + int specifier_printf(const char *text, const Specifier table[], void *userdata, char **_ret) { char *ret, *t; const char *f; @@ -97,7 +101,10 @@ int specifier_printf(const char *text, const Specifier table[], void *userdata, ret = n; t = n + j + k; - } else { + } else if (strchr(POSSIBLE_SPECIFIERS, *f)) + /* Oops, an unknown specifier. */ + return -EBADSLT; + else { *(t++) = '%'; *(t++) = *f; } diff --git a/src/test/test-systemd-tmpfiles.py b/src/test/test-systemd-tmpfiles.py index 0f45bd4f76..75d8fd3657 100755 --- a/src/test/test-systemd-tmpfiles.py +++ b/src/test/test-systemd-tmpfiles.py @@ -32,10 +32,10 @@ if __name__ == '__main__': test_invalid_line('f++ /too/many/plusses') test_invalid_line('f+!+ /too/many/plusses') test_invalid_line('f!+! /too/many/bangs') - #test_invalid_line('w /unresolved/argument - - - - "%Y"') - #test_invalid_line('w /unresolved/argument/sandwich - - - - "%v%Y%v"') - #test_invalid_line('w /unresolved/filename/%Y - - - - "whatever"') - #test_invalid_line('w /unresolved/filename/sandwich/%v%Y%v - - - - "whatever"') + test_invalid_line('w /unresolved/argument - - - - "%Y"') + test_invalid_line('w /unresolved/argument/sandwich - - - - "%v%Y%v"') + test_invalid_line('w /unresolved/filename/%Y - - - - "whatever"') + test_invalid_line('w /unresolved/filename/sandwich/%v%Y%v - - - - "whatever"') test_invalid_line('w - - - - - "no file specfied"') test_invalid_line('C - - - - - "no file specfied"') test_invalid_line('C non/absolute/path - - - - -') diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index e0d96d612d..4e615288da 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1867,7 +1867,8 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool &age, NULL); if (r < 0) { - if (r == -EINVAL) /* invalid quoting and such */ + if (IN_SET(r, -EINVAL, -EBADSLT)) + /* invalid quoting and such or an unknown specifier */ *invalid_config = true; return log_error_errno(r, "[%s:%u] Failed to parse line: %m", fname, line); } @@ -1916,8 +1917,8 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool if (r == -ENOKEY) return log_unresolvable_specifier(fname, line); if (r < 0) { - /* ENOMEM is the only return value left after ENOKEY, - * so *invalid_config should not be set. */ + if (IN_SET(r, -EINVAL, -EBADSLT)) + *invalid_config = true; return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", fname, line, path); } @@ -2028,7 +2029,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool return -EBADMSG; } r = parse_attribute_from_arg(&i); - if (r == -EINVAL) + if (IN_SET(r, -EINVAL, -EBADSLT)) *invalid_config = true; if (r < 0) return r; @@ -2054,9 +2055,12 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool r = specifier_expansion_from_arg(&i); if (r == -ENOKEY) return log_unresolvable_specifier(fname, line); - if (r < 0) + if (r < 0) { + if (IN_SET(r, -EINVAL, -EBADSLT)) + *invalid_config = true; return log_error_errno(r, "[%s:%u] Failed to substitute specifiers in argument: %m", fname, line); + } if (arg_root) { char *p; From e286dbaf9bce7445a03acec9e0d814ee613cc39a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 22 Nov 2017 14:19:13 +0100 Subject: [PATCH 05/20] tmpfiles: downgrade warning about duplicate line This happens occasionally, especially when moving lines between configuration files in different packages, and usually is not a big deal. --- src/tmpfiles/tmpfiles.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 4e615288da..7061b2fefb 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -2143,8 +2143,8 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool for (n = 0; n < existing->count; n++) { if (!item_compatible(existing->items + n, &i)) { - log_warning("[%s:%u] Duplicate line for path \"%s\", ignoring.", - fname, line, i.path); + log_notice("[%s:%u] Duplicate line for path \"%s\", ignoring.", + fname, line, i.path); return 0; } } From 79a1b18711d927c68cce4424d1731f73cf001611 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 22 Nov 2017 14:55:14 +0100 Subject: [PATCH 06/20] tmpfiles: fix typo in error message Fixes #4097. As of current master, systemd-tmpfiles behaves correctly, apart from a trivial typo. So let's tell github to close the bug. With current git: $ sudo SYSTEMD_LOG_LEVEL=debug build/systemd-tmpfiles --create `pwd`/test/tmpfiles.d/link-loop.conf Successfully loaded SELinux database in 2.385ms, size on heap is 321K. Reading config file "/home/zbyszek/src/systemd-work/test/tmpfiles.d/link-loop.conf". Running create action for entry D /run/hello2 Found existing directory "/run/hello2". "/run/hello2" has right mode 41777 Running create action for entry f /run/hello2/hello2.test "/run/hello2/hello2.test" has been created. "/run/hello2/hello2.test" has right mode 101777 chown "/run/hello2/hello2.test" to 0.84 Running create action for entry L /run/hello2/hello2.link Found existing symlink "/run/hello2/hello2.link". Running create action for entry z /run/hello2/hello2.test "/run/hello2/hello2.test" has right mode 101777 chown "/run/hello2/hello2.test" to 0.0 Running create action for entry z /run/hello2/hello2.link Skipping mode an owner fix for symlink /run/hello2/hello2.link. and the permissions are: $ ls -dl /run/hello2/ /run/hello2/* drwxrwxrwt. 2 foo bar 80 Nov 22 14:40 /run/hello2/ lrwxrwxrwx. 1 root root 23 Nov 22 14:40 /run/hello2/hello2.link -> /run/hello2/hello2.test -rwxrwxrwt. 1 root root 0 Nov 22 14:40 /run/hello2/hello2.test Everything seems correct. --- src/tmpfiles/tmpfiles.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 7061b2fefb..2b568daa23 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -671,7 +671,7 @@ static int path_set_perms(Item *i, const char *path) { return log_error_errno(errno, "Failed to fstat() file %s: %m", path); if (S_ISLNK(st.st_mode)) - log_debug("Skipping mode an owner fix for symlink %s.", path); + log_debug("Skipping mode and owner fix for symlink %s.", path); else { char fn[strlen("/proc/self/fd/") + DECIMAL_STR_MAX(int)]; xsprintf(fn, "/proc/self/fd/%i", fd); From 65241c1485dbad934e22544cd9d8724f4d7ff91d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 22 Nov 2017 15:16:48 +0100 Subject: [PATCH 07/20] tmpfiles: "e" takes globs Fixes #7369. --- src/tmpfiles/tmpfiles.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 2b568daa23..c49c208078 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1632,12 +1632,12 @@ static int clean_item(Item *i) { case CREATE_SUBVOLUME: case CREATE_SUBVOLUME_INHERIT_QUOTA: case CREATE_SUBVOLUME_NEW_QUOTA: - case EMPTY_DIRECTORY: case TRUNCATE_DIRECTORY: case IGNORE_PATH: case COPY_FILES: clean_item_instance(i, i->path); return 0; + case EMPTY_DIRECTORY: case IGNORE_DIRECTORY_PATH: return glob_item(i, clean_item_instance, false); default: From ca4adeb7913712f3f676d9e1e5b5131ab9d6664e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 23 Nov 2017 11:41:28 +0100 Subject: [PATCH 08/20] shared: export xdg_user_dirs() and xdg_user_*_dir() --- src/shared/path-lookup.c | 82 +++++++++++++++++++++++----------------- src/shared/path-lookup.h | 4 ++ 2 files changed, 51 insertions(+), 35 deletions(-) diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index 57e0757529..d57c78a8b1 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -39,7 +39,7 @@ #include "user-util.h" #include "util.h" -static int user_runtime_dir(char **ret, const char *suffix) { +int xdg_user_runtime_dir(char **ret, const char *suffix) { const char *e; char *j; @@ -58,7 +58,7 @@ static int user_runtime_dir(char **ret, const char *suffix) { return 0; } -static int user_config_dir(char **ret, const char *suffix) { +int xdg_user_config_dir(char **ret, const char *suffix) { const char *e; char *j; int r; @@ -85,7 +85,7 @@ static int user_config_dir(char **ret, const char *suffix) { return 0; } -static int user_data_dir(char **ret, const char *suffix) { +int xdg_user_data_dir(char **ret, const char *suffix) { const char *e; char *j; int r; @@ -131,6 +131,41 @@ static const char* const user_config_unit_paths[] = { NULL }; +int xdg_user_dirs(char ***ret_config_dirs, char ***ret_data_dirs) { + /* Implement the mechanisms defined in + * + * http://standards.freedesktop.org/basedir-spec/basedir-spec-0.6.html + * + * We look in both the config and the data dirs because we + * want to encourage that distributors ship their unit files + * as data, and allow overriding as configuration. + */ + const char *e; + _cleanup_strv_free_ char **config_dirs = NULL, **data_dirs = NULL; + + e = getenv("XDG_CONFIG_DIRS"); + if (e) { + config_dirs = strv_split(e, ":"); + if (!config_dirs) + return -ENOMEM; + } + + e = getenv("XDG_DATA_DIRS"); + if (e) + data_dirs = strv_split(e, ":"); + else + data_dirs = strv_new("/usr/local/share", + "/usr/share", + NULL); + if (!data_dirs) + return -ENOMEM; + + *ret_config_dirs = config_dirs; + *ret_data_dirs = data_dirs; + config_dirs = data_dirs = NULL; + return 0; +} + static char** user_dirs( const char *persistent_config, const char *runtime_config, @@ -144,38 +179,15 @@ static char** user_dirs( _cleanup_strv_free_ char **config_dirs = NULL, **data_dirs = NULL; _cleanup_free_ char *data_home = NULL; _cleanup_strv_free_ char **res = NULL; - const char *e; char **tmp; int r; - /* Implement the mechanisms defined in - * - * http://standards.freedesktop.org/basedir-spec/basedir-spec-0.6.html - * - * We look in both the config and the data dirs because we - * want to encourage that distributors ship their unit files - * as data, and allow overriding as configuration. - */ - - e = getenv("XDG_CONFIG_DIRS"); - if (e) { - config_dirs = strv_split(e, ":"); - if (!config_dirs) - return NULL; - } - - r = user_data_dir(&data_home, "/systemd/user"); - if (r < 0 && r != -ENXIO) + r = xdg_user_dirs(&config_dirs, &data_dirs); + if (r < 0) return NULL; - e = getenv("XDG_DATA_DIRS"); - if (e) - data_dirs = strv_split(e, ":"); - else - data_dirs = strv_new("/usr/local/share", - "/usr/share", - NULL); - if (!data_dirs) + r = xdg_user_data_dir(&data_home, "/systemd/user"); + if (r < 0 && r != -ENXIO) return NULL; /* Now merge everything we found. */ @@ -311,7 +323,7 @@ static int acquire_transient_dir( else if (scope == UNIT_FILE_SYSTEM) transient = strdup("/run/systemd/transient"); else - return user_runtime_dir(ret, "/systemd/transient"); + return xdg_user_runtime_dir(ret, "/systemd/transient"); if (!transient) return -ENOMEM; @@ -339,11 +351,11 @@ static int acquire_config_dirs(UnitFileScope scope, char **persistent, char **ru break; case UNIT_FILE_USER: - r = user_config_dir(&a, "/systemd/user"); + r = xdg_user_config_dir(&a, "/systemd/user"); if (r < 0 && r != -ENXIO) return r; - r = user_runtime_dir(runtime, "/systemd/user"); + r = xdg_user_runtime_dir(runtime, "/systemd/user"); if (r < 0) { if (r != -ENXIO) return r; @@ -399,11 +411,11 @@ static int acquire_control_dirs(UnitFileScope scope, char **persistent, char **r } case UNIT_FILE_USER: - r = user_config_dir(&a, "/systemd/system.control"); + r = xdg_user_config_dir(&a, "/systemd/system.control"); if (r < 0 && r != -ENXIO) return r; - r = user_runtime_dir(runtime, "/systemd/system.control"); + r = xdg_user_runtime_dir(runtime, "/systemd/system.control"); if (r < 0) { if (r != -ENXIO) return r; diff --git a/src/shared/path-lookup.h b/src/shared/path-lookup.h index bcf9ca4de6..42a870aa3e 100644 --- a/src/shared/path-lookup.h +++ b/src/shared/path-lookup.h @@ -68,6 +68,10 @@ struct LookupPaths { }; int lookup_paths_init(LookupPaths *p, UnitFileScope scope, LookupPathsFlags flags, const char *root_dir); +int xdg_user_dirs(char ***ret_config_dirs, char ***ret_data_dirs); +int xdg_user_runtime_dir(char **ret, const char *suffix); +int xdg_user_config_dir(char **ret, const char *suffix); +int xdg_user_data_dir(char **ret, const char *suffix); bool path_is_user_data_dir(const char *path); bool path_is_user_config_dir(const char *path); From 32a8f700a4acd601c6bac89e871d2e9e145f4adf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 23 Nov 2017 13:02:21 +0100 Subject: [PATCH 09/20] util-lib: kill duplicate slashes in lookup paths Since we're munging the array anyway, we can make the output a bit nicer too. --- src/basic/path-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 3bde1d1e01..efeca13594 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -223,8 +223,8 @@ int path_strv_make_absolute_cwd(char **l) { if (r < 0) return r; - free(*s); - *s = t; + path_kill_slashes(t); + free_and_replace(*s, t); } return 0; From e783f957181324110d2ee49377c48acc90d735a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 23 Nov 2017 13:23:42 +0100 Subject: [PATCH 10/20] Rename "system-preset" source dir to "presets" I want to add presets/user/ later. This mirrors the layout for units: we have units/ and units/user. The advantage is that we avoid having yet another directory at the top level. --- meson.build | 3 +-- {system-preset => presets}/90-systemd.preset | 0 presets/meson.build | 19 +++++++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) rename {system-preset => presets}/90-systemd.preset (100%) create mode 100644 presets/meson.build diff --git a/meson.build b/meson.build index 6288b5697b..4f63bffe95 100644 --- a/meson.build +++ b/meson.build @@ -2383,6 +2383,7 @@ subdir('units') subdir('sysctl.d') subdir('sysusers.d') subdir('tmpfiles.d') +subdir('presets') subdir('hwdb') subdir('network') subdir('man') @@ -2399,8 +2400,6 @@ install_subdir('factory/etc', install_data('xorg/50-systemd-user.sh', install_dir : xinitrcdir) -install_data('system-preset/90-systemd.preset', - install_dir : systempresetdir) install_data('modprobe.d/systemd.conf', install_dir : modprobedir) install_data('README', diff --git a/system-preset/90-systemd.preset b/presets/90-systemd.preset similarity index 100% rename from system-preset/90-systemd.preset rename to presets/90-systemd.preset diff --git a/presets/meson.build b/presets/meson.build new file mode 100644 index 0000000000..89bed8a05a --- /dev/null +++ b/presets/meson.build @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# Copyright 2017 Zbigniew Jędrzejewski-Szmek +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. +# +# systemd is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with systemd; If not, see . + +install_data('90-systemd.preset', + install_dir : systempresetdir) From ca23eeb54c7e164cd16a3a0a631b7f1b73584d89 Mon Sep 17 00:00:00 2001 From: ayekat Date: Fri, 24 Nov 2017 12:44:08 +0100 Subject: [PATCH 11/20] tmpfiles: Add specifiers to allow running as user instance This commit adds specifiers %U, %u and %h for the user UID, name and home directory, respectively. [zj: drop untrue copy-pasted comments and move the next text to the new "Specifiers" section. Now that #7444 has been merged, also drop the specifier functions.] --- man/tmpfiles.d.xml | 15 +++++++++++++++ src/tmpfiles/tmpfiles.c | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 642a124eac..5278c7e7cc 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -630,6 +630,21 @@ r! /tmp/.X[0-9]*-lock Kernel release Identical to uname -r output. + + %U + User UID + This is the numeric UID of the user running the service manager instance. In case of the system manager this resolves to 0. + + + %u + User name + This is the name of the user running the service manager instance. In case of the system manager this resolves to root. + + + %h + User home directory + This is the home directory of the user running the service manager instance. In case of the system manager this resolves to /root. + %% Escaped % diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index c49c208078..4a9c55462f 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -174,6 +174,10 @@ static const Specifier specifier_table[] = { { 'b', specifier_boot_id, NULL }, { 'H', specifier_host_name, NULL }, { 'v', specifier_kernel_release, NULL }, + + { 'U', specifier_user_id, NULL }, + { 'u', specifier_user_name, NULL }, + { 'h', specifier_user_home, NULL }, {} }; From f2b5ca0e4ea835f0d4d1960123bdfdc1430d940d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 23 Nov 2017 11:20:29 +0100 Subject: [PATCH 12/20] tmpfiles: add --user switch --- man/systemd-tmpfiles.xml | 8 +++- man/tmpfiles.d.xml | 14 ++++-- src/tmpfiles/tmpfiles.c | 97 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 107 insertions(+), 12 deletions(-) diff --git a/man/systemd-tmpfiles.xml b/man/systemd-tmpfiles.xml index 596bbfd88d..5c9660df64 100644 --- a/man/systemd-tmpfiles.xml +++ b/man/systemd-tmpfiles.xml @@ -115,7 +115,7 @@ T, a, and A have their ownership, access mode and - security labels set. + security labels set. @@ -134,6 +134,12 @@ removed. + + + Execute "user" configuration, i.e. tmpfiles.d + files in user configuration directories. + + Also execute lines with an exclamation mark. diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 5278c7e7cc..d89cb08f53 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -48,9 +48,17 @@ - /etc/tmpfiles.d/*.conf - /run/tmpfiles.d/*.conf - /usr/lib/tmpfiles.d/*.conf + /etc/tmpfiles.d/*.conf +/run/tmpfiles.d/*.conf +/usr/lib/tmpfiles.d/*.conf + + + ~/.config/user-tmpfiles.d/*.conf +$XDG_RUNTIME_DIR/user-tmpfiles.d/*.conf +~/.local/share/user-tmpfiles.d/*.conf +… +/usr/share/user-tmpfiles.d/*.conf + diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 4a9c55462f..8d57239177 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -60,6 +60,7 @@ #include "mkdir.h" #include "mount-util.h" #include "parse-util.h" +#include "path-lookup.h" #include "path-util.h" #include "rm-rf.h" #include "selinux-util.h" @@ -151,6 +152,7 @@ typedef struct ItemArray { size_t size; } ItemArray; +static bool arg_user = false; static bool arg_create = false; static bool arg_clean = false; static bool arg_remove = false; @@ -160,8 +162,6 @@ static char **arg_include_prefixes = NULL; static char **arg_exclude_prefixes = NULL; static char *arg_root = NULL; -static const char conf_file_dirs[] = CONF_PATHS_NULSTR("tmpfiles.d"); - #define MAX_DEPTH 256 static OrderedHashmap *items = NULL, *globs = NULL; @@ -214,6 +214,57 @@ static int log_unresolvable_specifier(const char *filename, unsigned line) { return 0; } +static int user_config_paths(char*** ret) { + _cleanup_strv_free_ char **config_dirs = NULL, **data_dirs = NULL; + _cleanup_free_ char *persistent_config = NULL, *runtime_config = NULL, *data_home = NULL; + _cleanup_strv_free_ char **res = NULL; + int r; + + r = xdg_user_dirs(&config_dirs, &data_dirs); + if (r < 0) + return r; + + r = xdg_user_config_dir(&persistent_config, "/user-tmpfiles.d"); + if (r < 0 && r != -ENXIO) + return r; + + r = xdg_user_runtime_dir(&runtime_config, "/user-tmpfiles.d"); + if (r < 0 && r != -ENXIO) + return r; + + r = xdg_user_data_dir(&data_home, "/user-tmpfiles.d"); + if (r < 0 && r != -ENXIO) + return r; + + r = strv_extend_strv_concat(&res, config_dirs, "/user-tmpfiles.d"); + if (r < 0) + return r; + + r = strv_extend(&res, persistent_config); + if (r < 0) + return r; + + r = strv_extend(&res, runtime_config); + if (r < 0) + return r; + + r = strv_extend(&res, data_home); + if (r < 0) + return r; + + r = strv_extend_strv_concat(&res, data_dirs, "/user-tmpfiles.d"); + if (r < 0) + return r; + + r = path_strv_make_absolute_cwd(res); + if (r < 0) + return r; + + *ret = res; + res = NULL; + return 0; +} + static bool needs_glob(ItemType t) { return IN_SET(t, WRITE_FILE, @@ -2175,6 +2226,7 @@ static void help(void) { printf("%s [OPTIONS...] [CONFIGURATION FILE...]\n\n" "Creates, deletes and cleans up volatile and temporary files and directories.\n\n" " -h --help Show this help\n" + " --user Execute user configuration\n" " --version Show package version\n" " --create Create marked files/directories\n" " --clean Clean up marked directories\n" @@ -2190,6 +2242,7 @@ static int parse_argv(int argc, char *argv[]) { enum { ARG_VERSION = 0x100, + ARG_USER, ARG_CREATE, ARG_CLEAN, ARG_REMOVE, @@ -2201,6 +2254,7 @@ static int parse_argv(int argc, char *argv[]) { static const struct option options[] = { { "help", no_argument, NULL, 'h' }, + { "user", no_argument, NULL, ARG_USER }, { "version", no_argument, NULL, ARG_VERSION }, { "create", no_argument, NULL, ARG_CREATE }, { "clean", no_argument, NULL, ARG_CLEAN }, @@ -2228,6 +2282,10 @@ static int parse_argv(int argc, char *argv[]) { case ARG_VERSION: return version(); + case ARG_USER: + arg_user = true; + break; + case ARG_CREATE: arg_create = true; break; @@ -2275,7 +2333,7 @@ static int parse_argv(int argc, char *argv[]) { return 1; } -static int read_config_file(const char *fn, bool ignore_enoent, bool *invalid_config) { +static int read_config_file(const char **config_dirs, const char *fn, bool ignore_enoent, bool *invalid_config) { _cleanup_fclose_ FILE *_f = NULL; FILE *f; char line[LINE_MAX]; @@ -2291,7 +2349,7 @@ static int read_config_file(const char *fn, bool ignore_enoent, bool *invalid_co fn = ""; f = stdin; } else { - r = search_and_fopen_nulstr(fn, "re", arg_root, conf_file_dirs, &_f); + r = search_and_fopen(fn, "re", arg_root, config_dirs, &_f); if (r < 0) { if (ignore_enoent && r == -ENOENT) { log_debug_errno(r, "Failed to open \"%s\", ignoring: %m", fn); @@ -2367,7 +2425,9 @@ int main(int argc, char *argv[]) { int r, k; ItemArray *a; Iterator iterator; + _cleanup_strv_free_ char **config_dirs = NULL; bool invalid_config = false; + char **f; r = parse_argv(argc, argv); if (r <= 0) @@ -2391,27 +2451,48 @@ int main(int argc, char *argv[]) { r = 0; + if (arg_user) { + r = user_config_paths(&config_dirs); + if (r < 0) { + log_error_errno(r, "Failed to initialize configuration directory list: %m"); + goto finish; + } + } else { + config_dirs = strv_split_nulstr(CONF_PATHS_NULSTR("tmpfiles.d")); + if (!config_dirs) { + r = log_oom(); + goto finish; + } + } + + { + _cleanup_free_ char *t = NULL; + + t = strv_join(config_dirs, "\n\t"); + if (t) + log_debug("Looking for configuration files in (higher priority first:\n\t%s", t); + } + if (optind < argc) { int j; for (j = optind; j < argc; j++) { - k = read_config_file(argv[j], false, &invalid_config); + k = read_config_file((const char**) config_dirs, argv[j], false, &invalid_config); if (k < 0 && r == 0) r = k; } } else { _cleanup_strv_free_ char **files = NULL; - char **f; - r = conf_files_list_nulstr(&files, ".conf", arg_root, 0, conf_file_dirs); + r = conf_files_list_strv(&files, ".conf", arg_root, 0, (const char* const*) config_dirs); if (r < 0) { log_error_errno(r, "Failed to enumerate tmpfiles.d files: %m"); goto finish; } STRV_FOREACH(f, files) { - k = read_config_file(*f, true, &invalid_config); + k = read_config_file((const char**) config_dirs, *f, true, &invalid_config); if (k < 0 && r == 0) r = k; } From cfdda37c9fe0d28fd29a74b665072b92228d68f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 23 Nov 2017 10:54:29 +0100 Subject: [PATCH 13/20] Hook up systemd-tmpfiles as user units An explicit --user switch is necessary because for the user@0.service instance systemd-tmpfiles is running as root, and we need to distinguish that from systemd-tmpfiles running in systemd-tmpfiles*.service. Fixes #2208. v2: - restore "systemd-" prefix - add systemd-tmpfiles-clean.{service,timer}, systemd-setup.service to systemd-tmpfiles(8) --- man/systemd-tmpfiles.xml | 14 +++++++---- presets/meson.build | 3 +++ presets/user/90-systemd.preset | 14 +++++++++++ units/user/meson.build | 3 +++ units/user/systemd-tmpfiles-clean.service.in | 21 ++++++++++++++++ units/user/systemd-tmpfiles-clean.timer | 19 +++++++++++++++ units/user/systemd-tmpfiles-setup.service.in | 25 ++++++++++++++++++++ 7 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 presets/user/90-systemd.preset create mode 100644 units/user/systemd-tmpfiles-clean.service.in create mode 100644 units/user/systemd-tmpfiles-clean.timer create mode 100644 units/user/systemd-tmpfiles-setup.service.in diff --git a/man/systemd-tmpfiles.xml b/man/systemd-tmpfiles.xml index 5c9660df64..24a08b0354 100644 --- a/man/systemd-tmpfiles.xml +++ b/man/systemd-tmpfiles.xml @@ -62,10 +62,16 @@ CONFIGFILE - systemd-tmpfiles-setup.service - systemd-tmpfiles-setup-dev.service - systemd-tmpfiles-clean.service - systemd-tmpfiles-clean.timer + System units: +systemd-tmpfiles-setup.service +systemd-tmpfiles-setup-dev.service +systemd-tmpfiles-clean.service +systemd-tmpfiles-clean.timer + + User units: +systemd-tmpfiles-setup.service +systemd-tmpfiles-clean.service +systemd-tmpfiles-clean.timer diff --git a/presets/meson.build b/presets/meson.build index 89bed8a05a..48aa8c9796 100644 --- a/presets/meson.build +++ b/presets/meson.build @@ -17,3 +17,6 @@ install_data('90-systemd.preset', install_dir : systempresetdir) + +install_data('user/90-systemd.preset', + install_dir : userpresetdir) diff --git a/presets/user/90-systemd.preset b/presets/user/90-systemd.preset new file mode 100644 index 0000000000..22fe41fc33 --- /dev/null +++ b/presets/user/90-systemd.preset @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +# These ones should be enabled by default, even if distributions +# generally follow a default-off policy. + +enable systemd-tmpfiles-setup.service +enable systemd-tmpfiles-clean.timer diff --git a/units/user/meson.build b/units/user/meson.build index fbe6e3fb13..e463ae226c 100644 --- a/units/user/meson.build +++ b/units/user/meson.build @@ -29,6 +29,7 @@ units = [ 'sockets.target', 'sound.target', 'timers.target', + 'systemd-tmpfiles-clean.timer', ] foreach file : units @@ -38,6 +39,8 @@ endforeach in_units = [ 'systemd-exit.service', + 'systemd-tmpfiles-clean.service', + 'systemd-tmpfiles-setup.service', ] foreach file : in_units diff --git a/units/user/systemd-tmpfiles-clean.service.in b/units/user/systemd-tmpfiles-clean.service.in new file mode 100644 index 0000000000..9cd19720d3 --- /dev/null +++ b/units/user/systemd-tmpfiles-clean.service.in @@ -0,0 +1,21 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=Cleanup of User's Temporary Files and Directories +Documentation=man:tmpfiles.d(5) man:systemd-tmpfiles(8) +DefaultDependencies=no +Conflicts=shutdown.target +Before=basic.target shutdown.target + +[Service] +Type=oneshot +ExecStart=@rootbindir@/systemd-tmpfiles --user --clean +SuccessExitStatus=65 +IOSchedulingClass=idle diff --git a/units/user/systemd-tmpfiles-clean.timer b/units/user/systemd-tmpfiles-clean.timer new file mode 100644 index 0000000000..d1dbad98de --- /dev/null +++ b/units/user/systemd-tmpfiles-clean.timer @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=Daily Cleanup of User's Temporary Directories +Documentation=man:tmpfiles.d(5) man:systemd-tmpfiles(8) + +[Timer] +OnStartupSec=5min +OnUnitActiveSec=1d + +[Install] +WantedBy=timers.target diff --git a/units/user/systemd-tmpfiles-setup.service.in b/units/user/systemd-tmpfiles-setup.service.in new file mode 100644 index 0000000000..6467dab896 --- /dev/null +++ b/units/user/systemd-tmpfiles-setup.service.in @@ -0,0 +1,25 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=Create User's Volatile Files and Directories +Documentation=man:tmpfiles.d(5) man:systemd-tmpfiles(8) +DefaultDependencies=no +Conflicts=shutdown.target +Before=basic.target shutdown.target +RefuseManualStop=yes + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=@rootbindir@/systemd-tmpfiles --user --create --remove --boot +SuccessExitStatus=65 + +[Install] +WantedBy=basic.target From 5a8575ef01328a25253822cc2407e6baaf6d9e8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 23 Nov 2017 13:56:32 +0100 Subject: [PATCH 14/20] tmpfiles: also add %t/%S/%C/%L specifiers sd_path_home() returns ENXIO when a variable (such as $XDG_RUNTIME_DIR) is not defined. Previously we used ENOKEY for unresolvable specifiers. To avoid having two codes, or translating ENXIO to ENOKEY, I replaced ENOKEY use with ENXIO. v2: - use sd_path_home and change to ENXIO everywhere --- man/tmpfiles.d.xml | 22 ++++++++- src/test/test-systemd-tmpfiles.py | 79 +++++++++++++++++++------------ src/tmpfiles/tmpfiles.c | 79 +++++++++++++++++++++++++------ 3 files changed, 133 insertions(+), 47 deletions(-) diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index d89cb08f53..f5d97aa38f 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -653,9 +653,29 @@ r! /tmp/.X[0-9]*-lock User home directory This is the home directory of the user running the service manager instance. In case of the system manager this resolves to /root. + + %t + System or user runtime directory + In --user mode, this is the same $XDG_RUNTIME_DIR, and /run otherwise. + + + %S + System or user state directory + In mode, this is the same as $XDG_CONFIG_HOME, and /var/lib otherwise. + + + %C + System or user cache directory + In mode, this is the same as $XDG_CACHE_HOME, and /var/cache otherwise. + + + %L + System or user log directory + In mode, this is the same as $XDG_CONFIG_HOME with /log appended, and /var/log otherwise. + %% - Escaped % + Escaped % Single percent sign. diff --git a/src/test/test-systemd-tmpfiles.py b/src/test/test-systemd-tmpfiles.py index 75d8fd3657..8c1bfde459 100755 --- a/src/test/test-systemd-tmpfiles.py +++ b/src/test/test-systemd-tmpfiles.py @@ -8,6 +8,7 @@ # the Free Software Foundation; either version 2.1 of the License, or # (at your option) any later version. +import os import sys import subprocess @@ -15,37 +16,53 @@ EX_DATAERR = 65 # from sysexits.h exe = sys.argv[1] -def test_invalid_line(line): - print('Running {} on {!r}'.format(exe, line)) - c = subprocess.run([exe, '--create', '-'], +def test_line(line, *, user, returncode=EX_DATAERR): + args = ['--user'] if user else [] + print('Running {} {} on {!r}'.format(exe, ' '.join(args), line)) + c = subprocess.run([exe, '--create', *args, '-'], input=line, stdout=subprocess.PIPE, universal_newlines=True) - assert c.returncode == EX_DATAERR, c + assert c.returncode == returncode, c + +def test_invalids(*, user): + test_line('asdfa', user=user) + test_line('f "open quote', user=user) + test_line('f closed quote""', user=user) + test_line('Y /unknown/letter', user=user) + test_line('w non/absolute/path', user=user) + test_line('s', user=user) # s is for short + test_line('f!! /too/many/bangs', user=user) + test_line('f++ /too/many/plusses', user=user) + test_line('f+!+ /too/many/plusses', user=user) + test_line('f!+! /too/many/bangs', user=user) + test_line('w /unresolved/argument - - - - "%Y"', user=user) + test_line('w /unresolved/argument/sandwich - - - - "%v%Y%v"', user=user) + test_line('w /unresolved/filename/%Y - - - - "whatever"', user=user) + test_line('w /unresolved/filename/sandwich/%v%Y%v - - - - "whatever"', user=user) + test_line('w - - - - - "no file specfied"', user=user) + test_line('C - - - - - "no file specfied"', user=user) + test_line('C non/absolute/path - - - - -', user=user) + test_line('b - - - - - -', user=user) + test_line('b 1234 - - - - -', user=user) + test_line('c - - - - - -', user=user) + test_line('c 1234 - - - - -', user=user) + test_line('t - - -', user=user) + test_line('T - - -', user=user) + test_line('a - - -', user=user) + test_line('A - - -', user=user) + test_line('h - - -', user=user) + test_line('H - - -', user=user) + +def test_unitialized_t(): + if os.getuid() == 0: + return + + try: + del os.environ['XDG_RUNTIME_DIR'] + except KeyError: + pass + test_line('w /foo - - - - "specifier for --user %t"', user=True, returncode=0) if __name__ == '__main__': - test_invalid_line('asdfa') - test_invalid_line('f "open quote') - test_invalid_line('f closed quote""') - test_invalid_line('Y /unknown/letter') - test_invalid_line('w non/absolute/path') - test_invalid_line('s') # s is for short - test_invalid_line('f!! /too/many/bangs') - test_invalid_line('f++ /too/many/plusses') - test_invalid_line('f+!+ /too/many/plusses') - test_invalid_line('f!+! /too/many/bangs') - test_invalid_line('w /unresolved/argument - - - - "%Y"') - test_invalid_line('w /unresolved/argument/sandwich - - - - "%v%Y%v"') - test_invalid_line('w /unresolved/filename/%Y - - - - "whatever"') - test_invalid_line('w /unresolved/filename/sandwich/%v%Y%v - - - - "whatever"') - test_invalid_line('w - - - - - "no file specfied"') - test_invalid_line('C - - - - - "no file specfied"') - test_invalid_line('C non/absolute/path - - - - -') - test_invalid_line('b - - - - - -') - test_invalid_line('b 1234 - - - - -') - test_invalid_line('c - - - - - -') - test_invalid_line('c 1234 - - - - -') - test_invalid_line('t - - -') - test_invalid_line('T - - -') - test_invalid_line('a - - -') - test_invalid_line('A - - -') - test_invalid_line('h - - -') - test_invalid_line('H - - -') + test_invalids(user=False) + test_invalids(user=True) + test_unitialized_t() diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 8d57239177..2344189426 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -37,6 +37,8 @@ #include #include +#include "sd-path.h" + #include "acl-util.h" #include "alloc-util.h" #include "btrfs-util.h" @@ -152,6 +154,14 @@ typedef struct ItemArray { size_t size; } ItemArray; +typedef enum DirectoryType { + DIRECTORY_RUNTIME = 0, + DIRECTORY_STATE, + DIRECTORY_CACHE, + DIRECTORY_LOGS, + _DIRECTORY_TYPE_MAX, +} DirectoryType; + static bool arg_user = false; static bool arg_create = false; static bool arg_clean = false; @@ -168,16 +178,21 @@ static OrderedHashmap *items = NULL, *globs = NULL; static Set *unix_sockets = NULL; static int specifier_machine_id_safe(char specifier, void *data, void *userdata, char **ret); +static int specifier_directory(char specifier, void *data, void *userdata, char **ret); static const Specifier specifier_table[] = { { 'm', specifier_machine_id_safe, NULL }, - { 'b', specifier_boot_id, NULL }, - { 'H', specifier_host_name, NULL }, - { 'v', specifier_kernel_release, NULL }, + { 'b', specifier_boot_id, NULL }, + { 'H', specifier_host_name, NULL }, + { 'v', specifier_kernel_release, NULL }, - { 'U', specifier_user_id, NULL }, - { 'u', specifier_user_name, NULL }, - { 'h', specifier_user_home, NULL }, + { 'U', specifier_user_id, NULL }, + { 'u', specifier_user_name, NULL }, + { 'h', specifier_user_home, NULL }, + { 't', specifier_directory, UINT_TO_PTR(DIRECTORY_RUNTIME) }, + { 'S', specifier_directory, UINT_TO_PTR(DIRECTORY_STATE) }, + { 'C', specifier_directory, UINT_TO_PTR(DIRECTORY_CACHE) }, + { 'L', specifier_directory, UINT_TO_PTR(DIRECTORY_LOGS) }, {} }; @@ -190,22 +205,56 @@ static int specifier_machine_id_safe(char specifier, void *data, void *userdata, r = specifier_machine_id(specifier, data, userdata, ret); if (r == -ENOENT) - return -ENOKEY; + return -ENXIO; return r; } +static int specifier_directory(char specifier, void *data, void *userdata, char **ret) { + struct table_entry { + uint64_t type; + const char *suffix; + }; + + static const struct table_entry paths_system[] = { + [DIRECTORY_RUNTIME] = { SD_PATH_SYSTEM_RUNTIME }, + [DIRECTORY_STATE] = { SD_PATH_SYSTEM_STATE_PRIVATE }, + [DIRECTORY_CACHE] = { SD_PATH_SYSTEM_STATE_CACHE }, + [DIRECTORY_LOGS] = { SD_PATH_SYSTEM_STATE_LOGS }, + }; + + static const struct table_entry paths_user[] = { + [DIRECTORY_RUNTIME] = { SD_PATH_USER_RUNTIME }, + [DIRECTORY_STATE] = { SD_PATH_USER_CONFIGURATION }, + [DIRECTORY_CACHE] = { SD_PATH_USER_STATE_CACHE }, + [DIRECTORY_LOGS] = { SD_PATH_USER_CONFIGURATION, "log" }, + }; + + unsigned i; + const struct table_entry *paths; + + assert_cc(ELEMENTSOF(paths_system) == ELEMENTSOF(paths_user)); + paths = arg_user ? paths_user : paths_system; + + i = PTR_TO_UINT(data); + assert(i < ELEMENTSOF(paths_system)); + + return sd_path_home(paths[i].type, paths[i].suffix, ret); +} + static int log_unresolvable_specifier(const char *filename, unsigned line) { static bool notified = false; - /* This is called when /etc is not fully initialized (e.g. in a chroot - * environment) where some specifiers are unresolvable. These cases are - * not considered as an error so log at LOG_NOTICE only for the first - * time and then downgrade this to LOG_DEBUG for the rest. */ + /* In system mode, this is called when /etc is not fully initialized (e.g. + * in a chroot environment) where some specifiers are unresolvable. In user + * mode, this is called when some variables are not defined. These cases are + * not considered as an error so log at LOG_NOTICE only for the first time + * and then downgrade this to LOG_DEBUG for the rest. */ log_full(notified ? LOG_DEBUG : LOG_NOTICE, - "[%s:%u] Failed to resolve specifier: uninitialized /etc detected, skipping", - filename, line); + "[%s:%u] Failed to resolve specifier: %s, skipping", + filename, line, + arg_user ? "Required $XDG_... variable not defined" : "uninitialized /etc detected"); if (!notified) log_notice("All rules containing unresolvable specifiers will be skipped."); @@ -1969,7 +2018,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool i.force = force; r = specifier_printf(path, specifier_table, NULL, &i.path); - if (r == -ENOKEY) + if (r == -ENXIO) return log_unresolvable_specifier(fname, line); if (r < 0) { if (IN_SET(r, -EINVAL, -EBADSLT)) @@ -2108,7 +2157,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool return 0; r = specifier_expansion_from_arg(&i); - if (r == -ENOKEY) + if (r == -ENXIO) return log_unresolvable_specifier(fname, line); if (r < 0) { if (IN_SET(r, -EINVAL, -EBADSLT)) From 0deb073a666540bed47c4b8add1f1e00ddeab9db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Dec 2017 18:53:10 +0100 Subject: [PATCH 15/20] man: improve formatting in systemd.unit.xml --- man/systemd.unit.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 06a7bbd3ba..159f629498 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1357,18 +1357,18 @@ %S - State directory root + State directory root This is either /var/lib (for the system manager) or the path $XDG_CONFIG_HOME resolves to (for user managers). %C - Cache directory root + Cache directory root This is either /var/cache (for the system manager) or the path $XDG_CACHE_HOME resolves to (for user managers). %L - Logs directory root - This is either /var/log (for the system manager) or the path $XDG_CONFIG_HOME resolves to with /log appended (for user managers). + Log directory root + This is either /var/log (for the system manager) or the path $XDG_CONFIG_HOME resolves to with /log appended (for user managers). %u From c987fefc43f826d1e480dc5772ce66533726e3a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Dec 2017 18:53:24 +0100 Subject: [PATCH 16/20] Fix typo --- src/shared/specifier.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/specifier.c b/src/shared/specifier.c index 9034cc20d7..6839d1892d 100644 --- a/src/shared/specifier.c +++ b/src/shared/specifier.c @@ -207,7 +207,7 @@ int specifier_user_name(char specifier, void *data, void *userdata, char **ret) /* If we are UID 0 (root), this will not result in NSS, otherwise it might. This is good, as we want to be able * to run this in PID 1, where our user ID is 0, but where NSS lookups are not allowed. - * We don't user getusername_malloc() here, because we don't want to look at $USER, to remain consistent with + * We don't use getusername_malloc() here, because we don't want to look at $USER, to remain consistent with * specifer_user_id() below. */ From 03025f46afa6e3c24e539a625a5ee15387fb4e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 1 Dec 2017 21:15:51 +0100 Subject: [PATCH 17/20] test-systemd-tmpfiles: add tests for specifiers --- src/test/test-systemd-tmpfiles.py | 68 ++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 6 deletions(-) diff --git a/src/test/test-systemd-tmpfiles.py b/src/test/test-systemd-tmpfiles.py index 8c1bfde459..b73368ddef 100755 --- a/src/test/test-systemd-tmpfiles.py +++ b/src/test/test-systemd-tmpfiles.py @@ -10,16 +10,25 @@ import os import sys +import socket import subprocess +import tempfile +import pwd + +try: + from systemd import id128 +except ImportError: + id128 = None EX_DATAERR = 65 # from sysexits.h exe = sys.argv[1] -def test_line(line, *, user, returncode=EX_DATAERR): +def test_line(line, *, user, returncode=EX_DATAERR, extra={}): args = ['--user'] if user else [] print('Running {} {} on {!r}'.format(exe, ' '.join(args), line)) c = subprocess.run([exe, '--create', *args, '-'], + **extra, input=line, stdout=subprocess.PIPE, universal_newlines=True) assert c.returncode == returncode, c @@ -56,13 +65,60 @@ def test_unitialized_t(): if os.getuid() == 0: return - try: - del os.environ['XDG_RUNTIME_DIR'] - except KeyError: - pass - test_line('w /foo - - - - "specifier for --user %t"', user=True, returncode=0) + test_line('w /foo - - - - "specifier for --user %t"', + user=True, returncode=0, extra={'env':{}}) + +def test_content(line, expected, *, user, extra={}): + d = tempfile.TemporaryDirectory(prefix='test-systemd-tmpfiles.') + arg = d.name + '/arg' + spec = line.format(arg) + test_line(spec, user=user, returncode=0, extra=extra) + content = open(arg).read() + print('expect: {!r}\nactual: {!r}'.format(expected, content)) + assert content == expected + +def test_valid_specifiers(*, user): + test_content('f {} - - - - two words', 'two words', user=user) + if id128: + test_content('f {} - - - - %m', '{}'.format(id128.get_machine().hex), user=user) + test_content('f {} - - - - %b', '{}'.format(id128.get_boot().hex), user=user) + test_content('f {} - - - - %H', '{}'.format(socket.gethostname()), user=user) + test_content('f {} - - - - %v', '{}'.format(os.uname().release), user=user) + test_content('f {} - - - - %U', '{}'.format(os.getuid()), user=user) + + user = pwd.getpwuid(os.getuid()) + test_content('f {} - - - - %u', '{}'.format(user.pw_name), user=user) + test_content('f {} - - - - %h', '{}'.format(user.pw_dir), user=user) + + xdg_runtime_dir = os.getenv('XDG_RUNTIME_DIR') + if xdg_runtime_dir is not None or not user: + test_content('f {} - - - - %t', + xdg_runtime_dir if user else '/run', + user=user) + + xdg_config_home = os.getenv('XDG_CONFIG_HOME') + if xdg_config_home is not None or not user: + test_content('f {} - - - - %S', + xdg_config_home if user else '/var/lib', + user=user) + + xdg_cache_home = os.getenv('XDG_CACHE_HOME') + if xdg_cache_home is not None or not user: + test_content('f {} - - - - %C', + xdg_cache_home if user else '/var/cache', + user=user) + + if xdg_config_home is not None or not user: + test_content('f {} - - - - %L', + xdg_config_home + '/log' if user else '/var/log', + user=user) + + test_content('f {} - - - - %%', '%', user=user) if __name__ == '__main__': test_invalids(user=False) test_invalids(user=True) test_unitialized_t() + + test_valid_specifiers(user=False) + test_valid_specifiers(user=True) From 7488492a81db8011758325bd6a7c949c38a0b7c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 2 Dec 2017 14:00:58 +0100 Subject: [PATCH 18/20] test-systemd-tmpfiles: skip on python3.4 python3.4 is used by our CI. Let's revert this when we stop supporting python < 3.5. --- src/test/test-systemd-tmpfiles.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/test-systemd-tmpfiles.py b/src/test/test-systemd-tmpfiles.py index b73368ddef..1368839381 100755 --- a/src/test/test-systemd-tmpfiles.py +++ b/src/test/test-systemd-tmpfiles.py @@ -21,15 +21,21 @@ except ImportError: id128 = None EX_DATAERR = 65 # from sysexits.h +EXIT_TEST_SKIP = 77 + +try: + subprocess.run +except AttributeError: + sys.exit(EXIT_TEST_SKIP) exe = sys.argv[1] def test_line(line, *, user, returncode=EX_DATAERR, extra={}): args = ['--user'] if user else [] print('Running {} {} on {!r}'.format(exe, ' '.join(args), line)) - c = subprocess.run([exe, '--create', *args, '-'], - **extra, - input=line, stdout=subprocess.PIPE, universal_newlines=True) + c = subprocess.run([exe, '--create', '-'] + args, + input=line, stdout=subprocess.PIPE, universal_newlines=True, + **extra) assert c.returncode == returncode, c def test_invalids(*, user): From df1172fe726422a46f36c992c5553698657a9bde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 2 Dec 2017 15:40:30 +0100 Subject: [PATCH 19/20] test-systemd-tmpfiles: ignore result of %m test It's failing on artful s390x and i386: Running /tmp/autopkgtest.Pexzdu/build.lfO/debian/build-deb/systemd-tmpfiles on 'f /tmp/test-systemd-tmpfiles.c236s1uq/arg - - - - %m' expect: '01234567890123456789012345678901' actual: 'e84bc78d162e472a8ac9759f5f1e4e0e' --- stderr --- Traceback (most recent call last): File "/tmp/autopkgtest.Pexzdu/build.lfO/debian/src/test/test-systemd-tmpfiles.py", line 129, in test_valid_specifiers(user=False) File "/tmp/autopkgtest.Pexzdu/build.lfO/debian/src/test/test-systemd-tmpfiles.py", line 89, in test_valid_specifiers test_content('f {} - - - - %m', '{}'.format(id128.get_machine().hex), user=user) File "/tmp/autopkgtest.Pexzdu/build.lfO/debian/src/test/test-systemd-tmpfiles.py", line 84, in test_content assert content == expected AssertionError ------- Let's skip the test for now until this is resolved properly on the autopkgtest side. --- src/test/test-systemd-tmpfiles.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/test-systemd-tmpfiles.py b/src/test/test-systemd-tmpfiles.py index 1368839381..00362c6952 100755 --- a/src/test/test-systemd-tmpfiles.py +++ b/src/test/test-systemd-tmpfiles.py @@ -86,7 +86,13 @@ def test_content(line, expected, *, user, extra={}): def test_valid_specifiers(*, user): test_content('f {} - - - - two words', 'two words', user=user) if id128: - test_content('f {} - - - - %m', '{}'.format(id128.get_machine().hex), user=user) + try: + test_content('f {} - - - - %m', '{}'.format(id128.get_machine().hex), user=user) + except AssertionError as e: + print(e) + print('/etc/machine-id: {!r}'.format(open('/etc/machine-id').read())) + print('/proc/cmdline: {!r}'.format(open('/proc/cmdline').read())) + print('skipping') test_content('f {} - - - - %b', '{}'.format(id128.get_boot().hex), user=user) test_content('f {} - - - - %H', '{}'.format(socket.gethostname()), user=user) test_content('f {} - - - - %v', '{}'.format(os.uname().release), user=user) From 2f813b8aae1a6f1960d41d8750712d5d6c651e03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 4 Dec 2017 09:05:05 +0100 Subject: [PATCH 20/20] test-systemd-tmpfiles: respect $HOME in test for %h expansion %h is a special specifier because we look at $HOME (unless running suid, but let's say that this case does not apply to tmpfiles, since the code is completely unready to be run suid). For all other specifiers we query the user db and use those values directly. I'm not sure if this exception is good, but let's just "document" status quo for now. If this is changes, it should be in a separate PR. --- src/test/test-systemd-tmpfiles.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/test-systemd-tmpfiles.py b/src/test/test-systemd-tmpfiles.py index 00362c6952..309cee23e6 100755 --- a/src/test/test-systemd-tmpfiles.py +++ b/src/test/test-systemd-tmpfiles.py @@ -100,7 +100,11 @@ def test_valid_specifiers(*, user): user = pwd.getpwuid(os.getuid()) test_content('f {} - - - - %u', '{}'.format(user.pw_name), user=user) - test_content('f {} - - - - %h', '{}'.format(user.pw_dir), user=user) + + # Note that %h is the only specifier in which we look the environment, + # because we check $HOME. Should we even be doing that? + home = os.path.expanduser("~") + test_content('f {} - - - - %h', '{}'.format(home), user=user) xdg_runtime_dir = os.getenv('XDG_RUNTIME_DIR') if xdg_runtime_dir is not None or not user: