Re: [Buildroot] [PATCH v3 2/2] package/ipmitool: use versioned or local PEN registry

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Dario, All.,

On 2024-09-06 19:15 +0200, Dario Binacchi spake thusly:
> The previous default URL used for the PEN registry was not stable and
> could change at any time, making it unacceptable to have to update its
> hash every time.
> 
> With this patch, ipmitool now uses by default a versioned PEN (Enterprise
> Numbers) registry file from IANA provided by the iana-assignments package.
> Alternatively, it also allows the use of a local file. URL paths for
> downloading such a file are no longer supported.
> 
> Co-Developed-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Co-Developed-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
[--SNIP--]
> +choice
> +	prompt "PEN registry"
> +	default BR2_PACKAGE_IPMITOOL_USE_IANA_PEN
> +
> +config BR2_PACKAGE_IPMITOOL_USE_IANA_PEN
> +	bool "Using IANA PEN registry"
> +	select BR2_PACKAGE_IANA_ASSIGNMENTS
> +	select BR2_PACKAGE_IANA_ASSIGNMENTS_PEN_REG
> +
> +config BR2_PACKAGE_IPMITOOL_USE_CUSTOM_PEN
> +	bool "Using a custom PEN registry file"

This is supposed to be covered by the rootfs-overlay feature, in my
opinion.

So, I would really suggest we do like I explained in my previous review:
if the PEN is installed by ieana-assginments, the ipmi-tools uses it.
Otherwise, it is the responsibility of the user to provide one in the
proper location.

Which means the IANADIR must always be set, and a comment in the help
text for ipmitool should explain where th euser can install their custom
PEN in an overlay.

[--SNIP--]
> diff --git a/package/ipmitool/ipmitool.mk b/package/ipmitool/ipmitool.mk
> index 4f2151904d43..f16500739ce6 100644
> --- a/package/ipmitool/ipmitool.mk
> +++ b/package/ipmitool/ipmitool.mk
> @@ -49,20 +49,20 @@ endef
>  IPMITOOL_POST_INSTALL_TARGET_HOOKS += IPMITOOL_REMOVE_IPMIEVD
>  endif
>  
> -IPMITOOL_PEN_REG_URI = $(call qstrip,$(BR2_PACKAGE_IPMITOOL_PEN_REG_URI))
> -ifneq ($(IPMITOOL_PEN_REG_URI),)
> -ifneq ($(findstring ://,$(IPMITOOL_PEN_REG_URI)),)
> -IPMITOOL_EXTRA_DOWNLOADS += $(IPMITOOL_PEN_REG_URI)
> -BR_NO_CHECK_HASH_FOR += $(notdir $(IPMITOOL_PEN_REG_URI))
> -IPMITOOL_PEN_REG = $(IPMITOOL_DL_DIR)/$(notdir $(IPMITOOL_PEN_REG_URI))
> +ifeq ($(BR2_PACKAGE_IPMITOOL_USE_IANA_PEN),y)
> +IPMITOOL_DEPENDENCIES += iana-assignments

There is no need for a build-time dependency, as the file is only ever
needed at runtime, is there?

Regards,
Yann E. MORIN.

> +IPMITOOL_CONF_ENV += IANADIR=/usr/share/misc/iana
>  else
> -IPMITOOL_PEN_REG = $(IPMITOOL_PEN_REG_URI)
> +IPMITOOL_PEN_FILE = $(call qstrip,$(BR2_PACKAGE_IPMITOOL_USE_CUSTOM_PEN_FILE))
> +ifneq ($(IPMITOOL_PEN_FILE),)
> +ifneq ($(findstring ://,$(IPMITOOL_PEN_FILE)),)
> +$(error "URL paths are no supported")
>  endif #findstring
> -define IPMITOOL_INSTALL_PEN_REG
> -	$(INSTALL) -D -m 0644 $(IPMITOOL_PEN_REG) \
> +define IPMITOOL_INSTALL_PEN_FILE
> +	$(INSTALL) -D -m 0644 $(IPMITOOL_PEN_FILE) \
>  		$(TARGET_DIR)/usr/share/misc/enterprise-numbers
>  endef
> -IPMITOOL_POST_INSTALL_TARGET_HOOKS += IPMITOOL_INSTALL_PEN_REG
> -endif # IPMITOOL_PEN_REG_URI !empty
> -
> +IPMITOOL_POST_INSTALL_TARGET_HOOKS += IPMITOOL_INSTALL_PEN_FILE
> +endif # IPMITOOL_PEN_REG_FILEI !empty
> +endif # BR2_PACKAGE_IPMITOOL_USE_IANA_PEN
>  $(eval $(autotools-package))
> -- 
> 2.43.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot