[Buildroot] [PATCH 2/2] package/skeleton-init-systemd: flexible reinstalls

[Buildroot] [PATCH 2/2] package/skeleton-init-systemd: flexible reinstalls

[James Knight]

The following commit tweaks the `skeleton-init-systemd` package to
gracefully handle if a user invokes a reinstall for the package (e.g.
when performing development tweaks to the initial skeleton).

Signed-off-by: James Knight <james.d.knight@live.com>
---
Only a suggestion; feel free to drop.
---
 .../skeleton-init-systemd.mk                   | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
index 4076821c0c0429cf90681f4b16be114c44bde282..18ae91f2eea30277805ee82791c62d53f24dca1e 100644
--- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
+++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
@@ -18,13 +18,23 @@ SKELETON_INIT_SYSTEMD_PROVIDES = skeleton
 ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
 
 define SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW
-	echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab
+	if ! grep -q "/dev/root" $(TARGET_DIR)/etc/fstab; then \
+		echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab; \
+	else \
+		sed -i "s|/dev/root.*|/dev/root / auto rw 0 1|g" \
+			$(TARGET_DIR)/etc/fstab; \
+	fi
 endef
 
 else
 
 define SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW
-	echo "/dev/root / auto ro 0 1" >$(TARGET_DIR)/etc/fstab
+	if ! grep -q "/dev/root" $(TARGET_DIR)/etc/fstab; then \
+		echo "/dev/root / auto ro 0 1" >$(TARGET_DIR)/etc/fstab; \
+	else \
+		sed -i "s|/dev/root.*|/dev/root / auto ro 0 1|g" \
+			$(TARGET_DIR)/etc/fstab; \
+	fi
 endef
 
 # On a R/O rootfs, /var is a tmpfs filesystem. So, at build time, we
@@ -78,9 +88,9 @@ define SKELETON_INIT_SYSTEMD_INSTALL_TARGET_CMDS
 	mkdir -p $(TARGET_DIR)/home
 	mkdir -p $(TARGET_DIR)/srv
 	mkdir -p $(TARGET_DIR)/var
-	ln -s ../run $(TARGET_DIR)/var/run
+	ln -sf ../run $(TARGET_DIR)/var/run
 	# prevent install scripts to create var/lock as directory
-	ln -s ../run/lock $(TARGET_DIR)/var/lock
+	ln -sf ../run/lock $(TARGET_DIR)/var/lock
 	install -D -m644 $(SKELETON_INIT_SYSTEMD_PKGDIR)/legacy.conf $(TARGET_DIR)/usr/lib/tmpfiles.d/legacy.conf
 	$(SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW)
 endef
-- 
2.40.1.windows.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 2/2] package/skeleton-init-systemd: flexible reinstalls

[Yann E. MORIN]

James, All,

On 2023-05-05 00:30 -0400, James Knight spake thusly:
> The following commit tweaks the `skeleton-init-systemd` package to
> gracefully handle if a user invokes a reinstall for the package (e.g.
> when performing development tweaks to the initial skeleton).
> 
> Signed-off-by: James Knight <james.d.knight@live.com>
> ---
> Only a suggestion; feel free to drop.

I've spent a bit of time on this, and was about to apply...

However, there are a few "issues" I noticed with it, mostly that
/etc/fstab should not be used with systemd, but mount units should be
used, and so /etc/fstab should only contain this one line, so it does
not make much sense to have it "reinstallable".

