Patchwork connman: enable connman client

login
register
mail settings
Submitter Cristian Iorga
Date Feb. 22, 2013, 8:10 p.m.
Message ID <1361563821-7683-1-git-send-email-cristian.iorga@intel.com>
Download mbox | patch
Permalink /patch/45011/
State New
Headers show

Comments

Cristian Iorga - Feb. 22, 2013, 8:10 p.m.
connmanctl is now included when connman is installed

Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
---
 meta/recipes-connectivity/connman/connman.inc |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Ross Burton - Feb. 23, 2013, 8:34 a.m.
Hi Cristian,

On Friday, 22 February 2013 at 20:10, Cristian Iorga wrote: 
> - --disable-client \

I'd prefer to see an explicit --enable-client here to make it clear what we're turning on and off.
 
> + install -m 0755 ${S}/client/connmanctl ${D}${bindir}

${S} (source) should be ${B} (build), although at the moment they are the same directory I've been slowly working on changing that.

Also the fact that you have to do this is depressing.  I wonder why upstream doesn't want to install any of the tools?

Ross
ml@communistcode.co.uk - Feb. 23, 2013, 9:02 a.m.
On 02/23/13 08:34, Ross Burton wrote:
> Hi Cristian,
>
> On Friday, 22 February 2013 at 20:10, Cristian Iorga wrote:
>> - --disable-client \
> I'd prefer to see an explicit --enable-client here to make it clear what we're turning on and off.
>   
>> + install -m 0755 ${S}/client/connmanctl ${D}${bindir}
> ${S} (source) should be ${B} (build), although at the moment they are the same directory I've been slowly working on changing that.
>
> Also the fact that you have to do this is depressing.  I wonder why upstream doesn't want to install any of the tools?

Connmanctl is a helper, even though I have connman in my image, I 
wouldn't want connmanctl. I assume this is why they don't ship it by 
default; as my use case is fairly normal for connman.

Which leads to the next point, this should be a seperate package.

>
> Ross
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Paul Eggleton - Feb. 23, 2013, 11:30 a.m.
On Saturday 23 February 2013 09:02:07 Jack Mitchell wrote:
> On 02/23/13 08:34, Ross Burton wrote:
> > Also the fact that you have to do this is depressing.  I wonder why
> > upstream doesn't want to install any of the tools?
>
> Connmanctl is a helper, even though I have connman in my image, I
> wouldn't want connmanctl. I assume this is why they don't ship it by
> default; as my use case is fairly normal for connman.

I would have thought if you had asked for the client to be built (or not asked 
for it not to be built, as the case may be) then surely you would want it 
installed (as far as make install goes anyway)?

> Which leads to the next point, this should be a seperate package.

Probably should be I think yes.

Cheers,
Paul
ml@communistcode.co.uk - Feb. 23, 2013, 12:43 p.m.
On 02/23/13 11:30, Paul Eggleton wrote:
> On Saturday 23 February 2013 09:02:07 Jack Mitchell wrote:
>> On 02/23/13 08:34, Ross Burton wrote:
>>> Also the fact that you have to do this is depressing.  I wonder why
>>> upstream doesn't want to install any of the tools?
>> Connmanctl is a helper, even though I have connman in my image, I
>> wouldn't want connmanctl. I assume this is why they don't ship it by
>> default; as my use case is fairly normal for connman.
> I would have thought if you had asked for the client to be built (or not asked
> for it not to be built, as the case may be) then surely you would want it
> installed (as far as make install goes anyway)?

Yes I misread, I thought we were explicity enabling it. I'll try to send 
an email off to upstream and see what they think.

>
>> Which leads to the next point, this should be a seperate package.
> Probably should be I think yes.

If no one beats me to it I'll look into this at work next week. Some of 
the tools should be split into packages too and I think there may be 
some distro specific tweaks also (e.g. wifi enabled etc).

>
> Cheers,
> Paul
>
Cristian Iorga - Feb. 25, 2013, 7:18 a.m.
Hello Ross,

1. As far as I have seen, building the connmanctl is the default, there is no --enable-client.
	Maybe I am wrong, I have extracted this info from the documentation.
2. Like you said, at the moment, the client executable is produced directly in the source directory, hence my install command.
	If I understand correctly, will be the same thing if I change into build directory?

Regards,
Cristian

-----Original Message-----
From: Ross Burton [mailto:ross.burton@intel.com] 
Sent: Saturday, February 23, 2013 10:34 AM
To: Iorga, Cristian
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] connman: enable connman client

