[v3,2/2] package/ipmitool: use versioned or local PEN registry
diff mbox series

Message ID 20240906171503.3495498-2-dario.binacchi@amarulasolutions.com
State New
Headers show
Series
  • [v3,1/2] package/iana-assignments: new package
Related show

Commit Message

Dario Binacchi Sept. 6, 2024, 5:15 p.m. UTC 
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>
---
Changes v2 -> v3:
  - Use by default enterprise-numbers from iana-assignments package.
  - Alternatively allows to use a local PEN registry file.
  - Don't support URL path for PEN registry.

Changes v1 -> v2:
  - use an enterprise-numbers file versioned

 package/ipmitool/Config.in   | 32 ++++++++++++++++++++++++++------
 package/ipmitool/ipmitool.mk | 24 ++++++++++++------------
 2 files changed, 38 insertions(+), 18 deletions(-)

Comments

Yann E. MORIN Sept. 6, 2024, 6:16 p.m. UTC | #1 
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 Sept. 6, 2024, 6:33 p.m. UTC | #2 
Dario, All,

On 2024-09-06 19:15 +0200, Dario Binacchi spake thusly:
[--SNIP--]
> diff --git a/package/ipmitool/Config.in b/package/ipmitool/Config.in
> index 9516ff8596d3..c4b3ee734d44 100644
> --- a/package/ipmitool/Config.in
> +++ b/package/ipmitool/Config.in
> @@ -9,14 +9,34 @@ config BR2_PACKAGE_IPMITOOL
>  
>  if BR2_PACKAGE_IPMITOOL
>  
> -config BR2_PACKAGE_IPMITOOL_PEN_REG_URI
> -	string "IANA PEN registry URL or path"
> -	default "https://www.iana.org/assignments/enterprise-numbers.txt"

You forgot to add legacy handling for this variable:

    config BR2_PACKAGE_IPMITOOL_PEN_REG_URI
        string "IANA PEN registry move to iana-assignment package"
        help
          Installation of the IANA PEN is now handled by the
          iana-assignment package; to install a custom PEN,
          use a rootfs-overlay for example.


    config BR2_PACKAGE_IPMITOOL_PEN_REG_URI_WRAP
        bool
        default y if BR2_PACKAGE_IPMITOOL_PEN_REG_URI != ""
        select BR2_LEGACY

Regards,
Yann E. MORIN.

Patch
diff mbox series 

diff --git a/package/ipmitool/Config.in b/package/ipmitool/Config.in
index 9516ff8596d3..c4b3ee734d44 100644
--- a/package/ipmitool/Config.in
+++ b/package/ipmitool/Config.in
@@ -9,14 +9,34 @@  config BR2_PACKAGE_IPMITOOL
 
 if BR2_PACKAGE_IPMITOOL
 
-config BR2_PACKAGE_IPMITOOL_PEN_REG_URI
-	string "IANA PEN registry URL or path"
-	default "https://www.iana.org/assignments/enterprise-numbers.txt"
+#
+# PEN registry selection
+#
+
+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"
 	help
-	  Enter an URL or a file path to the PEN registry to use.
+	  Enter a file path to the PEN registry to use.
+
+	  Leave empty to not use a registry; vendor IDs will be
+	  displayed instead of the corresponding names.
 
-	  Note that the official registry is 4MiB+ and may change any
-	  time and is thus not guaranteed to be reproducible.
+endchoice
+
+config BR2_PACKAGE_IPMITOOL_USE_CUSTOM_PEN_FILE
+	string "PEN registry file path"
+	depends on BR2_PACKAGE_IPMITOOL_USE_CUSTOM_PEN
+	help
+	  Path to the PEN registry file.
 
 	  Leave empty to not use a registry; vendor IDs will be
 	  displayed instead of the corresponding names.
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
+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))