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

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

[Torben Voltmer]

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>
---
 package/espflash/Config.in | 4 ++++
 1 file changed, 4 insertions(+)

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
\ No newline at end of file
-- 
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Thomas Petazzoni via buildroot]

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

[Buildroot] [PATCH v2 1/1] package/espflash: add udev dependency

[Torben Voltmer]

Adding the dependency on udev, since espflash links libudev
(via the libudev-sys crate).

Fixes: http://autobuild.buildroot.net/results/c61b6ed4b6fb4d18d503ac37add200f5e6ae3706

---
Changes v1 -> v2:
  - Added missing dependency in espflash.mk
  - Added missing depends on in Config.in
  - Verified that the updated patch fixes at least one autobuild failure

Signed-off-by: Torben Voltmer <mail@t-voltmer.net>
---
 package/espflash/Config.in   | 5 +++++
 package/espflash/espflash.mk | 1 +
 2 files changed, 6 insertions(+)

diff --git a/package/espflash/Config.in b/package/espflash/Config.in
index 2a7a0f6b38..a445101269 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,7 @@ 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_HOST_RUSTC_TARGET_ARCH_SUPPORTS
+	depends on !BR2_PACKAGE_HAS_UDEV
diff --git a/package/espflash/espflash.mk b/package/espflash/espflash.mk
index 43532de58c..ad2294e1bf 100644
--- a/package/espflash/espflash.mk
+++ b/package/espflash/espflash.mk
@@ -9,5 +9,6 @@ ESPFLASH_SITE = $(call github,esp-rs,espflash,v$(ESPFLASH_VERSION))
 ESPFLASH_SUBDIR = espflash
 ESPFLASH_LICENSE = Apache-2.0 or MIT
 ESPFLASH_LICENSE_FILES = LICENSE-APACHE LICENSE-MIT
+ESPFLASH_DEPENDENCIES = udev
 
 $(eval $(cargo-package))
-- 
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Thomas Petazzoni via buildroot]

Hello,

Thanks for the new iteration, which I have applied. Just one minor nit,
see below!

On Mon, 29 Jul 2024 21:32:43 +0200
Torben Voltmer <mail@t-voltmer.net> wrote:

> Adding the dependency on udev, since espflash links libudev
> (via the libudev-sys crate).
> 
> Fixes: http://autobuild.buildroot.net/results/c61b6ed4b6fb4d18d503ac37add200f5e6ae3706
> 
> ---
> Changes v1 -> v2:
>   - Added missing dependency in espflash.mk
>   - Added missing depends on in Config.in
>   - Verified that the updated patch fixes at least one autobuild failure
> 
> Signed-off-by: Torben Voltmer <mail@t-voltmer.net>

The Signed-off-by: line should be *before* the --- sign. Indeed, when
git applies a patch, it drops from the commit log everything which is
after the --- sign. This is why your changelog (Changes v1 -> v2) is at
the right: we don't want this changelog to be part of the final commit
message. However, we do want the Signed-off-by to be part of the commit
message, so it should be before the --- sign.

I fixed that up when applying. Thanks for your contribution!

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