From 941060bf5efae24eb9879e42444e15c649973473 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Apr 2016 13:26:12 +0200 Subject: [PATCH 1/5] path-util: document that we shouldn't add further entries to hidden_or_backup_file() And let's add ".bak" as a generic suffix for backups, that people can use without having to register their stuff in our list. --- src/basic/path-util.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 100e3f5af2..bcf72913df 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -772,6 +772,19 @@ bool hidden_or_backup_file(const char *filename) { if (!p) return false; + /* Please, let's not add more entries to the list below. If external projects think it's a good idea to come up + * with always new suffixes and that everybody else should just adjust to that, then it really should be on + * them. Hence, in future, let's not add any more entries. Instead, let's ask those packages to instead adopt + * one of the generic suffixes/prefixes for hidden files or backups, possibly augmented with an additional + * string. Specifically: there's now: + * + * The generic suffixes "~" and ".bak" for backup files + * The generic prefix "." for hidden files + * + * Thus, if a new package manager "foopkg" wants its own set of "foopkg-new", "foopkg-old", "foopkg-dist" or so + * registered, let's refuse that and ask them to use "foopkg-new.bak" or "foopkg-new~" instead. + */ + return STR_IN_SET(p + 1, "rpmnew", "rpmsave", @@ -786,7 +799,8 @@ bool hidden_or_backup_file(const char *filename) { "ucf-new", "ucf-old", "ucf-dist", - "swp"); + "swp", + "bak"); } bool is_device_path(const char *path) { From 14e2baa369b120788322e727ecdd249127a13fa1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Apr 2016 13:36:38 +0200 Subject: [PATCH 2/5] man: document that RemainAfterExit= doesn't make much sense for repetitive timers Fixes #3122 --- man/systemd.timer.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/man/systemd.timer.xml b/man/systemd.timer.xml index eb0afee141..0d0cccf152 100644 --- a/man/systemd.timer.xml +++ b/man/systemd.timer.xml @@ -73,6 +73,12 @@ foo.timer activates a matching service foo.service. The unit to activate may be controlled by Unit= (see below). + + Note that in case the unit to activate is already active at the time the timer elapses it is not restarted, + but simply left running. There is no concept of spawning new service instances in this case. Due to this, services + with RemainAfterExit= set (which stay around continously even after the service's main process + exited) are usually not suitable for activation via repetitive timers, as they will only be activated once, and + then stay around forever. From 0e2b2caccde55ba777b84d667d19541a82c3f3a3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Apr 2016 14:21:22 +0200 Subject: [PATCH 3/5] copy: also copy AF_UNIX sockets We previously would fail with EOPNOTSUPP when encountering an AF_UNIX socket in the directory tree to copy. Fix that, and copy them too (even if they are dead in the result). Fixes: #2914 --- src/basic/copy.c | 4 ++-- src/test/test-copy.c | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/basic/copy.c b/src/basic/copy.c index 03487a6878..3001234a01 100644 --- a/src/basic/copy.c +++ b/src/basic/copy.c @@ -364,7 +364,7 @@ static int fd_copy_directory( q = fd_copy_symlink(dirfd(d), de->d_name, &buf, fdt, de->d_name); else if (S_ISFIFO(buf.st_mode)) q = fd_copy_fifo(dirfd(d), de->d_name, &buf, fdt, de->d_name); - else if (S_ISBLK(buf.st_mode) || S_ISCHR(buf.st_mode)) + else if (S_ISBLK(buf.st_mode) || S_ISCHR(buf.st_mode) || S_ISSOCK(buf.st_mode)) q = fd_copy_node(dirfd(d), de->d_name, &buf, fdt, de->d_name); else q = -EOPNOTSUPP; @@ -396,7 +396,7 @@ int copy_tree_at(int fdf, const char *from, int fdt, const char *to, bool merge) return fd_copy_symlink(fdf, from, &st, fdt, to); else if (S_ISFIFO(st.st_mode)) return fd_copy_fifo(fdf, from, &st, fdt, to); - else if (S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode)) + else if (S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode) || S_ISSOCK(st.st_mode)) return fd_copy_node(fdf, from, &st, fdt, to); else return -EOPNOTSUPP; diff --git a/src/test/test-copy.c b/src/test/test-copy.c index cb437754b4..d1bf376385 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -95,6 +95,8 @@ static void test_copy_tree(void) { char **links = STRV_MAKE("link", "file", "link2", "dir1/file"); char **p, **link; + const char *unixsockp; + struct stat st; log_info("%s", __func__); @@ -116,6 +118,9 @@ static void test_copy_tree(void) { assert_se(symlink(f, l) == 0); } + unixsockp = strjoina(original_dir, "unixsock"); + assert_se(mknod(unixsockp, S_IFSOCK|0644, 0) >= 0); + assert_se(copy_tree(original_dir, copy_dir, true) == 0); STRV_FOREACH(p, files) { @@ -137,6 +142,10 @@ static void test_copy_tree(void) { assert_se(path_equal(f, target)); } + unixsockp = strjoina(copy_dir, "unixsock"); + assert_se(stat(unixsockp, &st) >= 0); + assert_se(S_ISSOCK(st.st_mode)); + assert_se(copy_tree(original_dir, copy_dir, false) < 0); assert_se(copy_tree("/tmp/inexistent/foo/bar/fsdoi", copy_dir, false) < 0); From 2397bc4312f6d1fbdf97bf826a1ca6ad409aabf3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Apr 2016 14:25:52 +0200 Subject: [PATCH 4/5] test-copy: never call alloca() in a loop That's a total no-no, hence rework this to use malloc()-based memory instead of alloca()-based memory. Also see CODING_STYLE about this. --- src/test/test-copy.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/test/test-copy.c b/src/test/test-copy.c index d1bf376385..68154fc4e8 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -104,15 +104,19 @@ static void test_copy_tree(void) { (void) rm_rf(original_dir, REMOVE_ROOT|REMOVE_PHYSICAL); STRV_FOREACH(p, files) { - char *f = strjoina(original_dir, *p); + _cleanup_free_ char *f; + + assert_se(f = strappend(original_dir, *p)); assert_se(mkdir_parents(f, 0755) >= 0); assert_se(write_string_file(f, "file", WRITE_STRING_FILE_CREATE) == 0); } STRV_FOREACH_PAIR(link, p, links) { - char *f = strjoina(original_dir, *p); - char *l = strjoina(original_dir, *link); + _cleanup_free_ char *f, *l; + + assert_se(f = strappend(original_dir, *p)); + assert_se(l = strappend(original_dir, *link)); assert_se(mkdir_parents(l, 0755) >= 0); assert_se(symlink(f, l) == 0); @@ -124,9 +128,10 @@ static void test_copy_tree(void) { assert_se(copy_tree(original_dir, copy_dir, true) == 0); STRV_FOREACH(p, files) { - _cleanup_free_ char *buf = NULL; + _cleanup_free_ char *buf = NULL, *f; size_t sz = 0; - char *f = strjoina(copy_dir, *p); + + assert_se(f = strappend(copy_dir, *p)); assert_se(access(f, F_OK) == 0); assert_se(read_full_file(f, &buf, &sz) == 0); @@ -134,9 +139,10 @@ static void test_copy_tree(void) { } STRV_FOREACH_PAIR(link, p, links) { - _cleanup_free_ char *target = NULL; - char *f = strjoina(original_dir, *p); - char *l = strjoina(copy_dir, *link); + _cleanup_free_ char *target = NULL, *f, *l; + + assert_se(f = strjoin(original_dir, *p, NULL)); + assert_se(l = strjoin(copy_dir, *link, NULL)); assert_se(readlink_and_canonicalize(l, &target) == 0); assert_se(path_equal(f, target)); From 94a0ef6e572896140b8f28f688e881e198e2f2ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 29 Apr 2016 10:17:43 -0400 Subject: [PATCH 5/5] path-util: also support ".old" and ".new" suffixes and recommend them ~ suffix works fine, but looks to much like it the file is supposed to be automatically cleaned up. For new versions of configuration files installers might want to using something that looks more permanent like foobar.new. So let's add treat ".old" and ".new" as special. Update test to match. --- src/basic/path-util.c | 8 +++++--- test/sysv-generator-test.py | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index bcf72913df..b2fa81a294 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -781,8 +781,8 @@ bool hidden_or_backup_file(const char *filename) { * The generic suffixes "~" and ".bak" for backup files * The generic prefix "." for hidden files * - * Thus, if a new package manager "foopkg" wants its own set of "foopkg-new", "foopkg-old", "foopkg-dist" or so - * registered, let's refuse that and ask them to use "foopkg-new.bak" or "foopkg-new~" instead. + * Thus, if a new package manager "foopkg" wants its own set of ".foopkg-new", ".foopkg-old", ".foopkg-dist" + * or so registered, let's refuse that and ask them to use ".foopkg.new", ".foopkg.old" or ".foopkg~" instead. */ return STR_IN_SET(p + 1, @@ -800,7 +800,9 @@ bool hidden_or_backup_file(const char *filename) { "ucf-old", "ucf-dist", "swp", - "bak"); + "bak", + "old", + "new"); } bool is_device_path(const char *path) { diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py index aadc29ebeb..838dd57a6f 100755 --- a/test/sysv-generator-test.py +++ b/test/sysv-generator-test.py @@ -397,11 +397,12 @@ class SysvGeneratorTest(unittest.TestCase): # backup files (not enabled in rcN.d/) shutil.copy(script, script + '.bak') shutil.copy(script, script + '.old') + shutil.copy(script, script + '.tmp') + shutil.copy(script, script + '.new') err, results = self.run_generator() print(err) - self.assertEqual(sorted(results), - ['foo.bak.service', 'foo.old.service', 'foo.service']) + self.assertEqual(sorted(results), ['foo.service', 'foo.tmp.service']) # ensure we don't try to create a symlink to itself self.assertNotIn('itself', err)