Re: [Buildroot] [PATCH 8/8] tipidee: new package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello,

On Thu, 04 Jul 2024 21:40:34 +0200
J. Neuschäfer via buildroot <buildroot@buildroot.org> wrote:

> Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>

Thanks for this contribution!

> +if BR2_PACKAGE_TIPIDEE
> +
> +config BR2_PACKAGE_TIPIDEE_CONFIGURATION
> +	bool "install a configuration file to /etc/tipidee.conf.cdb"
> +	select BR2_PACKAGE_HOST_TIPIDEE
> +	help
> +	  Build a tipidee configuration and install it to the target.
> +
> +config BR2_PACKAGE_TIPIDEE_CONFIGURATION_FILE
> +	string "configuration source file"
> +	default "$(CONFIG_DIR)/tipidee.conf"
> +	depends on BR2_PACKAGE_TIPIDEE_CONFIGURATION
> +	help
> +	  The configuration file in source (text) format, documented at:
> +
> +	  https://skarnet.org/software/tipidee/tipidee.conf.html

I'm not sure those configuration options are really needed. We usually
don't make the configuration file configurable, and leave that up to a
rootfs-overlay. I understand that the configuration is apparently
"compiled", but I would perhaps leave that up to be overridden by a
post-build script? Or perhaps we should do this configuration file
"compilation" as a TARGET_FINALIZE_HOOKS so that it happens after
rootfs-overlay are copied, allowing the user to override the
configuration file, while still seeing it "compiled"?

> diff --git a/package/tipidee/Config.in.host b/package/tipidee/Config.in.host
> new file mode 100644
> index 0000000000..93e548df1e
> --- /dev/null
> +++ b/package/tipidee/Config.in.host
> @@ -0,0 +1,8 @@
> +config BR2_HOST_PACKAGE_TIPIDEE
> +	bool "host tipidee"
> +	depends on BR2_USE_MMU # skalibs
> +	select BR2_PACKAGE_SKALIBS
> +	help
> +	  This package builds the tipidee-config tool for the host.
> +
> +	  https://skarnet.org/software/tipidee/

I'm not sure we really need a visible config option for this,
host-tipidee is going to be a dependency of tipidee anyway.

> diff --git a/package/tipidee/tipidee.mk b/package/tipidee/tipidee.mk
> new file mode 100644
> index 0000000000..2fde75b87d
> --- /dev/null
> +++ b/package/tipidee/tipidee.mk
> @@ -0,0 +1,72 @@
> +################################################################################
> +#
> +# tipidee
> +#
> +################################################################################
> +
> +TIPIDEE_VERSION = 0.0.5.0
> +TIPIDEE_SITE = https://skarnet.org/software/tipidee
> +TIPIDEE_LICENSE = ISC
> +TIPIDEE_LICENSE_FILES = COPYING
> +TIPIDEE_INSTALL_STAGING = YES
> +TIPIDEE_DEPENDENCIES = skalibs
> +
> +TIPIDEE_CONF_OPTS = \
> +	--prefix=/ \

This looks odd, why not --prefix=/usr ?

The rest really looks super reasonable to me, good work!

Best regards,

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