public inbox for openembedded-core@lists.openembedded.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Fixes for sanity.bbclass and sdk
@ 2016-11-14 14:34 Robert Yang
  2016-11-14 14:34 ` [PATCH 1/4] populate_sdk_ext.bbclass: use weak assignment for TOOLCHAINEXT_OUTPUTNAME Robert Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Robert Yang @ 2016-11-14 14:34 UTC (permalink / raw)
  To: openembedded-core

The following changes since commit 9303d8055c45a0f6af295d70a6f6a8b9d8d8a7c9:

  devtool: add "rename" subcommand (2016-11-07 11:04:17 +0000)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib rbt/4fixes
  http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/4fixes

Robert Yang (4):
  populate_sdk_ext.bbclass: use weak assignment for
    TOOLCHAINEXT_OUTPUTNAME
  testsdk.bbclass: print which file is not found
  sanity.bbclass:check_connectivity(): print more error messages
  sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"

 meta/classes/populate_sdk_ext.bbclass |  2 +-
 meta/classes/sanity.bbclass           | 19 +++++++++++++------
 meta/classes/testsdk.bbclass          |  6 +++---
 3 files changed, 17 insertions(+), 10 deletions(-)

-- 
2.10.2



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] populate_sdk_ext.bbclass: use weak assignment for TOOLCHAINEXT_OUTPUTNAME
  2016-11-14 14:34 [PATCH 0/4] Fixes for sanity.bbclass and sdk Robert Yang
@ 2016-11-14 14:34 ` Robert Yang
  2016-11-14 14:34 ` [PATCH 2/4] testsdk.bbclass: print which file is not found Robert Yang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Robert Yang @ 2016-11-14 14:34 UTC (permalink / raw)
  To: openembedded-core

The TOOLCHAINEXT_OUTPUTNAME is different from TOOLCHAIN_OUTPUTNAME, it
is used for eSDK only, so that it doesn't mix with SDK, use "?=" for it
so that other conf file can define it.

If we don't use "?=" here, then we need use forcevariable to redfine it:
TOOLCHAINEXT_OUTPUTNAME_forcevariable = "foo"

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 meta/classes/populate_sdk_ext.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/populate_sdk_ext.bbclass b/meta/classes/populate_sdk_ext.bbclass
index a0856d4..2c9def6 100644
--- a/meta/classes/populate_sdk_ext.bbclass
+++ b/meta/classes/populate_sdk_ext.bbclass
@@ -77,7 +77,7 @@ COREBASE_FILES ?= " \
 
 SDK_DIR_task-populate-sdk-ext = "${WORKDIR}/sdk-ext"
 B_task-populate-sdk-ext = "${SDK_DIR}"
-TOOLCHAINEXT_OUTPUTNAME = "${SDK_NAME}-toolchain-ext-${SDK_VERSION}"
+TOOLCHAINEXT_OUTPUTNAME ?= "${SDK_NAME}-toolchain-ext-${SDK_VERSION}"
 TOOLCHAIN_OUTPUTNAME_task-populate-sdk-ext = "${TOOLCHAINEXT_OUTPUTNAME}"
 
 SDK_EXT_TARGET_MANIFEST = "${SDK_DEPLOY}/${TOOLCHAINEXT_OUTPUTNAME}.target.manifest"
-- 
2.10.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] testsdk.bbclass: print which file is not found
  2016-11-14 14:34 [PATCH 0/4] Fixes for sanity.bbclass and sdk Robert Yang
  2016-11-14 14:34 ` [PATCH 1/4] populate_sdk_ext.bbclass: use weak assignment for TOOLCHAINEXT_OUTPUTNAME Robert Yang
