| Message ID | 20230710135752.1849099-1-t123yh.xyz@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] package/bluez5_utils: fix AC_CHECK_FILE error | expand |
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
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
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
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"))