Hi Cristian,

On Friday, 22 February 2013 at 20:10, Cristian Iorga wrote: 
> - --disable-client \

I'd prefer to see an explicit --enable-client here to make it clear what we're turning on and off.
 
> + install -m 0755 ${S}/client/connmanctl ${D}${bindir}

${S} (source) should be ${B} (build), although at the moment they are the same directory I've been slowly working on changing that.

Also the fact that you have to do this is depressing.  I wonder why upstream doesn't want to install any of the tools?

Ross
Cristian Iorga - Feb. 25, 2013, 7:36 a.m.
Also, please note that tools-test and tools-wispr use also the ${S} dir in install, so my code is in accordance to previous code (prior artwork :-) ).

-----Original Message-----
From: Iorga, Cristian 
Sent: Monday, February 25, 2013 9:19 AM
To: Burton, Ross
Cc: openembedded-core@lists.openembedded.org
Subject: RE: [OE-core] [PATCH] connman: enable connman client

Hello Ross,

1. As far as I have seen, building the connmanctl is the default, there is no --enable-client.
	Maybe I am wrong, I have extracted this info from the documentation.
2. Like you said, at the moment, the client executable is produced directly in the source directory, hence my install command.
	If I understand correctly, will be the same thing if I change into build directory?

Regards,
Cristian

-----Original Message-----
From: Ross Burton [mailto:ross.burton@intel.com] 
Sent: Saturday, February 23, 2013 10:34 AM
To: Iorga, Cristian
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] connman: enable connman client

Hi Cristian,

On Friday, 22 February 2013 at 20:10, Cristian Iorga wrote: 
> - --disable-client \

I'd prefer to see an explicit --enable-client here to make it clear what we're turning on and off.
 
> + install -m 0755 ${S}/client/connmanctl ${D}${bindir}

${S} (source) should be ${B} (build), although at the moment they are the same directory I've been slowly working on changing that.

Also the fact that you have to do this is depressing.  I wonder why upstream doesn't want to install any of the tools?

Ross
Ross Burton - Feb. 25, 2013, 8:21 a.m.
Hi Cristian,

On 25 February 2013 07:18, Iorga, Cristian <cristian.iorga@intel.com> wrote:
> 1. As far as I have seen, building the connmanctl is the default, there is no --enable-client.
>         Maybe I am wrong, I have extracted this info from the documentation.

With autotools if there's a --disable-something there's always a
--enable-something (actually, --disable-something is implemented as
--enable-something=no).

> 2. Like you said, at the moment, the client executable is produced directly in the source directory, hence my install command.
>         If I understand correctly, will be the same thing if I change into build directory?

The binary is produced in the build directory, it just happens that it
is the same directory as the source directory at present.

> Also, please note that tools-test and tools-wispr use also the ${S} dir in install, so my code is in accordance to previous code (prior artwork :-) ).

Yes, lots of code is wrong. I'm slowly working through all of oe-core...

Ross

Patch

diff --git a/meta/recipes-connectivity/connman/connman.inc b/meta/recipes-connectivity/connman/connman.inc
index b61e2af..69cb74c 100644
--- a/meta/recipes-connectivity/connman/connman.inc
+++ b/meta/recipes-connectivity/connman/connman.inc
@@ -20,7 +20,7 @@  DEPENDS  = "dbus glib-2.0 ppp iptables gnutls \
             ${@base_contains('DISTRO_FEATURES', '3g','ofono', '', d)} \
             "
 
-INC_PR = "r17"
+INC_PR = "r18"
 
 TIST = "--enable-tist"
 TIST_powerpc = ""
@@ -38,7 +38,6 @@  EXTRA_OECONF += "\
     --enable-tools \
     --enable-test \
     --disable-polkit \
-    --disable-client \
     --enable-fake \
     ${@base_contains('DISTRO_FEATURES', 'systemd', '--with-systemdunitdir=${systemd_unitdir}/system/', '', d)} \
 "
@@ -73,6 +72,7 @@  do_install_append() {
 	install -d ${D}${bindir}
 	install -m 0755 ${S}/tools/*-test ${D}${bindir}
 	install -m 0755 ${S}/tools/wispr ${D}${bindir}
+	install -m 0755 ${S}/client/connmanctl ${D}${bindir}
 
 	# We don't need to package an empty directory
 	rmdir ${D}${libdir}/connman/scripts