package: Fix overwrite inittab w/ default skeleton

If you select a custom skeleton, and have busybox or sysvinit as init, the install of their inittab would trigger, rendering yours useless.

The hooks doing the sed in the skeleton package would only trigger for a default skeleton.

---
 package/busybox/busybox.mk   | 2 +-
 package/sysvinit/sysvinit.mk | 8 +++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

index 6b2abca..7c208d3 100644
@@ -148,7 +148,7 @@ define BUSYBOX_INSTALL_LOGGING_SCRIPT
 	else rm -f $(TARGET_DIR)/etc/init.d/S01logging; fi
 endef
 
-ifeq ($(BR2_INIT_BUSYBOX),y)
+ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT)$(BR2_INIT_BUSYBOX),yy)
 define BUSYBOX_INSTALL_INITTAB
 	$(INSTALL) -D -m 0644 package/busybox/inittab $(TARGET_DIR)/etc/inittab
 endef

index 2460dd9..34800d8 100644
@@ -30,14 +30,20 @@ define SYSVINIT_BUILD_CMDS
 	$(MAKE) $(TARGET_CONFIGURE_OPTS) SULOGINLIBS="-lcrypt" -C $(@D)/src
 endef
 
+ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT)$(BR2_INIT_SYSVINIT),yy)
+define SYSVINIT_INSTALL_INITTAB
+	$(INSTALL) -D -m 0644 package/sysvinit/inittab $(TARGET_DIR)/etc/inittab
+endef
+endif
+
 define SYSVINIT_INSTALL_TARGET_CMDS
 	for x in halt init shutdown killall5; do \
 		$(INSTALL) -D -m 0755 $(@D)/src/$$x $(TARGET_DIR)/sbin/$$x || exit 1; \
 	done
-	$(INSTALL) -D -m 0644 package/sysvinit/inittab $(TARGET_DIR)/etc/inittab
 	ln -sf /sbin/halt $(TARGET_DIR)/sbin/reboot
 	ln -sf /sbin/halt $(TARGET_DIR)/sbin/poweroff
 	ln -sf killall5 $(TARGET_DIR)/sbin/pidof
+	$(SYSVINIT_INSTALL_INITTAB)
 endef
 
 $(eval $(generic-package))

2.1.4

(for busybox)

Dear Maxime Hadjinlian,

Unfortunately, even if Gustavo gave his Acked-by, I'm tempted to not apply this patch. There are *tons* of other stuff that we don't install conditionally. For example, we used to install init scripts only if an init script was not already installed to allow a custom skeleton to provide such init scripts. But a while ago, we decided to get rid of this policy, and install init scripts and configuration files unconditionally. Just a few commits related to that:

649a6a64290ea214a034e5fb138d5c205eb44d98 mpd: install configuration file unconditionally f1557b855c57c2f0dd24230213b3875ae75ef341 inadyn: install configuration file unconditionally e07c97adcb55629dc0cc72aed2bb652f7af640a5 php: install configuration file unconditionally e3d6179bb767c4a6d726cf897d47a076892357db usbmount: install configuration file unconditionally 963bbf266bfebc240888cf8bce502a2fadb38c48 screen: install configuration file unconditionally 428f1e7e70c6bc410b12bbb6f8137eae1fffaa6e sysklogd: install configuration file unconditionally 5ad5d55924bcc7086f834a28db9c227371728470 ifplugd: install configuration files unconditionally 0a28ed55f84ac75ac1b3f2374ef0b10aafb61d7f logrotate: install configuration file unconditionally e0e56499354d41a3512b674004d84f9e82f98b15 samba: install configuration file unconditionally 0e18b0a4e12e739aed6eb892a246d3d27a007a38 proftpd: install configuration file unconditionally 82f82117eeebb2694565d26c1d76019823c7c057 fluxbox: install session file unconditionally 19c8fa96b5a1396c49abe21efda8a31b9a743816 input-event-daemon: install configuration file unconditionally c51ec898127767a28d366ab69a230dd382f361fc stunnel: install configuration file unconditionally aa6c7a8a994adad042668b569543e308318ecc5b transmission: install init script unconditionally 8a6482db88fc9f6076bc25f9b2b9defadc8fc809 rsyslog: install init script and config file unconditionally 1c8369ce3f9d3c3316b36885f9124e5ae89a1837 rpcbind: install init script unconditionally bb231c7c2fa1e5cd70372aa416a7c63e641e8b09 lighttpd: install configuration files with sensible (0644) permissions fa36ccd2f549b141c241bf2232dd56dedd218d9d lighttpd: install init script and config file unconditionally febe876dcc18938b43fe1012b0acc689176188ac iucode-tool: install init script unconditionally e31c6b961aff7e5018f8f5c72d938f1ad577c21f dmraid: install init script unconditionally de78a9896fb89b3d5cff18c592ac1a61e2db23bf busybox: install init script and config file unconditionally dc82dea73b878432bde1b2b164f4c746036fd772 aiccu: install init script and config file unconditionally

