[PATCH 0/9] icecc.bbclass: Fix some issues

[PATCH 0/9] icecc.bbclass: Fix some issues

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

Hello,

while trying to get icecc work I faced some issues which are addressed
in this patch series.

First it was difficult to find out the reason why some packages don't
use icecc because the relevant information was missing in the
logs. There were also some messages which were disturbing the normal
log message flow during the build.

I also noticed that changing the ICECC blacklist variables always
caused a full rebuild because the sstate checksums didn't match. So
adding these blacklist variables to BB_HASHBASE_WHITELIST speeds up
working with incremental changes to these variables a lot.

Further I encountered some packages calling the compiler in the
install step. This caused either icecc not being used in this step or
even build failures. For this to work the icecc environment must also
be set in do_install. For this change also a dummy python version of
set_icecc_env was needed because there was at least one package which
provided its own python version of do_install where set_icecc_env
didn't exist.

I also had trouble with our own kernel recipe which caused an error
during recipe parsing. This recipe uses shell expansion in
KERNEL_CC. This causes a syntax error when this variable gets expanded
in get_cross_kernel_cc. This function now shell expands KERNEL_CC if
necessary before expanding it.

Further for some recipes (e.g. icedtea7 in meta-java) it was needed to
add a whitelist to be able to force icecc even if the recipe sets
PARALLEL_MAKE to "" but supports parallel building in a different way.

In some cases I had race conditions when creating the toolchain
package which gets distributed to the icecc nodes. This mostly
happened after the cross toolchain is ready because then several tasks
using the cross toolchain start in parallel. The result was in many
cases a broken toolchain package resulting in build failures.


1..3 : Logging changes
4    : Improve sstate interaction
5..6 : Enable icecc also in install step
7    : Improve KERNEL_CC handling
8    : Add package whitelist
9    : Fix race condition when packaging toolchain

Best regards
Tobias Henkel

Tobias Henkel (9):
  icecc: Log reason in error cases
  icecc: Remove output on stderr when calling which
  icecc: Reduce verbosity with empty PARALLEL_MAKE
  icecc: Add blacklist vars to BB_HASHBASE_WHITELIST
  icecc: Add dummy python version of set_icecc_env
  icecc: Enable icecc also for install step
  icecc: Support shell evaluation of KERNEL_CC
  icecc: Add package whitelist
  icecc: Fix race condition when packaging toolchain

 meta/classes/icecc.bbclass | 75 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 13 deletions(-)

-- 
1.8.3.1

[PATCH 1/9] icecc: Log reason in error cases

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

