tmpfiles: make "f" lines behaviour match what the documentation says

CHANGE OF BEHAVIOUR — with this commit "f" line's behaviour is altered
to match what the documentation says: if an "argument" string is
specified it is written to the file only when the file didn't exist
before. Previously, it would be appended to the file each time
systemd-tmpfiles was invoked — which is not a particularly useful
behaviour as the tool is not idempotent then and the indicated files
grow without bounds each time the tool is invoked.

I did some spelunking whether this change in behaviour would break
things, but afaics nothing relies on the previous O_APPEND behaviour of
this line type, hence I think it's relatively safe to make "f" lines
work the way the docs say, rather than adding a new modifier for it or
so.

Triggered by:

https://lists.freedesktop.org/archives/systemd-devel/2018-January/040171.html

diff --git a/NEWS b/NEWS
index 46b8e12fb9ccaa989e05b0da8290cc9f20cf3144..70014888db8540f857aec3574634135e9e3061e1 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,18 @@ CHANGES WITH 237 in spe:
           keycodes are not recognized by any major desktop. They now produce
           Up/Down key events so that they can be used for scrolling.
 
+        * INCOMPATIBILITY: systemd-tmpfiles' "f" lines changed behaviour
+          slightly: previously, if an argument was specified for lines of this
+          type (i.e. the right-most column was set) this string was appended to
+          existing files each time systemd-tmpfiles was run. This behaviour was
+          different from what the documentation said, and not particularly
+          useful, as repeated systemd-tmpfiles invocations would not be
+          idempotent and grow such files without bounds. With this release
+          behaviour has been altered slightly, to match what the documentation
+          says: lines of this type only have an effect if the indicated files
+          don't exist yet, and only then the argument string is written to the
+          file.
+
         — Berlin, 2018-XX-XX
 
 CHANGES WITH 236:

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 30aa886388f1169b8aa3eda3a1fb8b923a6fb81e..c3a43d36e43dff13817922e20f43478a2bc19c01 100644 (file)
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -147,9 +147,8 @@ L     /tmp/foobar -    -    -    -   /dev/null</programlisting>
       <variablelist>
         <varlistentry>
           <term><varname>f</varname></term>
-          <listitem><para>Create a file if it does not exist yet. If
-          the argument parameter is given, it will be written to the
-          file. Does not follow symlinks.</para></listitem>
+          <listitem><para>Create a file if it does not exist yet. If the argument parameter is given and the file did
+          not exist yet, it will be written to the file. Does not follow symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -585,21 +584,14 @@ r! /tmp/.X[0-9]*-lock</programlisting>
     <refsect2>
       <title>Argument</title>
 
-      <para>For <varname>L</varname> lines determines the destination
-      path of the symlink. For <varname>c</varname> and
-      <varname>b</varname>, determines the major/minor of the device
-      node, with major and minor formatted as integers, separated by
-      <literal>:</literal>, e.g.  <literal>1:3</literal>. For
-      <varname>f</varname>, <varname>F</varname>, and
-      <varname>w</varname>, the argument may be used to specify a short string that
-      is written to the file, suffixed by a newline. For
-      <varname>C</varname>, specifies the source file or
-      directory. For <varname>t</varname> and <varname>T</varname>,
-      determines extended attributes to be set. For
-      <varname>a</varname> and <varname>A</varname>, determines ACL
-      attributes to be set. For <varname>h</varname> and
-      <varname>H</varname>, determines the file attributes to
-      set. Ignored for all other lines.</para>
+      <para>For <varname>L</varname> lines determines the destination path of the symlink. For <varname>c</varname> and
+      <varname>b</varname>, determines the major/minor of the device node, with major and minor formatted as integers,
+      separated by <literal>:</literal>, e.g.  <literal>1:3</literal>. For <varname>f</varname>, <varname>F</varname>,
+      and <varname>w</varname>, the argument may be used to specify a short string that is written to the file,
+      suffixed by a newline. For <varname>C</varname>, specifies the source file or directory. For <varname>t</varname>
+      and <varname>T</varname>, determines extended attributes to be set. For <varname>a</varname> and
+      <varname>A</varname>, determines ACL attributes to be set. For <varname>h</varname> and <varname>H</varname>,
+      determines the file attributes to set. Ignored for all other lines.</para>
 
       <para>This field can contain specifiers, see below.</para>
     </refsect2>

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 4d8c36870c8205196e2763d8b70aee3632465d0d..38cbb739c01f826dd3abd7ee11d125e057897a23 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1189,7 +1189,7 @@ static int write_one_file(Item *i, const char *path) {
         assert(i);
         assert(path);
 
-        flags = i->type == CREATE_FILE ? O_CREAT|O_APPEND|O_NOFOLLOW :
+        flags = i->type == CREATE_FILE ? O_CREAT|O_EXCL|O_NOFOLLOW :
                 i->type == TRUNCATE_FILE ? O_CREAT|O_TRUNC|O_NOFOLLOW : 0;
 
         RUN_WITH_UMASK(0000) {
@@ -1200,9 +1200,13 @@ static int write_one_file(Item *i, const char *path) {
 
         if (fd < 0) {
                 if (i->type == WRITE_FILE && errno == ENOENT) {
-                        log_debug_errno(errno, "Not writing \"%s\": %m", path);
+                        log_debug_errno(errno, "Not writing missing file \"%s\": %m", path);
                         return 0;
                 }
+                if (i->type == CREATE_FILE && errno == EEXIST) {
+                        log_debug_errno(errno, "Not writing to pre-existing file \"%s\": %m", path);
+                        goto done;
+                }
 
                 r = -errno;
                 if (!i->argument && errno == EROFS && stat(path, &st) == 0 &&
@@ -1223,6 +1227,7 @@ static int write_one_file(Item *i, const char *path) {
 
         fd = safe_close(fd);
 
+done:
         if (stat(path, &st) < 0)
                 return log_error_errno(errno, "stat(%s) failed: %m", path);