@ 2016-11-14 14:34 ` Robert Yang
  2016-11-14 14:34 ` [PATCH 3/4] sanity.bbclass:check_connectivity(): print more error messages Robert Yang
  2016-11-14 14:34 ` [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0" Robert Yang
  3 siblings, 0 replies; 11+ messages in thread
From: Robert Yang @ 2016-11-14 14:34 UTC (permalink / raw)
  To: openembedded-core

This is helpful when debug.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 meta/classes/testsdk.bbclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta/classes/testsdk.bbclass b/meta/classes/testsdk.bbclass
index 43342b1..06b4c50 100644
--- a/meta/classes/testsdk.bbclass
+++ b/meta/classes/testsdk.bbclass
@@ -59,7 +59,7 @@ def testsdk_main(d):
 
     tcname = d.expand("${SDK_DEPLOY}/${TOOLCHAIN_OUTPUTNAME}.sh")
     if not os.path.exists(tcname):
-        bb.fatal("The toolchain is not built. Build it before running the tests: 'bitbake <image> -c populate_sdk' .")
+        bb.fatal("The toolchain %s is not built. Build it before running the tests: 'bitbake <image> -c populate_sdk' ." % tcname)
 
     sdktestdir = d.expand("${WORKDIR}/testimage-sdk/")
     bb.utils.remove(sdktestdir, True)
@@ -109,8 +109,8 @@ def testsdkext_main(d):
 
     tcname = d.expand("${SDK_DEPLOY}/${TOOLCHAINEXT_OUTPUTNAME}.sh")
     if not os.path.exists(tcname):
-        bb.fatal("The toolchain ext is not built. Build it before running the" \
-                 " tests: 'bitbake <image> -c populate_sdk_ext' .")
+        bb.fatal("The toolchain ext %s is not built. Build it before running the" \
+                 " tests: 'bitbake <image> -c populate_sdk_ext' ." % tcname)
 
     testdir = d.expand("${WORKDIR}/testsdkext/")
     bb.utils.remove(testdir, True)
-- 
2.10.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] sanity.bbclass:check_connectivity(): print more error messages
  2016-11-14 14:34 [PATCH 0/4] Fixes for sanity.bbclass and sdk Robert Yang
  2016-11-14 14:34 ` [PATCH 1/4] populate_sdk_ext.bbclass: use weak assignment for TOOLCHAINEXT_OUTPUTNAME Robert Yang
  2016-11-14 14:34 ` [PATCH 2/4] testsdk.bbclass: print which file is not found Robert Yang
@ 2016-11-14 14:34 ` Robert Yang
  2016-11-14 14:34 ` [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0" Robert Yang
  3 siblings, 0 replies; 11+ messages in thread
From: Robert Yang @ 2016-11-14 14:34 UTC (permalink / raw)
  To: openembedded-core

This can help fix the problem when the error happens.

Now the error message is:
    Fetcher failure for URL: 'https://www.example.com/'. URL https://www.example.com/ doesn't work.
    Please ensure your host's network is configured correctly,
    or set BB_NO_NETWORK = "1" to disable network access if
    all required sources are on local disk.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 meta/classes/sanity.bbclass | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 7682ffb..7e383f9 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -380,7 +380,10 @@ def check_connectivity(d):
             # pointed to a support mechanism.
             msg = data.getVar('CONNECTIVITY_CHECK_MSG', True) or ""
             if len(msg) == 0:
-                msg = "%s. Please ensure your network is configured correctly.\n" % err
+                msg = "%s.\n" % err
+                msg += "    Please ensure your host's network is configured correctly,\n"
+                msg += "    or set BB_NO_NETWORK = \"1\" to disable network access if\n"
+                msg += "    all required sources are on local disk.\n"
             retval = msg
 
     return retval
-- 
2.10.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
  2016-11-14 14:34 [PATCH 0/4] Fixes for sanity.bbclass and sdk Robert Yang
                   ` (2 preceding siblings ...)
  2016-11-14 14:34 ` [PATCH 3/4] sanity.bbclass:check_connectivity(): print more error messages Robert Yang
@ 2016-11-14 14:34 ` Robert Yang
  2016-11-14 15:03   ` Christopher Larson
  3 siblings, 1 reply; 11+ messages in thread
From: Robert Yang @ 2016-11-14 14:34 UTC (permalink / raw)
  To: openembedded-core

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>
---
 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
+
     # Only check connectivity if network enabled and the
     # CONNECTIVITY_CHECK_URIS are set
-    network_enabled = not d.getVar('BB_NO_NETWORK', True)
+    network_enabled = not (bbn == '1')
     check_enabled = len(test_uris)
-    # Take a copy of the data store and unset MIRRORS and PREMIRRORS
-    data = bb.data.createCopy(d)
-    data.delVar('PREMIRRORS')
-    data.delVar('MIRRORS')
     if check_enabled and network_enabled:
+        # Take a copy of the data store and unset MIRRORS and PREMIRRORS
+        data = bb.data.createCopy(d)
+        data.delVar('PREMIRRORS')
+        data.delVar('MIRRORS')
         try:
             fetcher = bb.fetch2.Fetch(test_uris, data)
             fetcher.checkstatus()
-- 
2.10.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
  2016-11-14 14:34 ` [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0" Robert Yang
@ 2016-11-14 15:03   ` Christopher Larson
  2016-11-14 15:37     ` Robert Yang
  0 siblings, 1 reply; 11+ messages in thread
From: Christopher Larson @ 2016-11-14 15:03 UTC (permalink / raw)
  To: Robert Yang; +Cc: Patches and discussions about the oe-core layer

[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]

On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang <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>
> ---
>  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?
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 1965 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
  2016-11-14 15:03   ` Christopher Larson
@ 2016-11-14 15:37     ` Robert Yang
  2016-11-14 15:38       ` Christopher Larson
  0 siblings, 1 reply; 11+ messages in thread
From: Robert Yang @ 2016-11-14 15:37 UTC (permalink / raw)
  To: Christopher Larson; +Cc: Patches and discussions about the oe-core layer



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>> 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>>
>     ---
>      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.

// Robert

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
  2016-11-14 15:37     ` Robert Yang
@ 2016-11-14 15:38       ` Christopher Larson
  2016-11-14 16:05         ` Robert Yang
  0 siblings, 1 reply; 11+ messages in thread
From: Christopher Larson @ 2016-11-14 15:38 UTC (permalink / raw)
  To: Robert Yang; +Cc: Patches and discussions about the oe-core layer

[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]

On Mon, Nov 14, 2016 at 8:37 AM, Robert Yang <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>> 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>>
>>     ---
>>      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.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 3362 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
  2016-11-14 15:38       ` Christopher Larson
@ 2016-11-14 16:05         ` Robert Yang
  2016-11-14 16:08           ` Christopher Larson
  0 siblings, 1 reply; 11+ messages in thread
From: Robert Yang @ 2016-11-14 16:05 UTC (permalink / raw)
  To: Christopher Larson; +Cc: Patches and discussions about the oe-core layer



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>> 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>>>
>         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>>>
>             ---
>              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".

// Robert

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
  2016-11-14 16:05         ` Robert Yang
@ 2016-11-14 16:08           ` Christopher Larson
  2016-11-14 16:19             ` Robert Yang
  0 siblings, 1 reply; 11+ messages in thread
From: Christopher Larson @ 2016-11-14 16:08 UTC (permalink / raw)
  To: Robert Yang; +Cc: Patches and discussions about the oe-core layer

[-- Attachment #1: Type: text/plain, Size: 3686 bytes --]

On Mon, Nov 14, 2016 at 9:05 AM, Robert Yang <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>> 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>>>
>>         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>>>
>>
>>             ---
>>              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.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 5511 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
  2016-11-14 16:08           ` Christopher Larson
@ 2016-11-14 16:19             ` Robert Yang
  0 siblings, 0 replies; 11+ messages in thread
From: Robert Yang @ 2016-11-14 16:19 UTC (permalink / raw)
  To: Christopher Larson; +Cc: Patches and discussions about the oe-core layer



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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-11-14 16:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-14 14:34 [PATCH 0/4] Fixes for sanity.bbclass and sdk Robert Yang
2016-11-14 14:34 ` [PATCH 1/4] populate_sdk_ext.bbclass: use weak assignment for TOOLCHAINEXT_OUTPUTNAME Robert Yang
2016-11-14 14:34 ` [PATCH 2/4] testsdk.bbclass: print which file is not found Robert Yang
2016-11-14 14:34 ` [PATCH 3/4] sanity.bbclass:check_connectivity(): print more error messages Robert Yang
2016-11-14 14:34 ` [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0" Robert Yang
2016-11-14 15:03   ` Christopher Larson
2016-11-14 15:37     ` Robert Yang
2016-11-14 15:38       ` Christopher Larson
2016-11-14 16:05         ` Robert Yang
2016-11-14 16:08           ` Christopher Larson
2016-11-14 16:19             ` Robert Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox