- Rather than having one script per fix, use a single script (or "fixlet") per

(sub) package that contains all the fixups relative to a (sub) package. This
  has the advantage to limit the number of scripts but more importantly it will
  ease the sharing of the spec file between TW and SLE. We should also be able
  to compare the fixlets of two distros even if the spec files have diverged.
  Note that all the fixups are run just once now.

OBS-URL: https://build.opensuse.org/package/show/Base:System/systemd?expand=0&rev=1386

files.container

@@ -30,7 +30,7 @@
 %{_mandir}/man5/org.freedesktop.machine1.5.gz
 %{_mandir}/man8/systemd-machined.8.gz
 %{_mandir}/man8/systemd-machined.service.8.gz
-%{_systemd_util_dir}/rpm/fixlet-container-machines-btrfs-subvol.sh
+%{_systemd_util_dir}/rpm/fixlet-container-post.sh
 %{_systemd_util_dir}/systemd-machined
 %{_tmpfilesdir}/systemd-nspawn.conf
 %{_unitdir}/dbus-org.freedesktop.machine1.service

files.systemd

@@ -30,6 +30,7 @@
 %dir %{_libdir}/systemd
 %dir %{_localstatedir}/lib/systemd
 %dir %{_localstatedir}/lib/systemd/catalog
+%dir %{_localstatedir}/lib/systemd/rpm
 %dir %{_modprobedir}
 %dir %{_sysconfdir}/X11/xorg.conf.d
 %dir %{_sysconfdir}/binfmt.d
@@ -91,7 +92,6 @@
 %ghost %config(noreplace) %{_sysconfdir}/vconsole.conf
 %ghost %dir %attr(2755, root, systemd-journal) %{_localstatedir}/log/journal
 %ghost %{_localstatedir}/lib/systemd/catalog/database
-%ghost %{_localstatedir}/lib/systemd/i18n-migrated
 %license LICENSE.GPL2
 %license LICENSE.LGPL2.1
 %{_bindir}/busctl
@@ -440,8 +440,7 @@
 %{_sysctldir}/99-sysctl.conf
 %{_systemd_user_env_generator_dir}/30-systemd-environment-d-generator
 %{_systemd_util_dir}/ntp-units.d/80-systemd-timesync.list
-%{_systemd_util_dir}/rpm/fixlet-migrate-sysconfig-i18n.sh
+%{_systemd_util_dir}/rpm/fixlet-systemd-post.sh
-%{_systemd_util_dir}/rpm/fixlet-upgrade-from-sysvinit.sh
 %{_systemd_util_dir}/system-preset/99-default.preset
 %{_systemd_util_dir}/systemd
 %{_systemd_util_dir}/systemd-binfmt

fixlet-container-machines-btrfs-subvol.sh

