[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Bernd Kuhls]

Upstream commit
https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=bee3796113196da1b3f56da42fcae4d9bae6695e

moved code depending on readline from the client tool to the shared
library, readline is therefore not an optional dependency anymore.

Fixes
http://autobuild.buildroot.net/results/019/0197ee0036e129b736c1dc0e83722236fb656618/

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/bluez5_utils/Config.in       | 1 +
 package/bluez5_utils/bluez5_utils.mk | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/package/bluez5_utils/Config.in b/package/bluez5_utils/Config.in
index 55831e9b61..bb0a2905c5 100644
--- a/package/bluez5_utils/Config.in
+++ b/package/bluez5_utils/Config.in
@@ -9,6 +9,7 @@ config BR2_PACKAGE_BLUEZ5_UTILS
 	depends on BR2_TOOLCHAIN_HAS_SYNC_4
 	select BR2_PACKAGE_DBUS
 	select BR2_PACKAGE_LIBGLIB2
+	select BR2_PACKAGE_READLINE
 	help
 	  bluez utils version 5.x
 
diff --git a/package/bluez5_utils/bluez5_utils.mk b/package/bluez5_utils/bluez5_utils.mk
index 0634ec9f6f..055d44de78 100644
--- a/package/bluez5_utils/bluez5_utils.mk
+++ b/package/bluez5_utils/bluez5_utils.mk
@@ -8,7 +8,7 @@ BLUEZ5_UTILS_VERSION = 5.48
 BLUEZ5_UTILS_SOURCE = bluez-$(BLUEZ5_UTILS_VERSION).tar.xz
 BLUEZ5_UTILS_SITE = $(BR2_KERNEL_MIRROR)/linux/bluetooth
 BLUEZ5_UTILS_INSTALL_STAGING = YES
-BLUEZ5_UTILS_DEPENDENCIES = dbus libglib2
+BLUEZ5_UTILS_DEPENDENCIES = dbus libglib2 readline
 BLUEZ5_UTILS_LICENSE = GPL-2.0+, LGPL-2.1+
 BLUEZ5_UTILS_LICENSE_FILES = COPYING COPYING.LIB
 
@@ -26,7 +26,6 @@ endif
 
 ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_CLIENT),y)
 BLUEZ5_UTILS_CONF_OPTS += --enable-client
-BLUEZ5_UTILS_DEPENDENCIES += readline
 else
 BLUEZ5_UTILS_CONF_OPTS += --disable-client
 endif
-- 
2.11.0

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Baruch Siach]

Hi Bernd,

On Sat, Jan 13, 2018 at 11:45:32PM +0100, Bernd Kuhls wrote:
> Upstream commit
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=bee3796113196da1b3f56da42fcae4d9bae6695e
> 
> moved code depending on readline from the client tool to the shared
> library, readline is therefore not an optional dependency anymore.
> 
> Fixes
> http://autobuild.buildroot.net/results/019/0197ee0036e129b736c1dc0e83722236fb656618/
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/bluez5_utils/Config.in       | 1 +
>  package/bluez5_utils/bluez5_utils.mk | 3 +--
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/bluez5_utils/Config.in b/package/bluez5_utils/Config.in
> index 55831e9b61..bb0a2905c5 100644
> --- a/package/bluez5_utils/Config.in
> +++ b/package/bluez5_utils/Config.in
> @@ -9,6 +9,7 @@ config BR2_PACKAGE_BLUEZ5_UTILS
>  	depends on BR2_TOOLCHAIN_HAS_SYNC_4
>  	select BR2_PACKAGE_DBUS
>  	select BR2_PACKAGE_LIBGLIB2
> +	select BR2_PACKAGE_READLINE

I don't think that this is the correct fix. The bluez README file says:

        --disable-client

                Disable support for the command line client

                By default the command line client is enabled and uses the
                readline library. For specific systems where BlueZ is
                configured by other means, the command line client can be
                disabled and the dependency on readline is removed.

So this does not look like a decision to make readline a mandatory dependency, 
but a bug that needs to be fixed upstream.

>  	help
>  	  bluez utils version 5.x
>  
> diff --git a/package/bluez5_utils/bluez5_utils.mk b/package/bluez5_utils/bluez5_utils.mk
> index 0634ec9f6f..055d44de78 100644
> --- a/package/bluez5_utils/bluez5_utils.mk
> +++ b/package/bluez5_utils/bluez5_utils.mk
> @@ -8,7 +8,7 @@ BLUEZ5_UTILS_VERSION = 5.48
>  BLUEZ5_UTILS_SOURCE = bluez-$(BLUEZ5_UTILS_VERSION).tar.xz
>  BLUEZ5_UTILS_SITE = $(BR2_KERNEL_MIRROR)/linux/bluetooth
>  BLUEZ5_UTILS_INSTALL_STAGING = YES
> -BLUEZ5_UTILS_DEPENDENCIES = dbus libglib2
> +BLUEZ5_UTILS_DEPENDENCIES = dbus libglib2 readline
>  BLUEZ5_UTILS_LICENSE = GPL-2.0+, LGPL-2.1+
>  BLUEZ5_UTILS_LICENSE_FILES = COPYING COPYING.LIB
>  
> @@ -26,7 +26,6 @@ endif
>  
>  ifeq ($(BR2_PACKAGE_BLUEZ5_UTILS_CLIENT),y)
>  BLUEZ5_UTILS_CONF_OPTS += --enable-client
> -BLUEZ5_UTILS_DEPENDENCIES += readline
>  else
>  BLUEZ5_UTILS_CONF_OPTS += --disable-client
>  endif

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Thomas Petazzoni]

