On 11.10.2016 21:34, Christopher Larson wrote:
>
> On Tue, Oct 11, 2016 at 12:03 PM, Jagadeesh Krishnanjanappa
> <jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com>> wrote:
>
>     On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson
>     <clarson@kergoth.com <mailto:clarson@kergoth.com>> wrote:
>
>
>         On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa
>         <jkrishnanjanappa@mvista.com
>         <mailto:jkrishnanjanappa@mvista.com>> wrote:
>
>             Thanks for reviewing the patch.
>
>
>                 I think this is too fragile to land in OE-Core. What
>                 happens if a
>                 network driver prints "device=eth0"? Or if other kernel
>                 messages make
>                 the string disappear from dmesg and connman gets
>                 restarted later?
>
>
>             True. If device=eth0 gets disappeared/corrupted, then we may
>             have problem.
>
>
>                 FWIW, I'm attaching my current wrapper script for
>                 connmand. I don't
>                 think it's perfect, but at least it doesn't depend on
>                 any init manager
>                 and works across restarts.
>
>             The wrapper script attached by you, takes care of the
>             missing scenarios. Seems to be more complete.
>
>
>
>                 Add these lines to connman's do_install, in case you'd
>                 like to test
>                 and/or submit it:
>
>                 mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real
>                 install -m 755 ${WORKDIR}/connmand-nfsroot.in
>                 <http://connmand-nfsroot.in> ${D}${sbindir}/connmand
>                 sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand
>
>             I think it would be good idea to integrate your changes into
>             the already existing OE-core's connman script, instead of a
>             calling original connman script from the wrapper script.
>
>
>         I threw
>         together https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs
>         <https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs> last
>         night, only limited testing. I think using a script in
>         libexecdir is rather cleaner than wrapping connmand in place,
>         for something like this, personally.
>
>
>     In the above commit,  the last line of connman/connman/start-connman
>     file, should be exec @SBINDIR@/connmand "$@" instead of exec
>     @SBINDIR@/connman "$@" ; as the actual binary file name is connmand.
>
>
> Thanks, didn’t notice that. Obviously it needs polish and testing before
> submission, I mainly linked it to solicit thoughts on the approach :)

your proposal adds some complexity to the recipe, which I tried to
avoid, because this wrapper script is something we'll probably have to
maintain forever.

For example, your recipe might break without notice if upstream is going
to ship another binary as ${sbindir}/connmand-something in the future;
or if upstream decides to change the systemd unit to use a variable
instead of a hardcoded path to connmand.

Besides that, I don't have a strong opinion on the location of the
wrapper script.