Our reasoning is that using a custom skeleton is highly discouraged, and people should instead use a rootfs overlay or a post-build script for their customization.

I don't see why inittab should be handled differently.

I'll mark your patch as Rejected for the time being. If there is some more arguments on why we should have a special handling of inittab, then we can rediscuss the patch of course.

Thanks!

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Actually i reported the bug to Maxime, he just forgot to add the Reported-By.
The problem with this is that it's a change in behaviour, this didn't happen in the previous releases.

My usage scenario is customized initscripts that rely on a very minimal inittab (rcS, rcK and getty, nothing more).
All of the basic startup is handled in initscripts, which i think we should do for BR as well - there's no reason to stick a lot of stuff in inittab IMO.
But even if we do that i'm still concerned about the behaviour change that can lead to non-working systems when upgrading buildroot. Another option is to just get rid of the custom skeleton altogether, if it's being stepped on right and left what's the purpose? Regards.

Hello,

Ok, thanks for the info.

Well, all the commits I cited in my previous reply definitely changed the behavior in a similar way, just for different files.

Possibly yes, though it's a different question. Even if the default inittab is more minimal, there are still lots of reasons for which you might need a custom version of it. So making the default version more minimal is kinda unrelated to whether we keep or overwrite the inittab of a custom skeleton.

My personal preference would be to ultimately get rid of the custom skeleton thing. We have been encouraging rootfs overlay and post-build scripts since quite a while in the Buildroot manual, see
http://buildroot.org/downloads/manual/manual.html#rootfs-custom:

"""
The two recommended methods, which can co-exist, are root filesystem overlay(s) and post build script(s).

Root filesystem overlays (BR2_ROOTFS_OVERLAY)

Post-build scripts (BR2_ROOTFS_POST_BUILD_SCRIPT)

[...]

Below two more methods of customizing the target filesystem are described, but they are not recommended.

Direct modification of the target filesystem

Custom target skeleton (BR2_ROOTFS_SKELETON_CUSTOM)

[...]

This method is not recommended because it duplicates the entire skeleton, which prevents taking advantage of the fixes or improvements brought to the default skeleton in later Buildroot releases. """

If were to not overwrite the inittab, we should also revert all of the commits I mentioned, and guarantee that the files in the custom skeleton would not be overwritten.

I agree with you that this is a change in behavior, but I would say it's a change for good: making people realize that using a custom skeleton very often doesn't work as they intend it to work.

Any opinions from others?

Thanks!

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

I agree that a custom skeleton is not the proper way to go and leads to more errors and trouble than anything.
I would remove the possibility entirely.
I can cook up a patch at that effect.
It would greatly disturb many user though. It needs to be well thought through.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150716/dd79fae8/attachment.html>

[snip]

I have one use case that still needs a custom skeleton (i.e. I found no other
way to handle it). There's a system where I want /bin to symlink to /usr/bin, to avoid some problems with wrong absolute paths in scripts. Turns out that doing that in a post-build script doesn't work because some other symlinks get broken, so it needs to be done in the skeleton...

Now, Peter wasn't against changing the default skeleton to do exactly that,
because it also solves the issue when busybox installs something in /bin while the full version installs it in /usr/bin. So in that case, yes we could probably eliminate the custom skeleton. Though there may be other cases.

Regards,
Arnout

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Hi.
Yes, albeit in less intrusive files.
Also not all of the packages did the check, we just made it consistently overwrite.

It's just an example, i might just as well configure "no init system" since part of the customization is nuking all of the BR-provided ones since i provide more feature-complete ones and it will definitely fix my problem - i'm not concerned about my corner case, it's just debate about what's the right thing to do.

Well, exactly what i'm going to, if we don't respect it why keep it? Going forward it's getting more overrides from new stuff, we don't recommend it and so on and on, maybe it's just time to kill it.

I was kind of joking with Maxime on IRC about defining an INSTALL_NO_CLOBBER to avoid checks on these things, it would avoid going all atomic on overwrites by using a custom install all along, maybe that would be a solution.
Regards.