Hello,

On Sun, 14 Jan 2018 07:32:04 +0200, Baruch Siach wrote:
> Hi Bernd,
> 
> On Sat, Jan 13, 2018 at 11:45:32PM +0100, Bernd Kuhls wrote:
> > Upstream commit
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=bee3796113196da1b3f56da42fcae4d9bae6695e
> > 
> > moved code depending on readline from the client tool to the shared
> > library, readline is therefore not an optional dependency anymore.
> > 
> > Fixes
> > http://autobuild.buildroot.net/results/019/0197ee0036e129b736c1dc0e83722236fb656618/
> > 
> > Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> > ---
> >  package/bluez5_utils/Config.in       | 1 +
> >  package/bluez5_utils/bluez5_utils.mk | 3 +--
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/package/bluez5_utils/Config.in b/package/bluez5_utils/Config.in
> > index 55831e9b61..bb0a2905c5 100644
> > --- a/package/bluez5_utils/Config.in
> > +++ b/package/bluez5_utils/Config.in
> > @@ -9,6 +9,7 @@ config BR2_PACKAGE_BLUEZ5_UTILS
> >  	depends on BR2_TOOLCHAIN_HAS_SYNC_4
> >  	select BR2_PACKAGE_DBUS
> >  	select BR2_PACKAGE_LIBGLIB2
> > +	select BR2_PACKAGE_READLINE  
> 
> I don't think that this is the correct fix. The bluez README file says:
> 
>         --disable-client
> 
>                 Disable support for the command line client
> 
>                 By default the command line client is enabled and uses the
>                 readline library. For specific systems where BlueZ is
>                 configured by other means, the command line client can be
>                 disabled and the dependency on readline is removed.
> 
> So this does not look like a decision to make readline a mandatory dependency, 
> but a bug that needs to be fixed upstream.

I agree it would be nice to check with upstream. They seem to have
intentionally added shell.c to the shared library, and this file
clearly unconditionally uses readline.

Perhaps it's their README file that is now incorrect.

Bernd, could you check with the upstream developers what they say about
this?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Thomas Petazzoni]

Bernd,

On Sun, 14 Jan 2018 14:53:24 +0100, Thomas Petazzoni wrote:

> I agree it would be nice to check with upstream. They seem to have
> intentionally added shell.c to the shared library, and this file
> clearly unconditionally uses readline.
> 
> Perhaps it's their README file that is now incorrect.
> 
> Bernd, could you check with the upstream developers what they say about
> this?

Have you had the chance to bring this up to the upstream developers?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Bernd Kuhls]

Am Sun, 14 Jan 2018 07:32:04 +0200 schrieb Baruch Siach:

> Hi Bernd,
> 
> On Sat, Jan 13, 2018 at 11:45:32PM +0100, Bernd Kuhls wrote:
[...]
>> +	select BR2_PACKAGE_READLINE
> 
> I don't think that this is the correct fix. The bluez README file says:
> 
>         --disable-client
> 
>                 Disable support for the command line client
> 
>                 By default the command line client is enabled and uses
>                 the readline library. For specific systems where BlueZ
>                 is configured by other means, the command line client
>                 can be disabled and the dependency on readline is
>                 removed.
> 
> So this does not look like a decision to make readline a mandatory
> dependency,
> but a bug that needs to be fixed upstream.

Hi Baruch,

this commit https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?
id=70b8b754f8e6f9abe9211c686b279dbef16bf666

"client: Make use of bt_shell.
Use bt_shell instead of readline directly."

removed readline code from client code and replaced it with calls to 
bt_shell, bt_shell itself is not used exclusively by client code anymore, 
see the commit I mentioned in my patch ( https://git.kernel.org/pub/scm/
bluetooth/bluez.git/commit/?id=bee3796113196da1b3f56da42fcae4d9bae6695e )
and the many source code files which include "src/shared/shell.h".

Afaics this does look like an outdated README file and not a bug in the 
source code, what do you think? Before writing to the bluez devs I would 
like to reach a consensus about what to ask them ;)

Regards, Bernd

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Baruch Siach]

Hi Bernd,