The current implementation doesn't give a hint about the cause in case
something went wrong in set_icecc_env. This makes it harder to find
out why a package is not being built using icecc. Therefore warnings
are inserted in the various error cases.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index cf3f23d..71c556f 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -196,12 +196,14 @@ set_icecc_env() {
     ICECC_VERSION="${@icc_version(bb, d)}"
     if [ "x${ICECC_VERSION}" = "x" ]
     then
+        bbwarn "Cannot use icecc: could not get ICECC_VERSION"
         return
     fi
 
     ICE_PATH="${@icc_path(bb, d)}"
     if [ "x${ICE_PATH}" = "x" ]
     then
+        bbwarn "Cannot use icecc: could not get ICE_PATH"
         return
     fi
 
@@ -209,6 +211,7 @@ set_icecc_env() {
     ICECC_CXX="${@icc_get_and_check_tool(bb, d, "g++")}"
     if [ ! -x "${ICECC_CC}" -o ! -x "${ICECC_CXX}" ]
     then
+        bbwarn "Cannot use icecc: could not get ICECC_CC or ICECC_CXX"
         return
     fi
 
@@ -216,6 +219,7 @@ set_icecc_env() {
     ICECC_VERSION=`echo ${ICECC_VERSION} | sed -e "s/@VERSION@/$ICE_VERSION/g"`
     if [ ! -x "${ICECC_ENV_EXEC}" ]
     then
+        bbwarn "Cannot use icecc: invalid ICECC_ENV_EXEC"
         return
     fi
 
-- 
1.8.3.1

[PATCH 2/9] icecc: Remove output on stderr when calling which

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

The icecc class often uses 'which' for determining paths. This leads
to many messages on stderr in case 'which' doesn't find the
executable. Redirecting stderr to /dev/null inhibits these messages as
the result is handled correctly anyway.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index 71c556f..c4088af 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -48,7 +48,7 @@ def get_cross_kernel_cc(bb,d):
     return kernel_cc
 
 def get_icecc(d):
-    return d.getVar('ICECC_PATH') or os.popen("which icecc").read()[:-1]
+    return d.getVar('ICECC_PATH') or os.popen("which icecc 2> /dev/null").read()[:-1]
 
 def create_path(compilers, bb, d):
     """
@@ -161,9 +161,9 @@ def icc_get_external_tool(bb, d, tool):
 
 def icc_get_tool(bb, d, tool):
     if icc_is_native(bb, d):
-        return os.popen("which %s" % tool).read()[:-1]
+        return os.popen("which %s 2> /dev/null" % tool).read()[:-1]
     elif icc_is_kernel(bb, d):
-        return os.popen("which %s" % get_cross_kernel_cc(bb, d)).read()[:-1]
+        return os.popen("which %s 2> /dev/null" % get_cross_kernel_cc(bb, d)).read()[:-1]
     else:
         ice_dir = d.expand('${STAGING_BINDIR_TOOLCHAIN}')
         target_sys = d.expand('${TARGET_SYS}')
-- 
1.8.3.1

[PATCH 3/9] icecc: Reduce verbosity with empty PARALLEL_MAKE

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

Currently the icecc class prints a note for every package which
disables parallel make at parse time. This is unneccessary as many
packages don't support parallel building. Changing the log level from
info to debug hides these messages in normal builds without removing
the information when needed.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index c4088af..eaa2aac 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -106,7 +106,7 @@ def use_icc(bb,d):
             return "no"
 
     if d.getVar('PARALLEL_MAKE') == "":
-        bb.note(package_tmp, " ", d.expand('${PV}'), " has empty PARALLEL_MAKE, disable icecc")
+        bb.debug(1, package_tmp, " ", d.expand('${PV}'), " has empty PARALLEL_MAKE, disable icecc")
         return "no"
 
     return "yes"
-- 
1.8.3.1

[PATCH 4/9] icecc: Add blacklist vars to BB_HASHBASE_WHITELIST

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

Changing any of the ICECC blacklist variables should not change the
sstate checksum as this doesn't influence the build result.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index eaa2aac..a3e8f9b 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -26,7 +26,7 @@
 #Error checking is kept to minimum so double check any parameters you pass to the class
 ###########################################################################################
 
-BB_HASHBASE_WHITELIST += "ICECC_PARALLEL_MAKE ICECC_DISABLED"
+BB_HASHBASE_WHITELIST += "ICECC_PARALLEL_MAKE ICECC_DISABLED ICECC_USER_PACKAGE_BL ICECC_USER_CLASS_BL"
 
 ICECC_ENV_EXEC ?= "${STAGING_BINDIR_NATIVE}/icecc-create-env"
 
-- 
1.8.3.1

[PATCH 5/9] icecc: Add dummy python version of set_icecc_env

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

Bitbakes prepend mechanism for the tasks disregards the type of the
function. Thus bitbaking recipes using python functions for configure,
compile or install steps fail due to the missing python version of
set_icecc_env.

Assuming that icecc doesn't need to be used in such situations adding
a dummy python version of set_icecc_env fixes this.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index a3e8f9b..99749d3 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -188,6 +188,10 @@ def icc_get_and_check_tool(bb, d, tool):
     else:
         return t
 
+def set_icecc_env():
+    # dummy python version of set_icecc_env
+    return
+
 set_icecc_env() {
     if [ "x${ICECC_DISABLED}" != "x" ]
     then
-- 
1.8.3.1

[PATCH 6/9] icecc: Enable icecc also for install step

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

Some packages are calling the compiler in the install step. In this
case either the build breaks or icecc is not used for building. The
proper environment has to be set to enable icecc based building.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index 99749d3..debc05d 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -258,6 +258,6 @@ do_compile_kernelmodules_prepend() {
     set_icecc_env
 }
 
-#do_install_prepend() {
-#    set_icecc_env
-#}
+do_install_prepend() {
+    set_icecc_env
+}
-- 
1.8.3.1

[PATCH 7/9] icecc: Support shell evaluation of KERNEL_CC

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

In the current implementation a KERNEL_CC variable containing shell
evaluation breaks the build process. Shell expansion is not happening
before general expansion in get_cross_kernel_cc which results in a
syntax error and an aborted parse process.

Before expanding the KERNEL_CC variable get_cross_kernel_cc now checks
for backticks or '$(' in the KERNEL_CC variable and performs a shell
evaluation using a call to echo if it finds one.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index debc05d..0c9b260 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -41,7 +41,13 @@ def icecc_dep_prepend(d):
 DEPENDS_prepend += "${@icecc_dep_prepend(d)} "
 
 def get_cross_kernel_cc(bb,d):
-    kernel_cc = d.expand('${KERNEL_CC}')
+    kernel_cc = d.getVar('KERNEL_CC')
+
+    # evaluate the expression by the shell if necessary
+    if '`' in kernel_cc or '$(' in kernel_cc:
+        kernel_cc = os.popen("echo %s" % kernel_cc).read()[:-1]
+
+    kernel_cc = d.expand(kernel_cc)
     kernel_cc = kernel_cc.replace('ccache', '').strip()
     kernel_cc = kernel_cc.split(' ')[0]
     kernel_cc = kernel_cc.strip()
-- 
1.8.3.1

[PATCH 8/9] icecc: Add package whitelist

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

There are some recipes which parse the PARALLEL_MAKE variable by their
own and set them to an empty string afterwards. This disables icecc
for this recipe.

Adding a whitelist for forcing icecc makes it possible to use icecc
also with these recipes.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index 0c9b260..e3b05ac 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -21,12 +21,13 @@
 #
 #User can specify if specific packages or packages belonging to class should not use icecc to distribute
 #compile jobs to remote machines, but handled localy, by defining ICECC_USER_CLASS_BL and ICECC_PACKAGE_BL
-#with the appropriate values in local.conf
+#with the appropriate values in local.conf. In addition the user can force to enable icecc for packages
+#which set an empty PARALLEL_MAKE variable by defining ICECC_USER_PACKAGE_WL.
 #########################################################################################
 #Error checking is kept to minimum so double check any parameters you pass to the class
 ###########################################################################################
 
-BB_HASHBASE_WHITELIST += "ICECC_PARALLEL_MAKE ICECC_DISABLED ICECC_USER_PACKAGE_BL ICECC_USER_CLASS_BL"
+BB_HASHBASE_WHITELIST += "ICECC_PARALLEL_MAKE ICECC_DISABLED ICECC_USER_PACKAGE_BL ICECC_USER_CLASS_BL ICECC_USER_PACKAGE_WL"
 
 ICECC_ENV_EXEC ?= "${STAGING_BINDIR_NATIVE}/icecc-create-env"
 
@@ -104,6 +105,7 @@ def use_icc(bb,d):
     #for one reason or the other
     system_package_blacklist = [ "uclibc", "glibc", "gcc", "bind", "u-boot", "dhcp-forwarder", "enchant", "connman", "orbit2" ]
     user_package_blacklist = (d.getVar('ICECC_USER_PACKAGE_BL') or "").split()
+    user_package_whitelist = (d.getVar('ICECC_USER_PACKAGE_WL') or "").split()
     package_blacklist = system_package_blacklist + user_package_blacklist
 
     for black in package_blacklist:
@@ -111,6 +113,11 @@ def use_icc(bb,d):
             #bb.note(package_tmp, ' found in blacklist, disable icecc')
             return "no"
 
+    for white in user_package_whitelist:
+        if white in package_tmp:
+            bb.debug(1, package_tmp, " ", d.expand('${PV})'), " found in whitelist, enable icecc")
+            return "yes"
+
     if d.getVar('PARALLEL_MAKE') == "":
         bb.debug(1, package_tmp, " ", d.expand('${PV}'), " has empty PARALLEL_MAKE, disable icecc")
         return "no"
@@ -131,7 +138,8 @@ def icc_version(bb, d):
         return ""
 
     parallel = d.getVar('ICECC_PARALLEL_MAKE') or ""
-    d.setVar("PARALLEL_MAKE", parallel)
+    if not d.getVar('PARALLEL_MAKE') == "":
+        d.setVar("PARALLEL_MAKE", parallel)
 
     if icc_is_native(bb, d):
         archive_name = "local-host-env"
-- 
1.8.3.1

[PATCH 9/9] icecc: Fix race condition when packaging toolchain

[Tobias Henkel]

From: Tobias Henkel <tobias.henkel@bmw-carit.de>

In the current implementation there can be a race condition while
creating the toolchain archive causing the build to break.

This is fixed by locking the toolchain archiving step using flock.

Signed-off-by: Tobias Henkel <tobias.henkel@bmw-carit.de>
---
 meta/classes/icecc.bbclass | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/meta/classes/icecc.bbclass b/meta/classes/icecc.bbclass
index e3b05ac..970c557 100644
--- a/meta/classes/icecc.bbclass
+++ b/meta/classes/icecc.bbclass
@@ -202,6 +202,21 @@ def icc_get_and_check_tool(bb, d, tool):
     else:
         return t
 
+wait_for_file() {
+    local TIME_ELAPSED=0
+    local FILE_TO_TEST=$1
+    local TIMEOUT=$2
+    until [ -f "$FILE_TO_TEST" ]
+    do
+        TIME_ELAPSED=`expr $TIME_ELAPSED + 1`
+        if [ $TIME_ELAPSED -gt $TIMEOUT ]
+        then
+            return 1
+        fi
+        sleep 1
+    done
+}
+
 def set_icecc_env():
     # dummy python version of set_icecc_env
     return
@@ -247,10 +262,22 @@ set_icecc_env() {
         ICECC_AS="`which as`"
     fi
 
-    if [ ! -r "${ICECC_VERSION}" ]
+    if [ ! -f "${ICECC_VERSION}.done" ]
     then
         mkdir -p "`dirname "${ICECC_VERSION}"`"
-        ${ICECC_ENV_EXEC} "${ICECC_CC}" "${ICECC_CXX}" "${ICECC_AS}" "${ICECC_VERSION}"
+
+        # the ICECC_VERSION generation step must be locked by a mutex
+        # in order to prevent race conditions
+        if flock -n "${ICECC_VERSION}.lock" \
+            ${ICECC_ENV_EXEC} "${ICECC_CC}" "${ICECC_CXX}" "${ICECC_AS}" "${ICECC_VERSION}"
+        then
+            touch "${ICECC_VERSION}.done"
+        elif [ ! wait_for_file "${ICECC_VERSION}.done" 30 ]
+        then
+            # locking failed so wait for ${ICECC_VERSION}.done to appear
+            bbwarn "Timeout waiting for ${ICECC_VERSION}.done"
+            return
+        fi
     fi
 
     export ICECC_VERSION ICECC_CC ICECC_CXX
-- 
1.8.3.1

Re: [PATCH 2/9] icecc: Remove output on stderr when calling which

[Enrico Scholz]

Tobias Henkel <tobias.henkel-Y0tyK2LzuKvpgbXj0Ix11Q@public.gmane.org>
writes:

> -    return d.getVar('ICECC_PATH') or os.popen("which icecc").read()[:-1]
> +    return d.getVar('ICECC_PATH') or os.popen("which icecc 2> /dev/null").read()[:-1]
                                        ~~~~~~~~~~~~~~~~~~~~~~

I think, this can/should be replaced by

  bb.utils.which(os.getenv("PATH"), "icecc")

Ditto the other locations.


Enrico

Re: [PATCH 2/9] icecc: Remove output on stderr when calling which

[Tobias Henkel]

On 11/12/2013 12:04 PM, Enrico Scholz wrote:
> -    return d.getVar('ICECC_PATH') or os.popen("which icecc").read()[:-1]
> +    return d.getVar('ICECC_PATH') or os.popen("which icecc 2> /dev/null").read()[:-1]
>                                          ~~~~~~~~~~~~~~~~~~~~~~
>
> I think, this can/should be replaced by
>
>    bb.utils.which(os.getenv("PATH"), "icecc")
>
> Ditto the other locations.
Thank you for this point. I'll post this as a new patch as the other 
patches of this patch series already seem to be in master.

Best regards
Tobias