All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: Damien Le Moal <dlemoal@kernel.org>,
	linux-amarula@amarulasolutions.com,
	"Yann E . MORIN" <yann.morin.1998@free.fr>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH v3 1/3] package/tinyinit: new package
Date: Fri, 23 Aug 2024 18:17:41 +0200	[thread overview]
Message-ID: <20240823181741.499173f2@windsurf> (raw)
In-Reply-To: <20240822183742.3550055-2-dario.binacchi@amarulasolutions.com>

On Thu, 22 Aug 2024 20:37:40 +0200
Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote:

>  DEVELOPERS                   |  1 +
>  package/Config.in            |  1 +
>  package/tinyinit/Config.in   | 10 ++++++++++
>  package/tinyinit/init        | 32 ++++++++++++++++++++++++++++++++
>  package/tinyinit/tinyinit.mk | 12 ++++++++++++
>  5 files changed, 56 insertions(+)

We also need to change busybox.mk to ensure that tinyinit is built
before Busybox:

# Packages that provide commands that may also be busybox applets:
BUSYBOX_DEPENDENCIES = \
	...
	$(if $(BR2_PACKAGE_TINYINIT),tinyinit) \


> diff --git a/package/tinyinit/Config.in b/package/tinyinit/Config.in
> new file mode 100644
> index 000000000000..2ceb8e191e23
> --- /dev/null
> +++ b/package/tinyinit/Config.in
> @@ -0,0 +1,10 @@
> +config BR2_PACKAGE_TINYINIT
> +	bool "tinyinit"
> +	depends on BR2_INIT_NONE
> +	help
> +	  A Linux tiny initialization script suitable for resource
> +	  limited systems, which can be used as an alternative to the
> +	  one provided by Busybox.
> +
> +comment "tinyinit needs BR2_INIT_NONE, i. e. no init system installed"
> +	depends on !BR2_INIT_NONE

I find this a bit odd. In the end, shouldn't we simply promote tinyinit
as an init implementation, and have its own BR2_INIT_TINYINIT entry?

Also, should BR2_PACKAGE_TINYINIT select busybox? Or do we pretend that
it is possible to use it with some other implementations of mount and
other basic commands? Perhaps this is a good case to use "imply
BR2_PACKAGE_BUSYBOX", so that BR2_PACKAGE_TINYINIT enables busybox by
default, but still allows removing busybox if really the user knows
what (s)he is doing?

> +define TINYINIT_INSTALL_TARGET_CMDS
> +	$(INSTALL) -m 0755 -D $(TINYINIT_PKGDIR)/init $(TARGET_DIR)/sbin/init
> +	(cd $(TARGET_DIR); ln -sf /sbin/init init)

Hmm. I initially thought this symlink shouldn't be needed, as it's
created by fs/cpio/cpio.mk when you're building a cpio for initramfs:

ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)

define ROOTFS_CPIO_ADD_INIT
        if [ ! -e $(TARGET_DIR)/init ]; then \
                ln -sf sbin/init $(TARGET_DIR)/init; \
        fi
endef

else
# devtmpfs does not get automounted when initramfs is used.
# Add a pre-init script to mount it before running init
# We must have /dev/console very early, even before /init runs,
# for stdin/stdout/stderr
define ROOTFS_CPIO_ADD_INIT
        if [ ! -e $(TARGET_DIR)/init ]; then \
                $(INSTALL) -m 0755 fs/cpio/init $(TARGET_DIR)/init; \
        fi
        mkdir -p $(TARGET_DIR)/dev
        mknod -m 0622 $(TARGET_DIR)/dev/console c 5 1
endef

But actually, in the case of a non-static /dev, it doesn't do a
symlink, but adds a pre-init script, and your tinyinit already does the
tasks of this pre-init provided by fs/cpio/init.

So I guess what you did is probably OK, but maybe a comment would be
useful. The only downside of your approach is that it leaves a /init
symlink that isn't used/needed for non-initramfs use-cases, but perhaps
that's OK.


Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  reply	other threads:[~2024-08-23 16:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-22 18:37 [Buildroot] [PATCH v3 0/3] tinyinit and stm32f746_disco_sd_defconfig Dario Binacchi
2024-08-22 18:37 ` [Buildroot] [PATCH v3 1/3] package/tinyinit: new package Dario Binacchi
2024-08-23 16:17   ` Thomas Petazzoni via buildroot [this message]
2024-08-25 15:06     ` Dario Binacchi
2024-08-27 20:07     ` Yann E. MORIN
2024-08-29 21:47       ` Thomas Petazzoni via buildroot
2024-08-30  7:32         ` Yann E. MORIN
2024-08-22 18:37 ` [Buildroot] [PATCH v3 2/3] configs/stm32f746_disco_sd: new defconfig Dario Binacchi
2024-08-23 16:20   ` Thomas Petazzoni via buildroot
2024-08-25 15:23     ` Dario Binacchi
2024-08-22 18:37 ` [Buildroot] [PATCH v3 3/3] board/canaan/k210-soc: use tinyinit as Linux init process Dario Binacchi
2024-08-23 16:11 ` [Buildroot] [PATCH v3 0/3] tinyinit and stm32f746_disco_sd_defconfig Thomas Petazzoni via buildroot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240823181741.499173f2@windsurf \
    --to=buildroot@buildroot.org \
    --cc=dario.binacchi@amarulasolutions.com \
    --cc=dlemoal@kernel.org \
    --cc=linux-amarula@amarulasolutions.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=yann.morin.1998@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.