Also, with BR2_PER_PACKAGE_DIRECTORIES=y, when the "final" target is
aggregated there is no guarantee that the modified file gets used in
this case. Indeed, assuming the following dependency chain (simplified
for the sake of the explanation):

    S <-- A <-- target/
     `<-- Z <--'

I.e. A depends on S (the skeleton), Z also depends on S, and the target
is aggreagated from A and Z, and S. When you first build, Z inherits the
fstab from S, and because it sorts after S, that the one that gets used.

If you rebuild S, Z will not get rebuild, so will not get the new fstab.
And thus when you assemble target after the rebuild, you still get the
old file from Z, not the new one from S.

(there are more complex cases, the above is just a trivial example).

Finally, the only guarantee Buildroot really offers is fof a full build
from scratch.

And last, reinstalling a skeleton package is very rare, so fixing it is
not worth the effort, considering all the above.

So I just decided to drop it.

Regards,
Yann E. MORIN.

> ---
>  .../skeleton-init-systemd.mk                   | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
> index 4076821c0c0429cf90681f4b16be114c44bde282..18ae91f2eea30277805ee82791c62d53f24dca1e 100644
> --- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
> +++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
> @@ -18,13 +18,23 @@ SKELETON_INIT_SYSTEMD_PROVIDES = skeleton
>  ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
>  
>  define SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW
> -	echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab
> +	if ! grep -q "/dev/root" $(TARGET_DIR)/etc/fstab; then \
> +		echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab; \
> +	else \
> +		sed -i "s|/dev/root.*|/dev/root / auto rw 0 1|g" \
> +			$(TARGET_DIR)/etc/fstab; \
> +	fi
>  endef
>  
>  else
>  
>  define SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW
> -	echo "/dev/root / auto ro 0 1" >$(TARGET_DIR)/etc/fstab
> +	if ! grep -q "/dev/root" $(TARGET_DIR)/etc/fstab; then \
> +		echo "/dev/root / auto ro 0 1" >$(TARGET_DIR)/etc/fstab; \
> +	else \
> +		sed -i "s|/dev/root.*|/dev/root / auto ro 0 1|g" \
> +			$(TARGET_DIR)/etc/fstab; \
> +	fi
>  endef
>  
>  # On a R/O rootfs, /var is a tmpfs filesystem. So, at build time, we
> @@ -78,9 +88,9 @@ define SKELETON_INIT_SYSTEMD_INSTALL_TARGET_CMDS
>  	mkdir -p $(TARGET_DIR)/home
>  	mkdir -p $(TARGET_DIR)/srv
>  	mkdir -p $(TARGET_DIR)/var
> -	ln -s ../run $(TARGET_DIR)/var/run
> +	ln -sf ../run $(TARGET_DIR)/var/run
>  	# prevent install scripts to create var/lock as directory
> -	ln -s ../run/lock $(TARGET_DIR)/var/lock
> +	ln -sf ../run/lock $(TARGET_DIR)/var/lock
>  	install -D -m644 $(SKELETON_INIT_SYSTEMD_PKGDIR)/legacy.conf $(TARGET_DIR)/usr/lib/tmpfiles.d/legacy.conf
>  	$(SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW)
>  endef
> -- 
> 2.40.1.windows.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 2/2] package/skeleton-init-systemd: flexible reinstalls

[Arnout Vandecappelle via buildroot]

On 01/10/2023 18:05, Yann E. MORIN wrote:
> James, All,
> 
> On 2023-05-05 00:30 -0400, James Knight spake thusly:
>> The following commit tweaks the `skeleton-init-systemd` package to
>> gracefully handle if a user invokes a reinstall for the package (e.g.
>> when performing development tweaks to the initial skeleton).
>>
>> Signed-off-by: James Knight <james.d.knight@live.com>
>> ---
>> Only a suggestion; feel free to drop.
> 
> I've spent a bit of time on this, and was about to apply...
> 
> However, there are a few "issues" I noticed with it, mostly that
> /etc/fstab should not be used with systemd, but mount units should be

  Really? The mount unit man page [1] says:

"In general, configuring mount points through /etc/fstab is the preferred 
approach to manage mounts for humans. For tooling, writing mount units should be 
preferred over editing /etc/fstab."

I'm not sure if the skeleton counts as tooling or as a human. Hm, probably 
tooling, indeed.


> used, and so /etc/fstab should only contain this one line, so it does

  Well, no, if we say that the skeleton is tooling, it shouldn't contain that 
line at all; instead, there should be a mount unit to remount rw (and no mount 
unit at all for ro).

> not make much sense to have it "reinstallable".
[snip]
>> diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
>> index 4076821c0c0429cf90681f4b16be114c44bde282..18ae91f2eea30277805ee82791c62d53f24dca1e 100644
>> --- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
>> +++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
>> @@ -18,13 +18,23 @@ SKELETON_INIT_SYSTEMD_PROVIDES = skeleton
>>   ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
>>   
>>   define SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW
>> -	echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab

  The original here _was_ already "reinstallable" because it just overwrites 
fstab. If there is any other package that writes fstab, it's wrong anyway. 
Unless it's done in the overlay or post-build-script, but the overlay/script 
anyway still gets applied after rebuild.

>> +	if ! grep -q "/dev/root" $(TARGET_DIR)/etc/fstab; then \
>> +		echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab; \
>> +	else \
>> +		sed -i "s|/dev/root.*|/dev/root / auto rw 0 1|g" \
>> +			$(TARGET_DIR)/etc/fstab; \
>> +	fi
>>   endef
>>   
>>   else
>>   
>>   define SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW
>> -	echo "/dev/root / auto ro 0 1" >$(TARGET_DIR)/etc/fstab
>> +	if ! grep -q "/dev/root" $(TARGET_DIR)/etc/fstab; then \
>> +		echo "/dev/root / auto ro 0 1" >$(TARGET_DIR)/etc/fstab; \
>> +	else \
>> +		sed -i "s|/dev/root.*|/dev/root / auto ro 0 1|g" \
>> +			$(TARGET_DIR)/etc/fstab; \
>> +	fi
>>   endef
>>   
>>   # On a R/O rootfs, /var is a tmpfs filesystem. So, at build time, we
>> @@ -78,9 +88,9 @@ define SKELETON_INIT_SYSTEMD_INSTALL_TARGET_CMDS
>>   	mkdir -p $(TARGET_DIR)/home
>>   	mkdir -p $(TARGET_DIR)/srv
>>   	mkdir -p $(TARGET_DIR)/var
>> -	ln -s ../run $(TARGET_DIR)/var/run
>> +	ln -sf ../run $(TARGET_DIR)/var/run

  IMHO this part is simply a good idea.

  Regards,
  Arnout

[1] https://www.freedesktop.org/software/systemd/man/systemd.mount.html



>>   	# prevent install scripts to create var/lock as directory
>> -	ln -s ../run/lock $(TARGET_DIR)/var/lock
>> +	ln -sf ../run/lock $(TARGET_DIR)/var/lock
>>   	install -D -m644 $(SKELETON_INIT_SYSTEMD_PKGDIR)/legacy.conf $(TARGET_DIR)/usr/lib/tmpfiles.d/legacy.conf
>>   	$(SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW)
>>   endef
>> -- 
>> 2.40.1.windows.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot@buildroot.org
>> https://lists.buildroot.org/mailman/listinfo/buildroot
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot