Re: [Buildroot] [PATCH v2 2/3] package/mender-update-modules: new package

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

Hello Adam,

On Tue, 29 Apr 2025 12:16:06 +0200
Adam Duskett <adam.duskett@amarulasolutions.com> wrote:

> Contains community supported Update Modules. An Update Module is an extension
> to the Mender client for supporting a new type of software update, such as a
> package manager, container, bootloader or even updates of nearby
> microcontrollers. An Update Module can be tailored to a specific device or
> environment (e.g. update a proprietary bootloader), or be more
> general-purpose (e.g. install a set of .rpm packages.).
> 
> Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>

Thanks for this patch! I think it looks good overall, and I was almost
going to apply it, but I have a few questions/concerns, see below.


> diff --git a/package/mender-update-modules/Config.in b/package/mender-update-modules/Config.in
> new file mode 100644
> index 0000000000..b789a0a00c
> --- /dev/null
> +++ b/package/mender-update-modules/Config.in
> @@ -0,0 +1,190 @@
> +config BR2_PACKAGE_MENDER_UPDATE_MODULES
> +	bool "mender-update-modules"
> +	depends on BR2_PACKAGE_HOST_GO_HOST_ARCH_SUPPORTS
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # boost-log
> +	depends on BR2_TOOLCHAIN_SUPPORTS_ALWAYS_LOCKFREE_ATOMIC_INTS # boost-log
> +	depends on !BR2_TOOLCHAIN_HAS_GCC_BUG_64735 # boost-log
> +	depends on BR2_USE_MMU # libglib2
> +	depends on BR2_USE_WCHAR # libglib2

I am confused about those dependencies due to libglib2 and boost-log.
Where do they come from? You don't select libglib2 or boost, and
neither BR2_PACKAGE_MENDER nor BR2_PACKAGE_HOST_MENDER_ARTIFACT use
libglib/boost as far as I can see. Could you clarify?

> +	select BR2_PACKAGE_HOST_MENDER_ARTIFACT
> +	select BR2_PACKAGE_MENDER

I think it would make more sense for this to be a:

	depends on BR2_PACKAGE_MENDER

indeed, this mender-update-modules is really an extension of Mender.
Surely if you want to use those extensions, you know that you want to
use Mender and therefore that you have to enable the Mender package
first.

> +config BR2_PACKAGE_MENDER_UPDATE_MODULES_DFU
> +	bool "DFU"
> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_9 # libusb
> +	select BR2_PACKAGE_DFU_UTIL
> +	help
> +	  The DFU Update Module is able to update peripheral devices
> +	  connected to the device running Mender.
> +	  Example use-cases:
> +	    - Deploy firmware updates to peripheral devices using the
> +	      USB Device Firmware Update (DFU) protocol
> +
> +	  https://github.com/mendersoftware/mender-update-modules/tree/master/dfu

Config.in comment for the gcc version dependency? I agree a bit
pedantic, but oh well, let's have it for completeness.


> +ifeq ($(BR2_PACKAGE_MENDER_UPDATE_MODULES_REBOOT),y)
> +MENDER_UPDATE_MODULES_MODULES += reboot
> +define MENDER_UPDATE_MODULES_INSTALL_MENDER_REBOOT_GEN
> +	$(INSTALL) -D -m 0755 $(@D)/reboot/reboot-gen \
> +		$(HOST_DIR)/bin/reboot-artifact-gen
> +endef

Too bad this one doesn't follow the pattern of
$(@D)/$(f)/module-artifact-gen like the other ones. But OK, fine that's
not your fault :-)

> +MENDER_UPDATE_MODULES_POST_INSTALL_TARGET_HOOKS += MENDER_UPDATE_MODULES_INSTALL_MENDER_REBOOT_GEN
> +endif
> +
> +ifeq ($(BR2_PACKAGE_MENDER_UPDATE_MODULES_ROOTFS_VERSION_CHECK),y)
> +MENDER_UPDATE_MODULES_DEPENDENCIES += python3
> +MENDER_UPDATE_MODULES_MODULES += rootfs-version-check
> +define MENDER_UPDATE_MODULES_INSTALL_MENDER_COMPARE_VERSIONS
> +	$(INSTALL) -D -m 0755 $(@D)/rootfs-version-check/mender-compare-versions \
> +		$(TARGET_DIR)/usr/bin/mender-compare-versions
> +endef
> +MENDER_UPDATE_MODULES_POST_INSTALL_TARGET_HOOKS += MENDER_UPDATE_MODULES_INSTALL_MENDER_COMPARE_VERSIONS
> +endif
> +
> +ifeq ($(BR2_PACKAGE_MENDER_UPDATE_MODULES_SWU),y)
> +MENDER_UPDATE_MODULES_MODULES += swu
> +endif
> +
> +define MENDER_UPDATE_MODULES_INSTALL_TARGET_CMDS
> +	$(foreach f,$(MENDER_UPDATE_MODULES_MODULES), \
> +	$(INSTALL) -D -m 0775 $(@D)/$(f)/module/$(f) \
> +		$(TARGET_DIR)/usr/share/mender/modules/v3/$(f); \
> +	if [ -d $(@D)/$(f)/module-artifact-gen ]; then \
> +		$(INSTALL) -D -m 0775 $(@D)/$(f)/module-artifact-gen/$(f)-artifact-gen \
> +			$(HOST_DIR)/bin/$(f)-artifact-gen; \
> +	fi; \

Could you indent this by one more level inside the foreach loop?

So overall really it's the libglib/boost question that prevented me
from applying. Everything else is minor stuff, or stuff I could have
tweaked when applying.

Thanks a lot!

Thomas Petazzoni
-- 
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