@@ -1,139 +0,0 @@
-#! /bin/bash
-#
-# This is used to initially create /var/lib/machines subvolume in case
-# the system we're running on is using BTRFS with the specific layout
-# used by snapper to perform snapshots, rollbacks, etc...
-#
-# Unfortunately some distros (TW) already shipped versions with
-# systemd creating a plain subvolume which breaks snapper.
-#
-# If /var/lib/machines is already populated then it's going to be
-# pretty ugly to convert the old subvolume into a new one specially
-# since it can be in use.
-#
-# Hopefully not a lot of users are using machinectl to import
-# container/VM images. So in most of the cases this directory should
-# be empty and we can then simple delete the subvolume and create a
-# new one respecting the snapper layout.
-#
-# In the rare case where /var/lib/machines is populated, we will warn
-# the user and let him fix it manually.
-#
-# In order to avoid ugly dependencies added in systemd package, this
-# script should only be called during package updates when
-# mksubvolume(8) is available. During installation, /var/lib/machines
-# is supposed to be created by the installer now.
-#
-# See bsc#992573
-#
-
-warn() {
-	echo >&2 "warning: $@"
-}
-
-is_btrfs_subvolume() {
-	# On btrfs subvolumes always have the inode 256
-	test $(stat --format=%i "$1") -eq 256
-}
-
-# This assumes the directory/subvol is emptied by the caller.
-rm_subvolume_or_directory() {
-	is_btrfs_subvolume "$1" && {
-		btrfs subvolume delete "$1"
-		return
-	}
-	rmdir "$1"
-}
-
-on_exit() {
-	# Simply print a common error message in case something went
-	# wrong.
-	if test $? -ne 0; then
-		warn "Please fix /var/lib/machines manually."
-		# FIXME: point to a documentation explaining how to do
-		# that.
-		exit 1
-	fi
-}
-
-#
-# If there's already an entry in fstab for /var/lib/machines, it
-# means that:
-#
-#   - the installer initialized /var/lib/machines correctly (default)
-#   - we already fixed it
-#   - the sysadmin added it manually
-#
-# In any cases we should exit.
-#
-# Note: we can't simply check if /var/lib/machines has been mounted
-# because an update through a chroot might be in progress (see
-# bsc#1030290).
-#
-if mount --fake /var/lib/machines 2>/dev/null; then
-	exit
-fi
-
-#
-# If there is already an entry in fstab for /var, it means that:
-#
-#   - the system has a seperate /var subvolume (default from Feb 2018)
-#   - the system has a seperate /var partition
-#
-# In any case we should exit
-#
-if mount --fake /var 2>/dev/null; then
-	exit
-fi
-
-#
-# If something is already mounted don't try to fix anything, it's been
-# done manually by the sysadmin.
-#
-if mountpoint -q /var/lib/machines; then
-	exit
-fi
-
-#
-# Let's try to figure out if the current filesystem uses a Snapper
-# BTRFS specific layout. Note that TW uses a different layout than
-# SLE...
-#
-# FIXME: not sure if it's correct, reliable or optimal.
-#
-case $(findmnt -nr -t btrfs -o FSROOT / 2>/dev/null) in
-*.snapshots/*/snapshot*)
-	;;
-*)
-	exit 0
-esac
-
-trap on_exit EXIT
-
-if test -d /var/lib/machines; then
-	#
-	# Ok, we're on a system supporting rollbacks and
-	# /var/lib/machines is not a subvolume remotely mounted so it
-	# cannot be suitable for systems supporting rollback. Fix it.
-	#
-	echo "Making /var/lib/machines suitable for rollbacks..."
-
-	type mksubvolume >/dev/null 2>&1 || {
-		warn "mksubvolume(8) is not installed, aborting."
-		exit 1
-	}
-	test "$(ls -A /var/lib/machines/)" && {
-		warn "/var/lib/machines is not empty, aborting."
-		exit 1
-	}
-
-	echo "Deleting empty /var/lib/machines directory/subvolume"
-	rm_subvolume_or_directory /var/lib/machines || {
-		warn "fail to delete /var/lib/machines"
-		exit 1
-	}
-fi
-
-# At this point /var/lib/machines shouldn't exist.
-echo "Creating /var/lib/machines subvolume suitable for rollbacks."
-mksubvolume /var/lib/machines

fixlet-container-post.sh

@@ -0,0 +1,146 @@
+#! /bin/bash
+#
+# This script contains all the fixups run when systemd-container package is
+# installed or updated.
+#
+
+warn() {
+	echo >&2 "warning: $@"
+}
+
+is_btrfs_subvolume() {
+	# On btrfs subvolumes always have the inode 256
+	test $(stat --format=%i "$1") -eq 256
+}
+
+# This assumes the directory/subvol is emptied by the caller.
+rm_subvolume_or_directory() {
+	is_btrfs_subvolume "$1" && {
+		btrfs subvolume delete "$1"
+		return
+	}
+	rmdir "$1"
+}
+
+# On systems using BTRFS, convert /var/lib/machines into a subvolume suitable
+# for snapper to perform snapshots, rollbacks.. in case it was not properly set
+# up, see bsc#992573. The installer has been fixed to properly initialize it at
+# installation time.
+#
+# The conversion might only be problematic for openSUSE distros (TW/Factory)
+# where the subvolume was created at the wrong place (via tmpfiles for example)
+# and it got populated before we had time to fix it. In this case we'll let the
+# user fix it manually.
+#
+# On SLE12 this subvolume was only introduced during the upgrade from v210 to
+# v228 (ie SLE12-SP[01] -> SLE12-SP2+ when we added this workaround hence no
+# user should had time to populate it. Note that the subvolume is still created
+# at the wrong place due to the call to tmpfiles_create macro in the %post
+# section however it's empty so again we shouldn't face any issue to convert it.
+#
+# In order to avoid ugly dependencies added in systemd package, this function
+# should only be called during package updates when mksubvolume(8) is
+# available. During installation, /var/lib/machines is supposed to be created by
+# the installer now.
+#
+# See bsc#992573
+#
+fix_machines_subvol() {
+	local tagfile=/var/lib/systemd/rpm/container-machines_subvol
+
+	if [ -e $tagfile ]; then
+		return 0
+	fi
+	touch $tagfile
+
+	#
+	# If there's already an entry in fstab for /var/lib/machines, it
+	# means that:
+	#
+	#   - the installer initialized /var/lib/machines correctly (default)
+	#   - we already fixed it
+	#   - the sysadmin added it manually
+	#
+	# In any cases we should return.
+	#
+	# Note: we can't simply check if /var/lib/machines has been mounted
+	# because an update through a chroot might be in progress (see
+	# bsc#1030290).
+	#
+	if mount --fake /var/lib/machines 2>/dev/null; then
+		return
+	fi
+
+	#
+	# If there is already an entry in fstab for /var, it means that:
+	#
+	#   - the system has a seperate /var subvolume (default from Feb 2018)
+	#   - the system has a seperate /var partition
+	#
+	# In any case we should return.
+	#
+	if mount --fake /var 2>/dev/null; then
+		return
+	fi
+
+	#
+	# If something is already mounted don't try to fix anything, it's been
+	# done manually by the sysadmin.
+	#
+	if mountpoint -q /var/lib/machines; then
+		return
+	fi
+
+	#
+	# Let's try to figure out if the current filesystem uses a Snapper
+	# BTRFS specific layout. Note that TW uses a different layout than
+	# SLE...
+	#
+	# FIXME: not sure if it's correct, reliable or optimal.
+	#
+	case $(findmnt -nr -t btrfs -o FSROOT / 2>/dev/null) in
+	*.snapshots/*/snapshot*)
+		;;
+	*)
+		return 0
+	esac
+
+	if test -d /var/lib/machines; then
+		#
+		# Ok, we're on a system supporting rollbacks and
+		# /var/lib/machines is not a subvolume remotely mounted so it
+		# cannot be suitable for systems supporting rollback. Fix it.
+		#
+		echo "Making /var/lib/machines suitable for rollbacks..."
+
+		type mksubvolume >/dev/null 2>&1 || {
+			warn "mksubvolume(8) is not installed, aborting."
+			return 1
+		}
+		test "$(ls -A /var/lib/machines/)" && {
+			warn "/var/lib/machines is not empty, aborting."
+			return 1
+		}
+
+		echo "Deleting empty /var/lib/machines directory/subvolume"
+		rm_subvolume_or_directory /var/lib/machines || {
+			warn "fail to delete /var/lib/machines"
+			return 1
+		}
+	fi
+
+	# At this point /var/lib/machines shouldn't exist.
+	echo "Creating /var/lib/machines subvolume suitable for rollbacks."
+	mksubvolume /var/lib/machines
+}
+
+r=0
+if [ $1 -gt 1 ]; then
+	# During upgrade
+	fix_machines_subvol || {
+		warn "Please fix /var/lib/machines manually."
+		r=1
+	}
+fi
+
+exit $r

