[Buildroot] [PATCH 1/1] package/bluez5_utils: fix AC_CHECK_FILE error

[Buildroot] [PATCH 1/1] package/bluez5_utils: fix AC_CHECK_FILE error

[t123yh.xyz]

From: Yunhao Tian <t123yh.xyz@gmail.com>

Commit [1] in bluez5 introuduces AC_CHECK_FILE against ell library,
which produces the following error when configuring:

  checking for ./ell/ell.h... configure: error: cannot check for file
  existence when cross compiling

As this check is not critical in our use case, this patch removes the
relevant lines in configure.ac and configure to prevent errors.

[1]:
https://github.com/bluez/bluez/commit/1106b28be85ac9586d1758839226e163e9030ee2

Signed-off-by: Yunhao Tian <t123yh.xyz@gmail.com>
---
 .../0001-remove-ell-ac-check-file.patch       | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 package/bluez5_utils/0001-remove-ell-ac-check-file.patch

diff --git a/package/bluez5_utils/0001-remove-ell-ac-check-file.patch b/package/bluez5_utils/0001-remove-ell-ac-check-file.patch
new file mode 100644
index 0000000000..32831cf923
--- /dev/null
+++ b/package/bluez5_utils/0001-remove-ell-ac-check-file.patch
@@ -0,0 +1,86 @@
+From 56de22f93ed738aab11c5e75d26259462ea13faa Mon Sep 17 00:00:00 2001
+From: Yunhao Tian <t123yh.xyz@gmail.com>
+Date: Mon, 10 Jul 2023 21:46:28 +0800
+Subject: [PATCH 1/1] configure: remove check ell path
+                                           
+This breaks configure when cross-compiling, so remove it
+                                           
+Signed-off-by: Yunhao Tian <t123yh.xyz@gmail.com>
+Upstream: N/A not fixed yet
+
+diff -ur bbb/configure bluez5_utils-5.68/configure
+--- bbb/configure	2023-07-10 21:33:33.829161104 +0800
++++ bluez5_utils-5.68/configure	2023-07-10 21:33:47.961142717 +0800
+@@ -15959,57 +15959,6 @@
+ 
+ 
+ fi
+-if (test "${enable_external_ell}" != "yes"); then
+-	as_ac_File=`printf "%s\n" "ac_cv_file_${srcdir}/ell/ell.h" | $as_tr_sh`
+-{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for ${srcdir}/ell/ell.h" >&5
+-printf %s "checking for ${srcdir}/ell/ell.h... " >&6; }
+-if eval test \${$as_ac_File+y}
+-then :
+-  printf %s "(cached) " >&6
+-else $as_nop
+-  test "$cross_compiling" = yes &&
+-  as_fn_error $? "cannot check for file existence when cross compiling" "$LINENO" 5
+-if test -r "${srcdir}/ell/ell.h"; then
+-  eval "$as_ac_File=yes"
+-else
+-  eval "$as_ac_File=no"
+-fi
+-fi
+-eval ac_res=\$$as_ac_File
+-	       { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+-printf "%s\n" "$ac_res" >&6; }
+-if eval test \"x\$"$as_ac_File"\" = x"yes"
+-then :
+-  dummy=yes
+-else $as_nop
+-  as_ac_File=`printf "%s\n" "ac_cv_file_${srcdir}/../ell/ell/ell.h" | $as_tr_sh`
+-{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for ${srcdir}/../ell/ell/ell.h" >&5
+-printf %s "checking for ${srcdir}/../ell/ell/ell.h... " >&6; }
+-if eval test \${$as_ac_File+y}
+-then :
+-  printf %s "(cached) " >&6
+-else $as_nop
+-  test "$cross_compiling" = yes &&
+-  as_fn_error $? "cannot check for file existence when cross compiling" "$LINENO" 5
+-if test -r "${srcdir}/../ell/ell/ell.h"; then
+-  eval "$as_ac_File=yes"
+-else
+-  eval "$as_ac_File=no"
+-fi
+-fi
+-eval ac_res=\$$as_ac_File
+-	       { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+-printf "%s\n" "$ac_res" >&6; }
+-if eval test \"x\$"$as_ac_File"\" = x"yes"
+-then :
+-  dummy=yes
+-else $as_nop
+-  as_fn_error $? "ELL source is required or use --enable-external-ell" "$LINENO" 5
+-fi
+-
+-fi
+-
+-fi
+  if test "${enable_external_ell}" = "yes" ||
+ 				(test "${enable_btpclient}" != "yes" &&
+ 						test "${enable_mesh}" != "yes"); then
+diff -ur bbb/configure.ac bluez5_utils-5.68/configure.ac
+--- bbb/configure.ac	2023-07-10 21:33:33.829161104 +0800
++++ bluez5_utils-5.68/configure.ac	2023-07-10 21:33:55.465132962 +0800
+@@ -297,11 +297,6 @@
+ 	AC_SUBST(ELL_CFLAGS)
+ 	AC_SUBST(ELL_LIBS)
+ fi
+-if (test "${enable_external_ell}" != "yes"); then
+-	AC_CHECK_FILE(${srcdir}/ell/ell.h, dummy=yes,
+-			AC_CHECK_FILE(${srcdir}/../ell/ell/ell.h, dummy=yes,
+-				AC_MSG_ERROR(ELL source is required or use --enable-external-ell)))
+-fi
+ AM_CONDITIONAL(EXTERNAL_ELL, test "${enable_external_ell}" = "yes" ||
+ 				(test "${enable_btpclient}" != "yes" &&
+ 						test "${enable_mesh}" != "yes"))
-- 
2.30.2

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

