<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 12, 2016 at 12:30 AM, Andreas Oberritter <span dir="ltr"><<a href="mailto:obi@opendreambox.org" target="_blank">obi@opendreambox.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="gmail-m_-1762581467229495523gmail-">On 11.10.2016 21:34, Christopher Larson wrote:<br>
><br>
> On Tue, Oct 11, 2016 at 12:03 PM, Jagadeesh Krishnanjanappa<br>
</span><span class="gmail-m_-1762581467229495523gmail-">> <<a href="mailto:jkrishnanjanappa@mvista.com" target="_blank">jkrishnanjanappa@mvista.com</a> <mailto:<a href="mailto:jkrishnanjanappa@mvista.com" target="_blank">jkrishnanjanappa@mvist<wbr>a.com</a>>> wrote:<br>
><br>
> On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson<br>
</span><span class="gmail-m_-1762581467229495523gmail-">> <<a href="mailto:clarson@kergoth.com" target="_blank">clarson@kergoth.com</a> <mailto:<a href="mailto:clarson@kergoth.com" target="_blank">clarson@kergoth.com</a>>> wrote:<br>
><br>
><br>
> On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa<br>
> <<a href="mailto:jkrishnanjanappa@mvista.com" target="_blank">jkrishnanjanappa@mvista.com</a><br>
</span><span class="gmail-m_-1762581467229495523gmail-">> <mailto:<a href="mailto:jkrishnanjanappa@mvista.com" target="_blank">jkrishnanjanappa@mvis<wbr>ta.com</a>>> wrote:<br>
><br>
> Thanks for reviewing the patch.<br>
><br>
><br>
> I think this is too fragile to land in OE-Core. What<br>
> happens if a<br>
> network driver prints "device=eth0"? Or if other kernel<br>
> messages make<br>
> the string disappear from dmesg and connman gets<br>
> restarted later?<br>
><br>
><br>
> True. If device=eth0 gets disappeared/corrupted, then we may<br>
> have problem.<br>
><br>
><br>
> FWIW, I'm attaching my current wrapper script for<br>
> connmand. I don't<br>
> think it's perfect, but at least it doesn't depend on<br>
> any init manager<br>
> and works across restarts.<br>
><br>
> The wrapper script attached by you, takes care of the<br>
> missing scenarios. Seems to be more complete.<br>
><br>
><br>
><br>
> Add these lines to connman's do_install, in case you'd<br>
> like to test<br>
> and/or submit it:<br>
><br>
> mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real<br>
> install -m 755 ${WORKDIR}/<a href="http://connmand-nfsroot.in" rel="noreferrer" target="_blank">connmand-nfsroot.in</a><br>
</span>> <<a href="http://connmand-nfsroot.in" rel="noreferrer" target="_blank">http://connmand-nfsroot.in</a>> ${D}${sbindir}/connmand<br>
<span class="gmail-m_-1762581467229495523gmail-">> sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand<br>
><br>
> I think it would be good idea to integrate your changes into<br>
> the already existing OE-core's connman script, instead of a<br>
> calling original connman script from the wrapper script.<br>
><br>
><br>
> I threw<br>
> together <a href="https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs" rel="noreferrer" target="_blank">https://github.com/openembedde<wbr>d/openembedded-core/compare/<wbr>master...kergoth:connman-<wbr>systemd-nfs</a><br>
> <<a href="https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs" rel="noreferrer" target="_blank">https://github.com/openembed<wbr>ded/openembedded-core/compare/<wbr>master...kergoth:connman-<wbr>systemd-nfs</a>> last<br>
> night, only limited testing. I think using a script in<br>
> libexecdir is rather cleaner than wrapping connmand in place,<br>
> for something like this, personally.<br>
><br>
><br>
> In the above commit, the last line of connman/connman/start-connman<br>
> file, should be exec @SBINDIR@/connmand "$@" instead of exec<br>
> @SBINDIR@/connman "$@" ; as the actual binary file name is connmand.<br>
><br>
><br>
> Thanks, didn’t notice that. Obviously it needs polish and testing before<br>
> submission, I mainly linked it to solicit thoughts on the approach :)<br>
<br>
</span>your proposal adds some complexity to the recipe, which I tried to<br>
avoid, because this wrapper script is something we'll probably have to<br>
maintain forever.<br>
<br>
For example, your recipe might break without notice if upstream is going<br>
to ship another binary as ${sbindir}/connmand-something in the future;<br>
or if upstream decides to change the systemd unit to use a variable<br>
instead of a hardcoded path to connmand.<br>
<br>
Besides that, I don't have a strong opinion on the location of the<br>
wrapper script.</blockquote><div><br></div><div>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.</div></div>-- <br><div class="gmail-m_-1762581467229495523gmail_signature">Christopher Larson<br>clarson at kergoth dot com<br>Founder - BitBake, OpenEmbedded, OpenZaurus<br>Maintainer - Tslib<br>Senior Software Engineer, Mentor Graphics</div>
</div></div>