fixlet-systemd-post.sh

@@ -1,4 +1,8 @@
 #! /bin/bash
+#
+# This script contains all the fixups run when systemd package is installed or
+# updated.
+#
 
 # /etc/sysconfig/console   | /etc/vconsole.conf
 # -------------------------+---------------------
@@ -76,13 +80,12 @@ migrate_keyboard () {
 
 # According to
 # https://www.suse.com/documentation/sles-12/book_sle_admin/data/sec_suse_l10n.html,
-# variables in /etc/sysconfig/language are supposed to be passed to
+# variables in /etc/sysconfig/language are supposed to be passed to the users'
-# the users' shell *only*. However it seems that there has been some
+# shell *only*. However it seems that there has been some confusion and they
-# confusion and they ended up configuring the system-wide locale as
+# ended up configuring the system-wide locale as well.  The logic followed by
-# well.  The logic followed by systemd was implemented in commit
+# systemd was implemented in commit 01c4b6f4f0d951d17f6873f68156ecd7763429c6,
-# 01c4b6f4f0d951d17f6873f68156ecd7763429c6, which was reverted. The
+# which was reverted. The code below follows the same logic to migrate content
-# code below follows the same logic to migrate content of
+# of /etc/sysconfig/language into locale.conf.
-# /etc/sysconfig/language into locale.conf.
 migrate_language () {
 	local lang=
 	local migrated=false
@@ -128,18 +131,89 @@ migrate_language () {
 	fi
 }
 
+# Migrate old i18n settings previously configured in /etc/sysconfig to the new
+# locations used by systemd (/etc/locale.conf, /etc/vconsole.conf, ...). Recent
+# versions of systemd parse the new locations only.
+#
+# This is needed both at package updates and package installations because we
+# might be upgrading from a system which was running SysV init (systemd package
+# is being installed).
+#
+# It's run only once.
+migrate_sysconfig_i18n() {
+	local tagfile=/var/lib/systemd/rpm/systemd-i18n_migrated
+	local -i rv=0
 
-# The marker could have been incorrectly put in /usr/lib. In this case
+	if [ -e $tagfile ]; then
-# move it to its new place.
+		return 0
-mv /usr/lib/systemd/scripts/.migrate-sysconfig-i18n.sh~done \
+	fi
-   /var/lib/systemd/i18n-migrated &>/dev/null
 
-if ! test -e /var/lib/systemd/i18n-migrated; then
+	# The marker could have been incorrectly put in /usr/lib.
-	declare -i rv=0
+	mv /usr/lib/systemd/scripts/.migrate-sysconfig-i18n.sh~done $tagfile &>/dev/null
+	# The tag files have been moved to /var/lib/systemd/rpm later.
+	mv /var/lib/systemd/i18n-migrated $tagfile &>/dev/null
+
+	if [ -e $tagfile ]; then
+		return 0
+	fi
+	touch $tagfile
 
 	migrate_locale;   rv+=$?
 	migrate_keyboard; rv+=$?
 	migrate_language; rv+=$?
 
-	test $rv -eq 0 && touch /var/lib/systemd/i18n-migrated
+	return $rv
-fi
+}
+
+#
+# This function is supposed to be called from the %post section of the main
+# package. It contains all the fixups needed when the system was running a
+# version of systemd older than v210.
+#
+# All hacks can potentially break the admin settings since they work in /etc.
+#
+fix_pre_210() {
+	local tagfile=/var/lib/systemd/rpm/systemd-pre_210_fixed
+
+	if [ -e $tagfile ]; then
+		return 0
+	fi
+	touch $tagfile
+
+	#
+	# During migration from sysvinit to systemd, we used to set the systemd
+	# default target to one of the 'runlevel*.target' after reading the
+	# default runlevel from /etc/inittab. We don't do that anymore because
+	# in most cases using the graphical.target target, which is the default,
+	# will do the right thing. Moreover the runlevel targets are considered
+	# as deprecated, so we convert them into "true" systemd targets instead
+	# here.
+	#
+	if target=$(readlink /etc/systemd/system/default.target); then
+		target=$(basename $target)
+		case "$target" in
+		runlevel?.target)
+			echo "Default target is '$target' but use of runlevels is deprecated, converting"
+			systemctl --no-reload set-default $target
+		esac
+	fi
+
+	#
+	# Migrate any symlink which may refer to the old path (ie /lib/systemd).
+	#
+	for f in $(find /etc/systemd/system -type l -xtype l); do
+		new_target="/usr$(readlink $f)"
+		[ -f "$new_target" ] && ln -s -f "$new_target" "$f"
+	done
+}
+
+r=0
+fix_pre_210 || {
+	r=1
+}
+migrate_sysconfig_i18n || {
+	echo >&2 "Failed to migrate i18n settings from /etc/sysconfig, continuing..."
+	r=1
+}
+
+exit $r

fixlet-upgrade-from-sysvinit.sh

@@ -1,34 +0,0 @@
-#! /bin/bash
-
-#
-# This script is supposed to be executed from the %post section. It contains all
-# hacks needed to update a system which was running systemd < v210. This also
-# includes systems migrating from SysV.
-#
-# All hacks can potentially break the admin settings since they work in /etc...
-#
-
-#
-# During migration from sysvinit to systemd, we used to set the systemd default
-# target to one of the 'runlevel*.target' after reading the default runlevel
-# from /etc/inittab. We don't do that anymore because in most cases using the
-# graphical.target target, which is the default, will do the right
-# thing. Moreover the runlevel targets are considered as deprecated, so we
-# convert them into "true" systemd targets instead here.
-#
-if target=$(readlink /etc/systemd/system/default.target); then
-	target=$(basename $target)
-	case "$target" in
-	runlevel?.target)
-		echo "Default systemd target is '$target' but use of runlevels is deprecated"
-		systemctl --no-reload set-default $target
-	esac
-fi
-
-#
-# Migrate any symlink which may refer to the old path.
-#
-for f in $(find /etc/systemd/system -type l -xtype l); do
-        new_target="/usr$(readlink $f)"
-        [ -f "$new_target" ] && ln -s -f $new_target $f
-done