Re: [Buildroot] [PATCH 1/1] package/bluez5_utils: fix AC_CHECK_FILE error

[Thomas Petazzoni via buildroot]

Hello Yunhao,

On Mon, 10 Jul 2023 21:57:52 +0800
t123yh.xyz@gmail.com wrote:

> From: Yunhao Tian <t123yh.xyz@gmail.com>
> 
> Commit [1] in bluez5 introuduces AC_CHECK_FILE against ell library,
> which produces the following error when configuring:
> 
>   checking for ./ell/ell.h... configure: error: cannot check for file
>   existence when cross compiling
> 
> As this check is not critical in our use case, this patch removes the
> relevant lines in configure.ac and configure to prevent errors.
> 
> [1]:
> https://github.com/bluez/bluez/commit/1106b28be85ac9586d1758839226e163e9030ee2
> 
> Signed-off-by: Yunhao Tian <t123yh.xyz@gmail.com>

Thanks a lot for this patch. However, I think there are several issues with it:

(1) We don't want to patch both configure and configure.ac, we want to
    patch only configure.ac, and use BLUEZ5_UTILS_AUTORECONF = YES

(2) AC_CHECK_FILE supports a cache variable that should allow to
    preseed the result of the check. So passing
    "ac_cv_file_${srcdir}/ell/ell.h=yes" should work-around the check,
    of course assuming the relevant quoting is added to prevent
    mis-interpretation of ${srcdir} in this context.

(3) Using AC_CHECK_FILE here is really horrible. The bluez5_utils
    configure script should be improved to use pkg-config to detect the
    external ELL library.

I think in Buildroot for now (2) would be acceptable, but (3) would
definitely be a nice contribution to upstream bluez5_utils.

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

Re: [Buildroot] [PATCH 1/1] package/bluez5_utils: fix AC_CHECK_FILE error

[Thomas Petazzoni via buildroot]

Hello Yunhao,

On Tue, 11 Jul 2023 00:39:38 +0800
Yunhao Tian <t123yh.xyz@gmail.com> wrote:

> > (1) We don't want to patch both configure and configure.ac, we want to
> >     patch only configure.ac, and use BLUEZ5_UTILS_AUTORECONF = YES  
> Understood, thanks for pointing out!
> >
> > (2) AC_CHECK_FILE supports a cache variable that should allow to
> >     preseed the result of the check. So passing
> >     "ac_cv_file_${srcdir}/ell/ell.h=yes" should work-around the check,
> >     of course assuming the relevant quoting is added to prevent
> >     mis-interpretation of ${srcdir} in this context.  
> I don't think this is worth the effort. Source code package of bluez
> already comes
> with internal ell, so it is not necessary to check it in our use case.

Aaah, but then I misunderstood. I thought the issue occurred when using
an external ELL.

Then in that case, the solution is "easier": we should not use the
internal copy of ELL in bluez-utils sources, but we should use the
external one. In Buildroot, whenever possible we try to use external
libraries, to avoid having multiple copies of the same code in the
target, and also to ensure security issues/bugs only need to be fixed
in one place.

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

Re: [Buildroot] [PATCH 1/1] package/bluez5_utils: fix AC_CHECK_FILE error

[Thomas Petazzoni via buildroot]

On Tue, 11 Jul 2023 00:39:38 +0800
Yunhao Tian <t123yh.xyz@gmail.com> wrote:

> Actually I found an upstream patch [1] that solves the problem. I will
> convert this to a patch
> together with the AUTORECONF option, and re-send.
> 
> [1]: https://lore.kernel.org/linux-bluetooth/20230701041252.139338-1-rudi@heitbaum.com/T/#t

In fact, Bernd already submitted such a patch:

  https://patchwork.ozlabs.org/project/buildroot/patch/20230704061944.496211-1-bernd@kuhls.net/

Best regards,

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