diff mbox series

[3/6,v3] package/skeleton-systemd: host the tmpfiles preparation script

Message ID 14736_1666122197_634F01D5_14736_488_1_88b7ddd5c7fe78927e68e2f29ea9c9e61e0a782f.1666122184.git.yann.morin@orange.com
State Accepted
Headers show
Series [1/6,v3] package/skeleton-systemd: move /var factory tmpfiles out of /etc | expand

Commit Message

Yann E. MORIN Oct. 18, 2022, 7:43 p.m. UTC 
From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Commit 0d9b84b7a83f (package/systemd: invoke systemd-tmpfilesd on final
image) forcefully introduced a call to systemd-tmpfiles as a per-rootfs
hook, on the premise that would help with read-only rootfs.

However, that did not account for the then-pre-existing handling of /var
as a factory when the user opted not to remount / read-write (by not
setting BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW).

This means that, for users who want to use a factory for /var, the
generated filesystem contains the factory files twice: once as stored in
the factory, and once as populated by systemd-tmpefilesd.

In the hope to reconcile the two solutions, we move the handling of
calling systemd-tmpfilesd to the skeleton-init-systemd package, where we
already handle the /var factory. Having the two in the same package will
make it easier, in the future, to provide the user with a choice whether
to use one of the other.

Note that it is very important to keep the order of the hooks as they
are.

Indeed, skeleton-init-systemd sorts before systemd, so its hooks were
registered before systemd's hooks; now that we move the CREATE_TMPFILES
hook, we must ensure it is called after the PRE_ROOTFS_VAR one, so that
the behaviour of acting on the var factory remains.

As a final note: we chose the move this way, rather than move the var
factory into the systemd package, because it is more related to the
system integration on the Buildroot side, rather than the integration
of the systemd package in Buildroot.

Similarly, the other four rootfs hooks, SYSTEMD_LOCALE_PURGE_CATALOGS,
SYSTEMD_UPDATE_CATALOGS, SYSTEMD_RM_CATALOG_UPDATE_SERVICE, and
specially SYSTEMD_PRESET_ALL, should also be moved out of the systemd
package, because they too are more related to the Buildroot system,
rather than to the systemd package itself; but the frontier is very
porous is either way, for such a package as special as systemd.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Norbert Lange <nolange79@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Romain Naour <romain.naour@smile.fr>
Cc: Jérémy Rosen <jeremy.rosen@smile.fr>
Cc: Yann E. MORIN <yann.morin@orange.com>
---
 .../{systemd => skeleton-init-systemd}/fakeroot_tmpfiles.sh | 0
 package/skeleton-init-systemd/skeleton-init-systemd.mk      | 6 ++++++
 package/systemd/systemd.mk                                  | 6 ------
 3 files changed, 6 insertions(+), 6 deletions(-)
 rename package/{systemd => skeleton-init-systemd}/fakeroot_tmpfiles.sh (100%)

Comments

Norbert Lange Nov. 6, 2022, 4:04 p.m. UTC | #1 
Am Di., 18. Okt. 2022 um 21:43 Uhr schrieb <yann.morin@orange.com>:
>
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Commit 0d9b84b7a83f (package/systemd: invoke systemd-tmpfilesd on final
> image) forcefully introduced a call to systemd-tmpfiles as a per-rootfs
> hook, on the premise that would help with read-only rootfs.

It helps also with a read-only root, but the basic premise is to do as
much work as possible up-front.

>
> However, that did not account for the then-pre-existing handling of /var
> as a factory when the user opted not to remount / read-write (by not
> setting BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW).
>
> This means that, for users who want to use a factory for /var, the
> generated filesystem contains the factory files twice: once as stored in
> the factory, and once as populated by systemd-tmpefilesd.

To be pragmatic: a systemd conform package would ship
its own tmpfiles.conf or other mechanism to deal with an empty /var.

>
> In the hope to reconcile the two solutions, we move the handling of
> calling systemd-tmpfilesd to the skeleton-init-systemd package, where we
> already handle the /var factory. Having the two in the same package will
> make it easier, in the future, to provide the user with a choice whether
> to use one of the other.
>
> Note that it is very important to keep the order of the hooks as they
> are.
>
> Indeed, skeleton-init-systemd sorts before systemd, so its hooks were
> registered before systemd's hooks; now that we move the CREATE_TMPFILES
> hook, we must ensure it is called after the PRE_ROOTFS_VAR one, so that
> the behaviour of acting on the var factory remains.
>
> As a final note: we chose the move this way, rather than move the var
> factory into the systemd package, because it is more related to the
> system integration on the Buildroot side, rather than the integration
> of the systemd package in Buildroot.
>
> Similarly, the other four rootfs hooks, SYSTEMD_LOCALE_PURGE_CATALOGS,
> SYSTEMD_UPDATE_CATALOGS, SYSTEMD_RM_CATALOG_UPDATE_SERVICE, and
> specially SYSTEMD_PRESET_ALL, should also be moved out of the systemd
> package, because they too are more related to the Buildroot system,
> rather than to the systemd package itself; but the frontier is very
> porous is either way, for such a package as special as systemd.