systemd.changes

@@ -1,3 +1,14 @@
+-------------------------------------------------------------------
+Thu May  4 15:20:49 UTC 2023 - Franck Bui <fbui@suse.com>
+
+- Rather than having one script per fix, use a single script (or "fixlet") per
+  (sub) package that contains all the fixups relative to a (sub) package. This
+  has the advantage to limit the number of scripts but more importantly it will
+  ease the sharing of the spec file between TW and SLE. We should also be able
+  to compare the fixlets of two distros even if the spec files have diverged.
+
+  Note that all the fixups are run just once now.
+
 -------------------------------------------------------------------
 Thu May  4 09:32:44 UTC 2023 - Franck Bui <fbui@suse.com>
 

systemd.spec

@@ -176,9 +176,8 @@ Source6:        baselibs.conf
 Source11:       after-local.service
 Source14:       kbd-model-map.legacy
 
-Source100:      fixlet-container-machines-btrfs-subvol.sh
+Source100:      fixlet-container-post.sh
-Source101:      fixlet-upgrade-from-sysvinit.sh
+Source101:      fixlet-systemd-post.sh
-Source102:      fixlet-migrate-sysconfig-i18n.sh
 
 Source200:      files.systemd
 Source201:      files.udev
@@ -772,7 +771,6 @@ mkdir -p %{buildroot}%{_systemd_util_dir}/rpm
 install -m0755 %{SOURCE100} %{buildroot}%{_systemd_util_dir}/rpm/
 %endif
 install -m0755 %{SOURCE101} %{buildroot}%{_systemd_util_dir}/rpm/
