package/bluez5_utils: needs readline

PATCH New Bernd Kuhls 5 participants 9 messages Jan 13, 2018 – Apr 20, 2018

lore

Share & tools

lore

Bernd Kuhls

author

2018-01-13 22:45 UTC #0 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

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 <[email protected]>

---
 package/bluez5_utils/Config.in       | 1 +
 package/bluez5_utils/bluez5_utils.mk | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

package/bluez5_utils/Config.in +1 -0

index 55831e9b61..bb0a2905c5 100644
@@ -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

package/bluez5_utils/bluez5_utils.mk +1 -2

index 0634ec9f6f..055d44de78 100644
@@ -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

Signature · 1 line

2.11.0

Baruch Siach

2018-01-14 05:32 UTC #1 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

Hi Bernd,

Quoted 20 lines · 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 <[email protected]>
---
 package/bluez5_utils/Config.in       | 1 +
 package/bluez5_utils/bluez5_utils.mk | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)
package/bluez5_utils/Config.in +0 -0
index 55831e9b61..bb0a2905c5 100644

package/bluez5_utils/Config.in config BR2_PACKAGE_BLUEZ5_UTILS · line 12

Quoted 4 lines

package/bluez5_utils/Config.in +1 -0

@@ -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.

Quoted 17 lines

package/bluez5_utils/Config.in +0 -0

 	help
 	  bluez utils version 5.x

package/bluez5_utils/bluez5_utils.mk +1 -1

index 0634ec9f6f..055d44de78 100644
@@ -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

package/bluez5_utils/bluez5_utils.mk endif · line 31

Quoted 7 lines

package/bluez5_utils/bluez5_utils.mk +0 -1

@@ -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

Signature · 3 lines

     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 -

Thomas Petazzoni

2018-01-14 13:53 UTC #2 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

Hello,

Quoted 34 lines · Baruch Siach

Hi Bernd,
Quoted 20 lines · 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 <[email protected]>
---
 package/bluez5_utils/Config.in       | 1 +
 package/bluez5_utils/bluez5_utils.mk | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)
package/bluez5_utils/Config.in +0 -0
index 55831e9b61..bb0a2905c5 100644
package/bluez5_utils/Config.in config BR2_PACKAGE_BLUEZ5_UTILS · line 12
Quoted 4 lines
package/bluez5_utils/Config.in +1 -0
@@ -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

Quoted 7 lines

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

Signature · 3 lines

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

Thomas Petazzoni

2018-01-17 08:43 UTC #3 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

Bernd,

Quoted 8 lines · Thomas Petazzoni

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

Signature · 3 lines

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

Bernd Kuhls

author

2018-01-17 21:23 UTC #4 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

In reply to Baruch Siach

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

Quoted 3 lines

Hi Bernd,
On Sat, Jan 13, 2018 at 11:45:32PM +0100, Bernd Kuhls wrote:

[...]

Quoted 14 lines

package/bluez5_utils/Config.in config BR2_PACKAGE_BLUEZ5_UTILS · line 12
Quoted 1 line
package/bluez5_utils/Config.in +1 -0
+	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.

Quoted 3 lines

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

Baruch Siach

2018-01-18 04:40 UTC #5 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

Hi Bernd,

Quoted 33 lines · Bernd Kuhls

Am Sun, 14 Jan 2018 07:32:04 +0200 schrieb Baruch Siach:
Quoted 1 line · Bernd Kuhls
[...]
Quoted 14 lines
package/bluez5_utils/Config.in config BR2_PACKAGE_BLUEZ5_UTILS · line 12
Quoted 1 line
package/bluez5_utils/Config.in +1 -0
+	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.
Quoted 3 lines
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".

Quoted 3 lines

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

Signature · 3 lines

     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 -

Jörg Krause

2018-03-02 20:59 UTC #6 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

Hi Baruch, Bernd,

Quoted 37 lines · Baruch Siach

Hi Bernd,
Quoted 34 lines · Bernd Kuhls
Am Sun, 14 Jan 2018 07:32:04 +0200 schrieb Baruch Siach:
Quoted 1 line · Bernd Kuhls
[...]
Quoted 14 lines
package/bluez5_utils/Config.in config BR2_PACKAGE_BLUEZ5_UTILS · line 12
Quoted 1 line
package/bluez5_utils/Config.in +1 -0
+	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.
Quoted 3 lines
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".

Quoted 7 lines

Quoted 3 lines
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

Thomas Petazzoni

2018-04-20 10:20 UTC #7 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

Hello,

Quoted 2 lines · J?rg Krause

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

Signature · 3 lines

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

Thomas Petazzoni

2018-04-20 14:08 UTC #8 lore raw

PATCH 1/1 package/bluez5_utils: needs readline

Hello,

Quoted 8 lines · Thomas Petazzoni

Quoted 2 lines
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

Signature · 3 lines

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