[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Maxime Hadjinlian]

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.

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
 package/busybox/busybox.mk   | 2 +-
 package/sysvinit/sysvinit.mk | 8 +++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 6b2abca..7c208d3 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -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
diff --git a/package/sysvinit/sysvinit.mk b/package/sysvinit/sysvinit.mk
index 2460dd9..34800d8 100644
--- a/package/sysvinit/sysvinit.mk
+++ b/package/sysvinit/sysvinit.mk
@@ -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

[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Gustavo Zacarias]

On 16/07/15 13:30, Maxime Hadjinlian wrote:

> 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.
>
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>

Acked-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
Tested-by: Gustavo Zacarias <gustavo@zacarias.com.ar>

(for busybox)

[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Thomas Petazzoni]

Dear Maxime Hadjinlian,

On Thu, 16 Jul 2015 18:30:35 +0200, Maxime Hadjinlian wrote:
> 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.
> 
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> ---
>  package/busybox/busybox.mk   | 2 +-
>  package/sysvinit/sysvinit.mk | 8 +++++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)

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

[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Gustavo Zacarias]

On 16/07/15 18:07, Thomas Petazzoni wrote:

> 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:

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.

> 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.

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.

[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Thomas Petazzoni]

Hello,

On Thu, 16 Jul 2015 18:17:21 -0300, Gustavo Zacarias wrote:

> Actually i reported the bug to Maxime, he just forgot to add the 
> Reported-By.

Ok, thanks for the info.

> The problem with this is that it's a change in behaviour, this didn't 
> happen in the previous releases.

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

> 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.

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.

> 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?

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

[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Maxime Hadjinlian]

On Jul 16, 2015 11:35 PM, "Thomas Petazzoni" <
thomas.petazzoni@free-electrons.com> wrote:
>
> Hello,
>
> On Thu, 16 Jul 2015 18:17:21 -0300, Gustavo Zacarias wrote:
>
> > Actually i reported the bug to Maxime, he just forgot to add the
> > Reported-By.
>
> Ok, thanks for the info.
>
> > The problem with this is that it's a change in behaviour, this didn't
> > happen in the previous releases.
>
> Well, all the commits I cited in my previous reply definitely changed
> the behavior in a similar way, just for different files.
>
> > 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.
>
> 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.
>
> > 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?
>
> 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?
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.
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150716/dd79fae8/attachment.html>

[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Arnout Vandecappelle]

On 07/16/15 23:42, Maxime Hadjinlian wrote:
> 
> On Jul 16, 2015 11:35 PM, "Thomas Petazzoni"
> <thomas.petazzoni@free-electrons.com
> <mailto:thomas.petazzoni@free-electrons.com>> wrote:
[snip]
>> 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?
> 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 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

> I can cook up a patch at that effect.
> It would greatly disturb many user though. It needs to be well thought through.
>>
>> Thanks!
>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Free Electrons
>> Embedded Linux, Kernel and Android engineering
>> http://free-electrons.com
> 
> 
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 


-- 
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

[Buildroot] [PATCH] package: Fix overwrite inittab w/ default skeleton

[Gustavo Zacarias]

On 16/07/15 18:35, Thomas Petazzoni wrote:

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

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

>> 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.
>
> 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.

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.

> 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:

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.

> 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?

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.