-install -m0755 %{SOURCE102} %{buildroot}%{_systemd_util_dir}/rpm/
 
 %if %{with split_usr}
 mkdir -p %{buildroot}/{bin,sbin}
@@ -843,6 +841,7 @@ mkdir -p %{buildroot}%{_sysconfdir}/X11/xorg.conf.d
 # Make sure directories in /var exist.
 mkdir -p %{buildroot}%{_localstatedir}/lib/systemd/coredump
 mkdir -p %{buildroot}%{_localstatedir}/lib/systemd/catalog
+mkdir -p %{buildroot}%{_localstatedir}/lib/systemd/rpm
 
 # Make sure the NTP units dir exists.
 mkdir -p %{buildroot}%{_ntpunitsdir}
@@ -887,7 +886,6 @@ touch %{buildroot}%{_sysconfdir}/vconsole.conf
 touch %{buildroot}%{_sysconfdir}/locale.conf
 touch %{buildroot}%{_sysconfdir}/machine-info
 touch %{buildroot}%{_localstatedir}/lib/systemd/catalog/database
-touch %{buildroot}%{_localstatedir}/lib/systemd/i18n-migrated
 
 %fdupes -s %{buildroot}%{_mandir}
 
@@ -1047,19 +1045,9 @@ if [ -L %{_localstatedir}/lib/systemd/timesync ]; then
         mv %{_localstatedir}/lib/private/systemd/timesync %{_localstatedir}/lib/systemd/timesync
 fi
 
