Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"

[Robert Yang <liezhi.yang@windriver.com>]

On 11/15/2016 12:08 AM, Christopher Larson wrote:
>
> On Mon, Nov 14, 2016 at 9:05 AM, Robert Yang <liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com>> wrote:
>
>     On 11/14/2016 11:38 PM, Christopher Larson wrote:
>
>
>         On Mon, Nov 14, 2016 at 8:37 AM, Robert Yang <liezhi.yang@windriver.com
>         <mailto:liezhi.yang@windriver.com>
>         <mailto:liezhi.yang@windriver.com <mailto:liezhi.yang@windriver.com>>>
>         wrote:
>
>             On 11/14/2016 11:03 PM, Christopher Larson wrote:
>
>
>                 On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang
>         <liezhi.yang@windriver.com <mailto:liezhi.yang@windriver.com>
>                 <mailto:liezhi.yang@windriver.com
>         <mailto:liezhi.yang@windriver.com>>
>                 <mailto:liezhi.yang@windriver.com
>         <mailto:liezhi.yang@windriver.com> <mailto:liezhi.yang@windriver.com
>         <mailto:liezhi.yang@windriver.com>>>>
>                 wrote:
>
>                     The old code:
>                     network_enabled = not d.getVar('BB_NO_NETWORK', True)
>
>                     It is True only when BB_NO_NETWORK is not set (None),
>                     but BB_NO_NETWORK = "0" should also be True while "1" means
>         no network,
>                     "0" means need network in a normal case.
>
>                     Signed-off-by: Robert Yang <liezhi.yang@windriver.com
>         <mailto:liezhi.yang@windriver.com>
>                 <mailto:liezhi.yang@windriver.com
>         <mailto:liezhi.yang@windriver.com>>
>                     <mailto:liezhi.yang@windriver.com
>         <mailto:liezhi.yang@windriver.com> <mailto:liezhi.yang@windriver.com
>         <mailto:liezhi.yang@windriver.com>>>>
>
>                     ---
>                      meta/classes/sanity.bbclass | 14 +++++++++-----
>                      1 file changed, 9 insertions(+), 5 deletions(-)
>
>                     diff --git a/meta/classes/sanity.bbclass
>         b/meta/classes/sanity.bbclass
>                     index 7e383f9..c5e3809 100644
>                     --- a/meta/classes/sanity.bbclass
>                     +++ b/meta/classes/sanity.bbclass
>                     @@ -363,15 +363,19 @@ def check_connectivity(d):
>                          test_uris = (d.getVar('CONNECTIVITY_CHECK_URIS', True) or
>                 "").split()
>                          retval = ""
>
>                     +    bbn = d.getVar('BB_NO_NETWORK', True)
>                     +    if bbn not in (None, '0', '1'):
>                     +        return 'BB_NO_NETWORK should be "0" or "1", but it
>         is "%s"'
>                 % bbn
>
>
>                 Does this mirror the same logic used in bitbake? What’s the
>         behavior if it’s
>                 set, but to the empty string?
>
>
>             bitbake only checks whether it equals "1" or not. Without this
>         patch, an empty
>             string is the same as not set since it doesn't equal to "1". But if
>         it is
>             set to "0", bitbake uses it as enable network, sanity.bbclass uses it
>             as disable netowrk, which are conflicted. We can add checking for
>         empty string,
>             but do we have to ? Limit it to "0" or "1" makes things clear.
>
>
>         IMO if we’re going to change the semantics, we should do it in bitbake
>         and then
>         mirror that in the metadata. Sanity checking should mirror the actual
>         variable
>         behavior where it’s used.
>
>
>     Sounds reasonable, but I'm not sure how to do it, ways I can think out:
>     1) Handle "0" as enable network as bitbake does in sanity.bbclass
>     2) If we want to limit its values, maybe we need check it in bitbake rather
>        than in sanity.bbclass, there are also other values have the similar
>        problems, I did a rough grep, such as BB_FETCH_PREMIRRORONLY:
>
>     fetch2/__init__.py:        premirroronly =
>     (self.d.getVar("BB_FETCH_PREMIRRORONLY", True) == "1")
>     fetch2/git.py:        if d.getVar("BB_FETCH_PREMIRRORONLY", True) is not None:
>
>     The __init__.py only checks whether it is "1" or not, but git.py checks if it
>     is None, there would be confusions when it is "" or "0".
>
>
> Sounds like bb.utils.to_boolean() may be our friend for a number of these.

Thanks, sounds good to me, let's see others comments, and I will work on that
later if no objections (maybe 1 month later). Currently I will simply make
sanity.bbclass handle "0" as bitbake does.

Have good day. I have to go to sleep now.

// Robert


> --
> Christopher Larson
> clarson at kergoth dot com
> Founder - BitBake, OpenEmbedded, OpenZaurus
> Maintainer - Tslib
> Senior Software Engineer, Mentor Graphics