Fixes for sanity.bbclass and sdk

PATCH In review Robert Yang 2 participants 11 messages Nov 14, 2016

lore

Share & tools

lore

Robert Yang

author

2016-11-14 14:34 UTC #0 lore raw

cover Fixes for sanity.bbclass and sdk

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(-)

Signature · 1 line

2.10.2

Robert Yang

author

2016-11-14 14:34 UTC #1 lore raw

PATCH 1/4 populate_sdk_ext.bbclass: use weak assignment for TOOLCHAINEXT_OUTPUTNAME

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 <[email protected]>

---
 meta/classes/populate_sdk_ext.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

meta/classes/populate_sdk_ext.bbclass +1 -1

index a0856d4..2c9def6 100644
@@ -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"

Signature · 1 line

2.10.2

Robert Yang

author

2016-11-14 14:34 UTC #2 lore raw

PATCH 2/4 testsdk.bbclass: print which file is not found

In reply to Robert Yang

This is helpful when debug.

Signed-off-by Robert Yang <[email protected]>

---
 meta/classes/testsdk.bbclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

meta/classes/testsdk.bbclass +3 -3

index 43342b1..06b4c50 100644
@@ -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)

Signature · 1 line

2.10.2

Robert Yang

author

2016-11-14 14:34 UTC #3 lore raw

PATCH 3/4 sanity.bbclass:check_connectivity(): print more error messages

In reply to Robert Yang

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 <[email protected]>

---
 meta/classes/sanity.bbclass | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

meta/classes/sanity.bbclass +4 -1

index 7682ffb..7e383f9 100644
@@ -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

Signature · 1 line

2.10.2

Robert Yang

author

2016-11-14 14:34 UTC #4 lore raw

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

In reply to Robert Yang

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 <[email protected]>

---
 meta/classes/sanity.bbclass | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

meta/classes/sanity.bbclass +9 -5

index 7e383f9..c5e3809 100644
@@ -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()

Signature · 1 line

2.10.2

Christopher Larson

2016-11-14 15:03 UTC #5 lore raw

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

Quoted 17 lines · Robert Yang

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 <[email protected]>
---
 meta/classes/sanity.bbclass | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
meta/classes/sanity.bbclass +0 -0
index 7e383f9..c5e3809 100644

meta/classes/sanity.bbclass def check_connectivity(d): · line 369

Quoted 7 lines

meta/classes/sanity.bbclass +3 -0

@@ -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?

Signature · 5 lines

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

Robert Yang

author

2016-11-14 15:37 UTC #6 lore raw

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

Quoted 25 lines · Christopher Larson

On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang <[email protected] <mailto:[email protected]>> 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 <[email protected]
<mailto:[email protected]>>
---
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 = ""

Quoted 7 lines

+ 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

Quoted 6 lines

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

Christopher Larson

2016-11-14 15:39 UTC #7 lore raw

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

Quoted 40 lines · Robert Yang

Quoted 33 lines · Christopher Larson
On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang <[email protected] <mailto:[email protected]>> 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 <[email protected]
    <mailto:[email protected]>>
    ---
     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
Quoted 4 lines
Does this mirror the same logic used in bitbake? What’s the behavior if it’s
set, but to the empty string?

Quoted 7 lines

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.

Signature · 5 lines

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

Robert Yang

author

2016-11-14 16:06 UTC #8 lore raw

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

Quoted 52 lines · Christopher Larson

On Mon, Nov 14, 2016 at 8:37 AM, Robert Yang <[email protected] <mailto:[email protected]>> wrote:
On 11/14/2016 11:03 PM, Christopher Larson wrote:
On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang <[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> 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 <[email protected]
<mailto:[email protected]>
<mailto:[email protected] <mailto:[email protected]>>>
---
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.

Quoted 3 lines

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

Quoted 6 lines

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

Christopher Larson

2016-11-14 16:08 UTC #9 lore raw

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

Quoted 79 lines · Robert Yang

Quoted 64 lines · Christopher Larson
On Mon, Nov 14, 2016 at 8:37 AM, Robert Yang <[email protected] <mailto:[email protected]>> wrote:
    On 11/14/2016 11:03 PM, Christopher Larson wrote:


        On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang <
[email protected]
<mailto:[email protected]> <mailto:[email protected] <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 <[email protected]
<mailto:[email protected]>
<mailto:[email protected] <mailto:
[email protected]>>>
            ---
             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.
Quoted 6 lines
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:

Quoted 8 lines

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.

Signature · 5 lines

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

Robert Yang

author

2016-11-14 16:19 UTC #10 lore raw

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

Quoted 93 lines · Christopher Larson

On Mon, Nov 14, 2016 at 9:05 AM, Robert Yang <[email protected] <mailto:[email protected]>> wrote:
On 11/14/2016 11:38 PM, Christopher Larson wrote:
On Mon, Nov 14, 2016 at 8:37 AM, Robert Yang <[email protected] <mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>> wrote:
On 11/14/2016 11:03 PM, Christopher Larson wrote:
On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang
<[email protected] <mailto:[email protected]>
<mailto:[email protected]
<mailto:[email protected]>>
<mailto:[email protected]
<mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>
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 <[email protected]
<mailto:[email protected]>
<mailto:[email protected]
<mailto:[email protected]>>
<mailto:[email protected]
<mailto:[email protected]> <mailto:[email protected] <mailto:[email protected]>>>>
---
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:

Quoted 5 lines

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

Quoted 6 lines

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