-# This includes all hacks needed when upgrading from SysV.
+# Run the hacks/fixups to clean up old garbages left by (very) old versions of
-%{_systemd_util_dir}/rpm/fixlet-upgrade-from-sysvinit.sh || :
+# systemd.
-
+%{_systemd_util_dir}/rpm/fixlet-systemd-post.sh $1 || :
-# Migrate old i18n settings previously configured in /etc/sysconfig to the new
-# locations used by systemd (/etc/locale.conf, /etc/vconsole.conf, ...). Recent
-# versions of systemd parse the new locations only.
-#
-# This is needed both at package updates and package installations because we
-# might be upgrading from a system which was running SysV init (systemd package
-# is being installed).
-#
-# It's run only once.
-%{_systemd_util_dir}/rpm/fixlet-migrate-sysconfig-i18n.sh || :
 
 %postun
 # daemon-reload is implied by systemd_postun_with_restart
@@ -1133,30 +1121,17 @@ rm -f /etc/udev/rules.d/{20,55,65}-cdrom.rules
 %postun -n libudev%{?mini}1 -p %ldconfig
 %postun -n libsystemd0%{?mini} -p %ldconfig
 
-%pre container
-%systemd_pre machines.target
-
 %post container
-%systemd_post machines.target
 %tmpfiles_create systemd-nspawn.conf
 %if %{with machined}
+%systemd_post machines.target
 %ldconfig
-if [ $1 -gt 1 ]; then
+%endif
-        # Convert /var/lib/machines subvolume to make it suitable for rollbacks,
+%{_systemd_util_dir}/rpm/fixlet-container-post.sh $1 || :
-        # if needed. See bsc#992573. The installer has been fixed to create it
+
-        # at installation time.
+%if %{with machined}
-        #
+%pre container
-        # The conversion might only be problematic for openSUSE distros
+%systemd_pre machines.target
-        # (TW/Factory) where previous versions had already created the subvolume
-        # at the wrong place (via tmpfiles for example) and user started to
-        # populate and use it. In this case we'll let the user fix it manually.
-        #
-        # For SLE12 this subvolume was only introduced during the upgrade from
-        # v210 to v228 when we added this workaround. Note that the subvolume is
-        # still created at the wrong place due to the call to tmpfiles_create
-        # macro previously however it's empty so there shouldn't be any issues.
-        %{_systemd_util_dir}/rpm/fixlet-container-machines-btrfs-subvol.sh || :
-fi
 
 %preun container
 %systemd_preun machines.target
@@ -1166,6 +1141,14 @@ fi
 %ldconfig
 %endif
 
+%post container
+%tmpfiles_create systemd-nspawn.conf
+%if %{with machined}
+%systemd_post machines.target
+%ldconfig
+%endif
+%{_systemd_util_dir}/rpm/fixlet-container-post.sh $1 || :
+
 %if %{with coredump}
 %post coredump
 %sysusers_create systemd-coredump.conf