tmpfiles: use dir_cleanup() for R and D

... i.e. apply nested config (exclusions and such) when executing R and D.

This fixes a long-standing RFE. The existing logic seems to have been an
accident of implementation. After all, if somebody specifies a config with
'R /foo; x /tmp/bar', then probably the goal is to remove stuff from under /foo,
but keep /tmp/bar. If they just wanted to nuke everything, then would not specify
the second item.

This also makes R and D use O_NOATIME, i.e. the access times of the directories
that are accessed will not be changed by the cleanup.

Obviously, we'll have to add this to NEWS and such.
Looking at the whole tmpfiles.d config in Fedora, this change has no effect.

The test cases are adjusted as appropriate. I also added another test case for
'R'/'D' with a file, just to test this code path more.

Replaces #20641.
Fixes #1633.

TODO

@@ -2446,7 +2446,6 @@ Features:
 * support crash reporting operation modes (https://live.gnome.org/GnomeOS/Design/Whiteboards/ProblemReporting)
 
 * tmpfiles:
-  - apply "x" on "D" too (see patch from William Douglas)
   - allow time-based cleanup in r and R too
   - instead of ignoring unknown fields, reject them.
   - creating new directories/subvolumes/fifos/device nodes

man/tmpfiles.d.xml

@@ -353,9 +353,7 @@ L     /tmp/foobar -    -    -     -   /dev/null</programlisting>
           <term><varname>x</varname></term>
           <listitem><para>Ignore a path during cleaning. Use this type
           to exclude paths from clean-up as controlled with the Age
-          parameter. Note that lines of this type do not influence the
-          effect of <varname>r</varname> or <varname>R</varname>
-          lines. Lines of this type accept shell-style globs in place
+          parameter. Lines of this type accept shell-style globs in place
           of normal path names.  </para></listitem>
         </varlistentry>
 
@@ -365,9 +363,7 @@ L     /tmp/foobar -    -    -     -   /dev/null</programlisting>
           to exclude paths from clean-up as controlled with the Age
           parameter. Unlike <varname>x</varname>, this parameter will
           not exclude the content if path is a directory, but only
-          directory itself. Note that lines of this type do not
-          influence the effect of <varname>r</varname> or
-          <varname>R</varname> lines. Lines of this type accept
+          directory itself. Lines of this type accept
           shell-style globs in place of normal path names.
           </para>
 

src/tmpfiles/tmpfiles.c

@@ -904,7 +904,7 @@ static int dir_cleanup(
         }
 
 finish:
-        if (deleted) {
+        if (deleted && (self_atime_nsec < NSEC_INFINITY || self_mtime_nsec < NSEC_INFINITY)) {
                 struct timespec ts[2];
 
                 log_debug("Restoring access and modification time on \"%s\": %s, %s",
@@ -2907,26 +2907,62 @@ static int create_item(Context *c, Item *i) {
         return 0;
 }
 
-static int purge_item_instance(Context *c, Item *i, const char *instance, CreationMode creation) {
+static int remove_recursive(
+                Context *c,
+                Item *i,
+                const char *instance,
+                bool remove_instance) {
+
+        _cleanup_closedir_ DIR *d = NULL;
+        STRUCT_STATX_DEFINE(sx);
+        bool mountpoint;
         int r;
 
-        /* FIXME: we probably should use dir_cleanup() here instead of rm_rf() so that 'x' is honoured. */
-        log_debug("rm -rf \"%s\"", instance);
-        r = rm_rf(instance, REMOVE_ROOT|REMOVE_SUBVOLUME|REMOVE_PHYSICAL);
-        if (r < 0 && r != -ENOENT)
-                return log_error_errno(r, "rm_rf(%s): %m", instance);
+        r = opendir_and_stat(instance, &d, &sx, &mountpoint);
+        if (r < 0)
+                return r;
+        if (r == 0) {
+                if (remove_instance) {
+                        log_debug("Removing file \"%s\".", instance);
+                        if (remove(instance) < 0 && errno != ENOENT)
+                                return log_error_errno(errno, "rm %s: %m", instance);
+                }
+                return 0;
+        }
 
+        r = dir_cleanup(c, i, instance, d,
+                        /* self_atime_nsec= */ NSEC_INFINITY,
+                        /* self_mtime_nsec= */ NSEC_INFINITY,
+                        /* cutoff_nsec= */ NSEC_INFINITY,
+                        sx.stx_dev_major, sx.stx_dev_minor,
+                        mountpoint,
+                        MAX_DEPTH,
+                        /* keep_this_level= */ false,
+                        /* age_by_file= */ 0,
+                        /* age_by_dir= */ 0);
+        if (r < 0)
+                return r;
+
+        if (remove_instance) {
+                log_debug("Removing directory \"%s\".", instance);
+                r = RET_NERRNO(rmdir(instance));
+                if (r < 0 && !IN_SET(r, -ENOENT, -ENOTEMPTY))
+                        return log_error_errno(r, "Failed to remove %s: %m", instance);
+        }
         return 0;
 }
 
-static int purge_item(Context *c, Item *i) {
+static int purge_item_instance(Context *c, Item *i, const char *instance, CreationMode creation) {
+        return remove_recursive(c, i, instance, /* remove_instance= */ true);
+}
 
+static int purge_item(Context *c, Item *i) {
         assert(i);
 
         if (!needs_purge(i->type))
                 return 0;
 
-        log_debug("Running purge owned action for entry %c %s", (char) i->type, i->path);
+        log_debug("Running purge action for entry %c %s", (char) i->type, i->path);
 
         if (needs_glob(i->type))
                 return glob_item(c, i, purge_item_instance);
@@ -2940,8 +2976,6 @@ static int remove_item_instance(
                 const char *instance,
                 CreationMode creation) {
 
-        int r;
-
         assert(c);
         assert(i);
 
@@ -2949,29 +2983,19 @@ static int remove_item_instance(
 
         case REMOVE_PATH:
                 if (remove(instance) < 0 && errno != ENOENT)
-                        return log_error_errno(errno, "rm(%s): %m", instance);
+                        return log_error_errno(errno, "rm %s: %m", instance);
 
-                break;
+                return 0;
 
         case RECURSIVE_REMOVE_PATH:
-                /* FIXME: we probably should use dir_cleanup() here instead of rm_rf() so that 'x' is honoured. */
-                log_debug("rm -rf \"%s\"", instance);
-                r = rm_rf(instance, REMOVE_ROOT|REMOVE_SUBVOLUME|REMOVE_PHYSICAL);
-                if (r < 0 && r != -ENOENT)
-                        return log_error_errno(r, "rm_rf(%s): %m", instance);
-
-                break;
+                return remove_recursive(c, i, instance, /* remove_instance= */ true);
 
         default:
                 assert_not_reached();
         }
-
-        return 0;
 }
 
 static int remove_item(Context *c, Item *i) {
-        int r;
-
         assert(c);
         assert(i);
 
@@ -2980,13 +3004,7 @@ static int remove_item(Context *c, Item *i) {
         switch (i->type) {
 
         case TRUNCATE_DIRECTORY:
-                /* FIXME: we probably should use dir_cleanup() here instead of rm_rf() so that 'x' is honoured. */
-                log_debug("rm -rf \"%s\"", i->path);
-                r = rm_rf(i->path, REMOVE_PHYSICAL);
-                if (r < 0 && r != -ENOENT)
-                        return log_error_errno(r, "rm_rf(%s): %m", i->path);
-
-                return 0;
+                return remove_recursive(c, i, i->path, /* remove_instance= */ false);
 
         case REMOVE_PATH:
         case RECURSIVE_REMOVE_PATH:

test/units/testsuite-22.11.sh

@@ -126,16 +126,34 @@ mkdir -p /tmp/x/{1,2}/a
 touch /tmp/x/1/a/{x1,x2} /tmp/x/2/a/{y1,y2}
 
 systemd-tmpfiles --remove - <<EOF
-# X is not supposed to influence R
+# Check that X is honoured below R
 X /tmp/x/1/a
 X /tmp/x/2/a
 R /tmp/x/1
 EOF
 
 find /tmp/x | sort
-test ! -d /tmp/x/1
-test ! -d /tmp/x/1/a
-test ! -f /tmp/x/1/a/x1
-test ! -f /tmp/x/1/a/x2
+test -d /tmp/x/1
+test -d /tmp/x/1/a
+test -f /tmp/x/1/a/x1
+test -f /tmp/x/1/a/x2
 test -f /tmp/x/2/a/y1
 test -f /tmp/x/2/a/y2
+
+#
+# 'r/R/D' and non-directories
+#
+
+touch /tmp/x/{11,22,33}
+
+systemd-tmpfiles --remove - <<EOF
+# Check that X is honoured below R
+r /tmp/x/11
+R /tmp/x/22
+D /tmp/x/33
+EOF
+
+find /tmp/x | sort
+test ! -f /tmp/x/11
+test ! -f /tmp/x/22
+test -f /tmp/x/33