Re: [Buildroot] [PATCH] boot/uboot: add BR2_TARGET_UBOOT_NEEDS_VIM option

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

Hello Jerry,

On Tue, 8 Nov 2022 08:43:07 -0600
Jerry Kooyman via buildroot <buildroot@buildroot.org> wrote:

> From: Jerry Kooyman <jerry.kooyman@entrust.com>
> 
> A host dependency to vim is required if the
> U-Boot board configuration has CONFIG_USE_DEFAULT_ENV_FILE enabled. So
> introduce a new BR U-Boot config option BR2_TARGET_UBOOT_NEEDS_VIM
> to solve this problem.

Thanks for the patch! First of all, in order for your patch to be
merged, we need it to carry a Signed-off-by line with your name and
email, like this:

Signed-off-by: Your Name <your.email@domain.com>

as the last line of your commit log.

> +config BR2_TARGET_UBOOT_NEEDS_VIM
> +	bool "U-Boot needs vim"

I am wondering if we shouldn't make this:

config BR2_TARGET_UBOOT_NEEDS_XXD
	bool "U-Boot needs xxd"

because really what U-Boot needs is xxd... which happens to be provided
by host-vim.

Indeed, if I see U-Boot failing to build because xxd is missing, if I'm
not familiar with this, I will have hard time realizing that what is
needed is this BR2_TARGET_UBOOT_NEEDS_VIM option.

In order to avoid building host-vim, which needs host-ncurses, we could
also package a standalone version of xxd, such as
https://github.com/ckormanyos/xxd. But that clearly isn't a
requirement, I will be happy to take a modified patch from you that has
the option renamed to BR2_TARGET_UBOOT_NEEDS_XXD.

Thanks a lot!

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