On Wed, Jan 17, 2018 at 10:23:11PM +0100, Bernd Kuhls wrote:
> Am Sun, 14 Jan 2018 07:32:04 +0200 schrieb Baruch Siach:
> > On Sat, Jan 13, 2018 at 11:45:32PM +0100, Bernd Kuhls wrote:
> [...]
> >> +	select BR2_PACKAGE_READLINE
> > 
> > I don't think that this is the correct fix. The bluez README file says:
> > 
> >         --disable-client
> > 
> >                 Disable support for the command line client
> > 
> >                 By default the command line client is enabled and uses
> >                 the readline library. For specific systems where BlueZ
> >                 is configured by other means, the command line client
> >                 can be disabled and the dependency on readline is
> >                 removed.
> > 
> > So this does not look like a decision to make readline a mandatory
> > dependency,
> > but a bug that needs to be fixed upstream.
> 
> this commit https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?
> id=70b8b754f8e6f9abe9211c686b279dbef16bf666
> 
> "client: Make use of bt_shell.
> Use bt_shell instead of readline directly."
> 
> removed readline code from client code and replaced it with calls to 
> bt_shell, bt_shell itself is not used exclusively by client code anymore, 
> see the commit I mentioned in my patch ( https://git.kernel.org/pub/scm/
> bluetooth/bluez.git/commit/?id=bee3796113196da1b3f56da42fcae4d9bae6695e )
> and the many source code files which include "src/shared/shell.h".
> 
> Afaics this does look like an outdated README file and not a bug in the 
> source code, what do you think? Before writing to the bluez devs I would 
> like to reach a consensus about what to ask them ;)

IMO you can simply ask them whether readline is now a mandatory dependency. If 
it is, then an update patch for the README file would be nice.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Jörg Krause]

Hi Baruch, Bernd,

On Thu, 2018-01-18 at 06:40 +0200, Baruch Siach wrote:
> Hi Bernd,
> 
> On Wed, Jan 17, 2018 at 10:23:11PM +0100, Bernd Kuhls wrote:
> > Am Sun, 14 Jan 2018 07:32:04 +0200 schrieb Baruch Siach:
> > > On Sat, Jan 13, 2018 at 11:45:32PM +0100, Bernd Kuhls wrote:
> > 
> > [...]
> > > > +	select BR2_PACKAGE_READLINE
> > > 
> > > I don't think that this is the correct fix. The bluez README file says:
> > > 
> > >         --disable-client
> > > 
> > >                 Disable support for the command line client
> > > 
> > >                 By default the command line client is enabled and uses
> > >                 the readline library. For specific systems where BlueZ
> > >                 is configured by other means, the command line client
> > >                 can be disabled and the dependency on readline is
> > >                 removed.
> > > 
> > > So this does not look like a decision to make readline a mandatory
> > > dependency,
> > > but a bug that needs to be fixed upstream.
> > 
> > this commit https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?
> > id=70b8b754f8e6f9abe9211c686b279dbef16bf666
> > 
> > "client: Make use of bt_shell.
> > Use bt_shell instead of readline directly."
> > 
> > removed readline code from client code and replaced it with calls to 
> > bt_shell, bt_shell itself is not used exclusively by client code anymore, 
> > see the commit I mentioned in my patch ( https://git.kernel.org/pub/scm/
> > bluetooth/bluez.git/commit/?id=bee3796113196da1b3f56da42fcae4d9bae6695e )
> > and the many source code files which include "src/shared/shell.h".
> > 
> > Afaics this does look like an outdated README file and not a bug in the 
> > source code, what do you think? Before writing to the bluez devs I would 
> > like to reach a consensus about what to ask them ;)
> 
> IMO you can simply ask them whether readline is now a mandatory dependency. If 
> it is, then an update patch for the README file would be nice.
> 

Any news on this issue? Would be great to have this fixed before the
next release.

I had a quick look and I would say that the README is indeed outdated
and readline is necessary for all parts depending on libshared-
{glib,mainloop}, which is true for most (all?) tools and bluetoothd.

Best regards,
J?rg Krause

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Thomas Petazzoni]

Hello,

On Fri, 02 Mar 2018 21:59:58 +0100, J?rg Krause wrote:

> Any news on this issue? Would be great to have this fixed before the
> next release.

I've sent an e-mail to upstream about this:

  https://marc.info/?l=linux-bluetooth&m=152421846903323&w=2

Let's see if they answer, and what they answer.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/bluez5_utils: needs readline

[Thomas Petazzoni]

Hello,

On Fri, 20 Apr 2018 12:20:13 +0200, Thomas Petazzoni wrote:

> > Any news on this issue? Would be great to have this fixed before the
> > next release.  
> 
> I've sent an e-mail to upstream about this:
> 
>   https://marc.info/?l=linux-bluetooth&m=152421846903323&w=2
> 
> Let's see if they answer, and what they answer.

They answered, and proposed a patch to fix it, which works. I've
submitted a Buildroot patch to integrate it:

  https://patchwork.ozlabs.org/patch/901944/

And therefore I'll mark the older patch proposed by Bernd as Superseded.

Best regards,

Thomas Petazzoni
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com