On Wed, Oct 12, 2016 at 12:30 AM, Andreas Oberritter <o...@opendreambox.org>
wrote:

> On 11.10.2016 21:34, Christopher Larson wrote:
> >
> > On Tue, Oct 11, 2016 at 12:03 PM, Jagadeesh Krishnanjanappa
> > <jkrishnanjana...@mvista.com <mailto:jkrishnanjana...@mvista.com>>
> wrote:
> >
> >     On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson
> >     <clar...@kergoth.com <mailto:clar...@kergoth.com>> wrote:
> >
> >
> >         On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa
> >         <jkrishnanjana...@mvista.com
> >         <mailto:jkrishnanjana...@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/openembedde
> d/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.


Good points. I think the future-proofing could be addressed with something
like sed -i -e "s#^ExecStart=.*#ExecStart=$wrapperpath#"
${D}${systemd_unitdir}/system/connman.service, and then switch away from
that weak start-connman script in favor of yours, but keep it in libexec
for cleanliness.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Reply via email to