I think that might make several things more ugly, atleast as long
as the config options are in the systemd package.

>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Norbert Lange <nolange79@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Romain Naour <romain.naour@smile.fr>
> Cc: Jérémy Rosen <jeremy.rosen@smile.fr>
> Cc: Yann E. MORIN <yann.morin@orange.com>
> ---
>  .../{systemd => skeleton-init-systemd}/fakeroot_tmpfiles.sh | 0
>  package/skeleton-init-systemd/skeleton-init-systemd.mk      | 6 ++++++
>  package/systemd/systemd.mk                                  | 6 ------
>  3 files changed, 6 insertions(+), 6 deletions(-)
>  rename package/{systemd => skeleton-init-systemd}/fakeroot_tmpfiles.sh (100%)
>
> diff --git a/package/systemd/fakeroot_tmpfiles.sh b/package/skeleton-init-systemd/fakeroot_tmpfiles.sh
> similarity index 100%
> rename from package/systemd/fakeroot_tmpfiles.sh
> rename to package/skeleton-init-systemd/fakeroot_tmpfiles.sh
> diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
> index 970951d553..89a28d1780 100644
> --- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
> +++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
> @@ -58,6 +58,12 @@ SKELETON_INIT_SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SKELETON_INIT_SYSTEMD_PRE_ROOTFS_V
>
>  endif
>
> +define SKELETON_INIT_SYSTEMD_CREATE_TMPFILES_HOOK
> +       HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
> +               $(SKELETON_INIT_SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
> +endef
> +SKELETON_INIT_SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SKELETON_INIT_SYSTEMD_CREATE_TMPFILES_HOOK
> +
>  define SKELETON_INIT_SYSTEMD_INSTALL_TARGET_CMDS
>         mkdir -p $(TARGET_DIR)/home
>         mkdir -p $(TARGET_DIR)/srv
> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
> index 1d7452de19..e4a8114221 100644
> --- a/package/systemd/systemd.mk
> +++ b/package/systemd/systemd.mk
> @@ -739,12 +739,6 @@ define SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>  endef
>  SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>
> -define SYSTEMD_CREATE_TMPFILES_HOOK
> -       HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
> -               $(SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
> -endef
> -SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_CREATE_TMPFILES_HOOK
> -
>  define SYSTEMD_PRESET_ALL
>         $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all
>  endef
> --
> 2.25.1
>
>
> _________________________________________________________________________________________________________________________
>
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
>
> This message and its attachments may contain confidential or privileged information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.
>

Acked-by: Norbert Lange <nolange79@gmail.com>
Yann E. MORIN Dec. 21, 2022, 9:18 p.m. UTC | #2 
Yann, All,

On 2022-10-18 21:43 +0200, yann.morin@orange.com spake thusly:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Commit 0d9b84b7a83f (package/systemd: invoke systemd-tmpfilesd on final
> image) forcefully introduced a call to systemd-tmpfiles as a per-rootfs
> hook, on the premise that would help with read-only rootfs.
> 
> However, that did not account for the then-pre-existing handling of /var
> as a factory when the user opted not to remount / read-write (by not
> setting BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW).
> 
> This means that, for users who want to use a factory for /var, the
> generated filesystem contains the factory files twice: once as stored in
> the factory, and once as populated by systemd-tmpefilesd.
> 
> In the hope to reconcile the two solutions, we move the handling of
> calling systemd-tmpfilesd to the skeleton-init-systemd package, where we
> already handle the /var factory. Having the two in the same package will
> make it easier, in the future, to provide the user with a choice whether
> to use one of the other.
> 
> Note that it is very important to keep the order of the hooks as they
> are.
> 
> Indeed, skeleton-init-systemd sorts before systemd, so its hooks were
> registered before systemd's hooks; now that we move the CREATE_TMPFILES
> hook, we must ensure it is called after the PRE_ROOTFS_VAR one, so that
> the behaviour of acting on the var factory remains.
> 
> As a final note: we chose the move this way, rather than move the var
> factory into the systemd package, because it is more related to the
> system integration on the Buildroot side, rather than the integration
> of the systemd package in Buildroot.
> 
> Similarly, the other four rootfs hooks, SYSTEMD_LOCALE_PURGE_CATALOGS,
> SYSTEMD_UPDATE_CATALOGS, SYSTEMD_RM_CATALOG_UPDATE_SERVICE, and
> specially SYSTEMD_PRESET_ALL, should also be moved out of the systemd
> package, because they too are more related to the Buildroot system,
> rather than to the systemd package itself; but the frontier is very
> porous is either way, for such a package as special as systemd.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Norbert Lange <nolange79@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Romain Naour <romain.naour@smile.fr>
> Cc: Jérémy Rosen <jeremy.rosen@smile.fr>
> Cc: Yann E. MORIN <yann.morin@orange.com>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  .../{systemd => skeleton-init-systemd}/fakeroot_tmpfiles.sh | 0
>  package/skeleton-init-systemd/skeleton-init-systemd.mk      | 6 ++++++
>  package/systemd/systemd.mk                                  | 6 ------
>  3 files changed, 6 insertions(+), 6 deletions(-)
>  rename package/{systemd => skeleton-init-systemd}/fakeroot_tmpfiles.sh (100%)
> 
> diff --git a/package/systemd/fakeroot_tmpfiles.sh b/package/skeleton-init-systemd/fakeroot_tmpfiles.sh
> similarity index 100%
> rename from package/systemd/fakeroot_tmpfiles.sh
> rename to package/skeleton-init-systemd/fakeroot_tmpfiles.sh
> diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
> index 970951d553..89a28d1780 100644
> --- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
> +++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
> @@ -58,6 +58,12 @@ SKELETON_INIT_SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SKELETON_INIT_SYSTEMD_PRE_ROOTFS_V
>  
>  endif
>  
> +define SKELETON_INIT_SYSTEMD_CREATE_TMPFILES_HOOK
> +	HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
> +		$(SKELETON_INIT_SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
> +endef
> +SKELETON_INIT_SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SKELETON_INIT_SYSTEMD_CREATE_TMPFILES_HOOK
> +
>  define SKELETON_INIT_SYSTEMD_INSTALL_TARGET_CMDS
>  	mkdir -p $(TARGET_DIR)/home
>  	mkdir -p $(TARGET_DIR)/srv
> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
> index 1d7452de19..e4a8114221 100644
> --- a/package/systemd/systemd.mk
> +++ b/package/systemd/systemd.mk
> @@ -739,12 +739,6 @@ define SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>  endef
>  SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>  
> -define SYSTEMD_CREATE_TMPFILES_HOOK
> -	HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
> -		$(SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
> -endef
> -SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_CREATE_TMPFILES_HOOK
> -
>  define SYSTEMD_PRESET_ALL
>  	$(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all
>  endef
> -- 
> 2.25.1
> 
> 
> _________________________________________________________________________________________________________________________
> 
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
diff mbox series

Patch

diff --git a/package/systemd/fakeroot_tmpfiles.sh b/package/skeleton-init-systemd/fakeroot_tmpfiles.sh
similarity index 100%
rename from package/systemd/fakeroot_tmpfiles.sh
rename to package/skeleton-init-systemd/fakeroot_tmpfiles.sh
diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
index 970951d553..89a28d1780 100644
--- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
+++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
@@ -58,6 +58,12 @@  SKELETON_INIT_SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SKELETON_INIT_SYSTEMD_PRE_ROOTFS_V
 
 endif
 
+define SKELETON_INIT_SYSTEMD_CREATE_TMPFILES_HOOK
+	HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
+		$(SKELETON_INIT_SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
+endef
+SKELETON_INIT_SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SKELETON_INIT_SYSTEMD_CREATE_TMPFILES_HOOK
+
 define SKELETON_INIT_SYSTEMD_INSTALL_TARGET_CMDS
 	mkdir -p $(TARGET_DIR)/home
 	mkdir -p $(TARGET_DIR)/srv
diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
index 1d7452de19..e4a8114221 100644
--- a/package/systemd/systemd.mk
+++ b/package/systemd/systemd.mk
@@ -739,12 +739,6 @@  define SYSTEMD_RM_CATALOG_UPDATE_SERVICE
 endef
 SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE
 
-define SYSTEMD_CREATE_TMPFILES_HOOK
-	HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
-		$(SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
-endef
-SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_CREATE_TMPFILES_HOOK
-
 define SYSTEMD_PRESET_ALL
 	$(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all
 endef