ci: build with -Werror on Fedora

Prompted by: https://github.com/systemd/systemd/pull/18400#issuecomment-771602705

.packit.yml

@@ -22,8 +22,18 @@ actions:
     # - Patch0000-0499: backported patches from upstream
     # - Patch0500-9999: downstream-only patches
     - "sed -ri '/^Patch0[0-4][0-9]{2}+\\:.+\\.patch/d' .packit_rpm/systemd.spec"
-    # Build the RPMs with -Werror to catch possible compiler warnings
+    # Build the RPMs with -Werror to catch possible compiler warnings. Since
-    - "sed -i 's/^%meson /%meson --werror /' .packit_rpm/systemd.spec"
+    # --werror in meson doesn't seem to work with -Db_lto=true [0], let's use
+    # -Dc_args= and -Dcpp_args= instead.
+    #
+    # Exceptions:
+    #   - use -Wno-deprecated-declarations to get around mallinfo() use in
+    #     basic/selinux-util.c
+    #   - don't use -Werror on x86 architectures, otherwise all function checks
+    #     will fail (with error: cast from pointer to integer of different size)
+    #
+    # [0] https://github.com/mesonbuild/meson/issues/7360
+    - 'sed -i "/^CONFIGURE_OPTS=(/a%ifnarch i386 i686\n-Dc_args=\"-Werror -Wno-deprecated-declarations\" -Dcpp_args=\"-Werror -Wno-deprecated-declarations\"\n%endif" .packit_rpm/systemd.spec'
 
 jobs:
 - job: copr_build

docs/PORTABILITY_AND_STABILITY.md

