Re: [Buildroot] [PATCH 1/1] package/espflash: add BR2_PACKAGE_HAS_UDEV dependency

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Torben Voltmer <mail@t-voltmer.net>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/1] package/espflash: add BR2_PACKAGE_HAS_UDEV dependency
Date: Mon, 29 Jul 2024 10:11:19 +0200	[thread overview]
Message-ID: <20240729101119.22f92091@windsurf> (raw)
In-Reply-To: <20240728214222.2515625-1-mail@t-voltmer.net>

Hello Torben,

On Sun, 28 Jul 2024 23:42:22 +0200
Torben Voltmer <mail@t-voltmer.net> wrote:

> Adding the dependency on BR2_PACKAGE_HAS_UDEV, since espflash links libudev
> (via the libudev-sys crate). This should fix some of the autobuilder
> failures caused by the espflash package.
> 
> Signed-off-by: Torben Voltmer <mail@t-voltmer.net>

Thanks a lot for the patch! However, this patch is not sufficient,
indeed your patch will not ensure that udev is built before espflash.
Therefore, you need to add ESPFLASH_DEPENDENCIES = udev in the .mk file.

Also, in the commit log, could you be more affirmative than "it should
fix some of the autobuilder failures"? Does it fix, or it doesn't?

If it does fix autobuilder issues, could you add something like this:

Fixes:

  http://autobuild.buildroot.net/results/339a5c9a82d2c2d2bd08f9a728b9ee8e8ab48928/

> diff --git a/package/espflash/Config.in b/package/espflash/Config.in
> index 2a7a0f6b38..62b1dc381c 100644
> --- a/package/espflash/Config.in
> +++ b/package/espflash/Config.in
> @@ -1,6 +1,7 @@
>  config BR2_PACKAGE_ESPFLASH
>  	bool "espflash"
>  	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
> +	depends on BR2_PACKAGE_HAS_UDEV
>  	select BR2_PACKAGE_HOST_RUSTC
>  	help
>  	  Serial flasher utilities for Espressif devices, based
> @@ -8,3 +9,6 @@ config BR2_PACKAGE_ESPFLASH
>  	  ESP32-C2/C3/C6, ESP32-H2, ESP32-P4, and ESP32-S2/S3.
>  
>  	  https://github.com/esp-rs/espflash
> +
> +comment "espflash needs udev /dev management"
> +	depends on !BR2_PACKAGE_HAS_UDEV

You also need to add:

	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS

here so that the comment doesn't appear on architectures which are
anyway not supported by Rust.

Could you adjust those small details, and send a v2?

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