@@ -120,7 +120,7 @@ And now, here's the list of (hopefully) all APIs that we have introduced with sy
 | `/run` | File hierarchy change | yes | yes | numerous | yes | OpenSUSE, Debian, ArchLinux | no |
 | [Generators](https://www.freedesktop.org/software/systemd/man/systemd.generator.html) | Subprocess | yes | yes | - | no | - | no |
 | [System Updates](https://www.freedesktop.org/software/systemd/man/systemd.offline-updates.html) | System Mode | yes | yes | - | no | - | no |
-| [Presets](https://freedesktop.org/wiki/Software/systemd/Preset) | File format | yes | yes | - | no | - | no |
+| [Presets](https://www.freedesktop.org/software/systemd/man/systemd.preset.html) | File format | yes | yes | - | no | - | no |
 | Udev rules | File format | yes | yes | numerous | no | no | partially |
 
 

man/systemctl.xml

@@ -793,9 +793,7 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
 
             <para>For more information on the preset policy format, see
             <citerefentry><refentrytitle>systemd.preset</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
-            For more information on the concept of presets, please consult the
+            </para>
-            <ulink url="https://www.freedesktop.org/wiki/Software/systemd/Preset">Preset</ulink>
-            document.</para>
           </listitem>
         </varlistentry>
 

man/systemd.preset.xml

@@ -32,28 +32,20 @@
   <refsect1>
     <title>Description</title>
 
-    <para>Preset files may be used to encode policy which units shall
+    <para>Preset files may be used to encode policy which units shall be enabled by default and which ones
-    be enabled by default and which ones shall be disabled. They are
+    shall be disabled. They are read by <command>systemctl preset</command> which uses this information to
-    read by <command>systemctl preset</command> (for more information
+    enable or disable a unit. Depending on that policy, <command>systemctl preset</command> is identical to
-    see
+    <command>systemctl enable</command> or <command>systemctl disable</command>.
-    <citerefentry><refentrytitle>systemctl</refentrytitle><manvolnum>1</manvolnum></citerefentry>)
-    which uses this information to enable or disable a unit according
-    to preset policy. <command>systemctl preset</command> is used by
-    the post install scriptlets of RPM packages (or other OS package
-    formats), to enable/disable specific units by default on package
-    installation, enforcing distribution, spin or administrator preset
-    policy. This allows choosing a certain set of units to be
-    enabled/disabled even before installing the actual package.</para>
 
-    <para>For more information on the preset logic please have a look
+    <command>systemctl preset</command> is used by the post install scriptlets of rpm packages (or other OS
-    at the <ulink
+    package formats), to enable/disable specific units by default on package installation, enforcing
-    url="https://www.freedesktop.org/wiki/Software/systemd/Preset">Presets</ulink>
+    distribution, spin or administrator preset policy. This allows choosing a certain set of units to be
-    document.</para>
+    enabled/disabled even before installing the actual package. For more information, see
+    <citerefentry><refentrytitle>systemctl</refentrytitle><manvolnum>1</manvolnum></citerefentry>.</para>
 
-    <para>It is not recommended to ship preset files within the
+    <para>It is not recommended to ship preset files within the respective software packages implementing the
-    respective software packages implementing the units, but rather
+    units, but rather centralize them in a distribution or spin default policy, which can be amended by
-    centralize them in a distribution or spin default policy, which
+    administrator policy, see below.</para>
-    can be amended by administrator policy.</para>
 
     <para>If no preset files exist, <command>systemctl
     preset</command> will enable all units that are installed by
@@ -175,6 +167,38 @@ disable *</programlisting>
     override all other preset policy files.</para>
   </refsect1>
 
+  <refsect1>
+    <title>Motiviation for the preset logic</title>
+
+    <para>Different distributions have different policies on which services shall be enabled by default when
+    the package they are shipped in is installed. On Fedora all services stay off by default, so that
+    installing a package will not cause a service to be enabled (with some exceptions). On Debian all
+    services are immediately enabled by default, so that installing a package will cause its services to be
+    enabled right-away.</para>
+
+    <para>Even within a single distribution, different spins (flavours, remixes, whatever you might want to
+    call them) of a distribution also have different policies on what services to enable, and what services
+    to leave off. For example, Fedora Workstation will enable <command>gdm</command> as display manager by
+    default, while the Fedora KDE spin will enable <command>sddm</command> instead.</para>
+
+    <para>Different sites might also have different policies what to turn on by default and what to turn
+    off. For example, one administrator would prefer to enforce the policy of "<command>sshd</command> should
+    be always on, but everything else off", while another one might say "<command>snmpd</command> always on,
+    and for everything else use the distribution policy defaults".</para>
+
+    <para>Traditionally, policy about which services shall be enabled were implemented in each package
+    individually. This made it cumbersome to implement different policies per spin or per site, or to create
+    software packages that do the right thing on more than one distribution. The enablement mechanism was
+    also encoding the enablement policy.</para>
+
+    <para>The preset mechanism allows clean separation of the enablement mechanism (inside the package
+    scriptlets, by invoking <command>systemctl preset</command>) and enablement policy (centralized in the
+    preset files), and lifts the configuration out of individual packages. Preset files may be written for
+    specific distributions, for specific spins or for specific sites, in order to enforce different policies
+    as needed. It is recommended to apply the policy encoded in preset files in package installation
+    scriptlets.</para>
+  </refsect1>
+
   <refsect1>
     <title>See Also</title>
     <para>
@@ -182,6 +206,13 @@ disable *</programlisting>
       <citerefentry><refentrytitle>systemctl</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>systemd-delta</refentrytitle><manvolnum>1</manvolnum></citerefentry>
     </para>
+
+    <para><citerefentry><refentrytitle>daemon</refentrytitle><manvolnum>8</manvolnum></citerefentry>
+    has a discussion of packaging scriptlets.</para>
+
+    <para>Fedora page introducing the use of presets:
+    <ulink url="https://fedoraproject.org/wiki/Features/PackagePresets">Features/PackagePresets</ulink>.
+    </para>
   </refsect1>
 
 </refentry>

meson.build

@@ -1450,14 +1450,6 @@ have = have and conf.get('HAVE_PAM') == 1
 conf.set10('ENABLE_PAM_HOME', have)
 
 have = get_option('oomd')
-if have == 'auto'
-        have = get_option('mode') == 'developer'
-else
-        have = have == 'true'
-        if have and get_option('mode') != 'developer'
-                warning('oomd is not ready for release mode (yet)')
-        endif
-endif
 conf.set10('ENABLE_OOMD', have)
 substs.set10('ENABLE_OOMD', have)
 
@@ -3668,8 +3660,8 @@ status = [
                                                                         conf.get('SYSTEM_ALLOC_UID_MIN')),
         'system GIDs:                       <=@0@ (alloc >=@1@)'.format(conf.get('SYSTEM_GID_MAX'),
                                                                         conf.get('SYSTEM_ALLOC_GID_MIN')),
-        'dynamic UIDs:                      @0@–@1@'.format(dynamic_uid_min, dynamic_uid_max),
+        'dynamic UIDs:                      @0@…@1@'.format(dynamic_uid_min, dynamic_uid_max),
-        'container UID bases:               @0@–@1@'.format(container_uid_base_min, container_uid_base_max),
+        'container UID bases:               @0@…@1@'.format(container_uid_base_min, container_uid_base_max),
         '/dev/kvm access mode:              @0@'.format(get_option('dev-kvm-mode')),
         'render group access mode:          @0@'.format(get_option('group-render-mode')),
         'certificate root directory:        @0@'.format(get_option('certificate-root')),

meson_options.txt

@@ -99,7 +99,7 @@ option('coredump', type : 'boolean',
        description : 'install the coredump handler')
 option('pstore', type : 'boolean',
        description : 'install the pstore archival tool')
-option('oomd', type : 'combo', choices : ['auto', 'true', 'false'],
+option('oomd', type : 'boolean',
        description : 'install the userspace oom killer')
 option('logind', type : 'boolean',
        description : 'install the systemd-logind stack')

src/basic/path-util.c

@@ -823,6 +823,8 @@ const char *last_path_component(const char *path) {
          *    Also, the empty string is mapped to itself.
          *
          * This is different than basename(), which returns "" when a trailing slash is present.
+         *
+         * This always succeeds (except if you pass NULL in which case it returns NULL, too).
          */
 
         unsigned l, k;
@@ -848,24 +850,24 @@ const char *last_path_component(const char *path) {
 
 int path_extract_filename(const char *p, char **ret) {
         _cleanup_free_ char *a = NULL;
-        const char *c, *e = NULL, *q;
+        const char *c;
 
         /* Extracts the filename part (i.e. right-most component) from a path, i.e. string that passes
-         * filename_is_valid(). A wrapper around last_path_component(), but eats up trailing slashes. */
+         * filename_is_valid(). A wrapper around last_path_component(), but eats up trailing slashes. Returns
+         * -EADDRNOTAVAIL if specified parameter includes no filename (i.e. is "/" or so). Returns -EINVAL if
+         * not a valid path in the first place. */
 
-        if (!p)
+        if (!path_is_valid(p))
                 return -EINVAL;
 
+        /* Special case the root dir, because in that case we simply have no filename, but
+         * last_path_component() won't complain */
+        if (path_equal(p, "/"))
+                return -EADDRNOTAVAIL;
+
         c = last_path_component(p);
 
-        for (q = c; *q != 0; q++)
+        a = strndup(c, strcspn(c, "/"));
-                if (*q != '/')
-                        e = q + 1;
-
-        if (!e) /* no valid character? */
-                return -EINVAL;
-
-        a = strndup(c, e - c);
         if (!a)
                 return -ENOMEM;
 
@@ -873,7 +875,6 @@ int path_extract_filename(const char *p, char **ret) {
                 return -EINVAL;
 
         *ret = TAKE_PTR(a);
-
         return 0;
 }
 

src/kernel-install/meson.build

@@ -4,8 +4,7 @@ want_kernel_install = get_option('kernel-install')
 
 if want_kernel_install
         install_data('kernel-install',
-                     install_mode : 'rwxr-xr-x',
+                     install_mode : 'rwxr-xr-x')
-                     install_dir : bindir)
 
         install_data('00-entry-directory.install',
                      '50-depmod.install',

src/test/test-path-util.c

@@ -578,9 +578,9 @@ static void test_path_extract_filename(void) {
         test_path_extract_filename_one(NULL, NULL, -EINVAL);
         test_path_extract_filename_one("a/b/c", "c", 0);
         test_path_extract_filename_one("a/b/c/", "c", 0);
-        test_path_extract_filename_one("/", NULL, -EINVAL);
+        test_path_extract_filename_one("/", NULL, -EADDRNOTAVAIL);
-        test_path_extract_filename_one("//", NULL, -EINVAL);
+        test_path_extract_filename_one("//", NULL, -EADDRNOTAVAIL);
-        test_path_extract_filename_one("///", NULL, -EINVAL);
+        test_path_extract_filename_one("///", NULL, -EADDRNOTAVAIL);
         test_path_extract_filename_one(".", NULL, -EINVAL);
         test_path_extract_filename_one("./.", NULL, -EINVAL);
         test_path_extract_filename_one("././", NULL, -EINVAL);