[Buildroot] [PATCH v2 00/10] autobuild-run: python3 compat and misc improvements

[Buildroot] [PATCH v2 00/10] autobuild-run: python3 compat and misc improvements

[André Erdmann]

Changes v1->v2:

* catch up with git HEAD: further cleanup/python3-compat changes (patches 1-6)

* "move check_requirements() to SystemInfo" is mostly identical to v1;
  changed: use "upload" var, remove entry from TODO list

* "encapsulate subprocess calls" had to be reworked

  It now wraps subprocess.Popen() instead of subprocess.call(),
  and implements run_cmd(), run_cmd_write_to() and (new:) run_cmd_get_stdout()
  on top of it. This covers all uses cases of the subprocess module and,
  in the case of run_cmd_get_stdout(), handles the bytes->str conversion.

  In v1, the "set-locale" patch added a subprocess.check_output() wrapper,
  which has been dropped.

* "set subprocess env" is a separate commit now,
  used to be part of the "set-locale" patch


Andr? Erdmann (10):
  autobuild-run: fixup imports
  autobuild-run, python3: dict.iteritems()->items()
  autobuild-run, python3: decode subprocess output
  autobuild-run, python3: encode/decode mmap data
  autobuild-run: avoid use of builtin as varname
  autobuild-run: reorganize imports
  autobuild-run: move check_requirements() to SystemInfo
  autobuild-run: encapsulate subprocess calls
  autobuild-run: control subprocess env
  autobuild-run: set locale to en_US or C

 scripts/autobuild-run | 280 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 183 insertions(+), 97 deletions(-)

-- 
2.3.2

[Buildroot] [PATCH v2 01/10] autobuild-run: fixup imports

[André Erdmann]

* drop "import ConfigParser", module gets imported as "configparser"
  See commit 16fcf29310bbb2533a98eb2247fc1415cb27e514

* drop "import argparse", currently unused

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 872ee9d..01f0957 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -121,8 +121,6 @@ import shutil
 from time import localtime, strftime
 import sys
 import hashlib
-import argparse
-import ConfigParser
 import docopt
 
 if sys.hexversion >= 0x3000000:
-- 
2.3.2

[Buildroot] [PATCH v2 02/10] autobuild-run, python3: dict.iteritems()->items()

[André Erdmann]

Some dict methods, like iteritems(), have been dropped in Python 3,
while the semantics of others, e.g. items(), has changed.

The Py3k variant of items() returns a "view" object, whereas the Py2k variant
returns a list, but both methods essentially do the same thing here.

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 01f0957..1edffc1 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -105,7 +105,7 @@ Default values for the arguments are:
 
   %s
 """ % '\n  '.join(
-    ['%s = %s' % (key, val) for (key, val) in defaults.iteritems()])
+    ['%s = %s' % (key, val) for (key, val) in defaults.items()])
 
 __doc__ = doc
 
-- 
2.3.2

[Buildroot] [PATCH v2 03/10] autobuild-run, python3: decode subprocess output

[André Erdmann]

subprocess.check_output() and its py 2.6 compat variant,
subprocess.Popen().communicate()[0], return a bytes object in Python 3,
but regexp.search() in get_failure_reason() wants a str:

  >>> re.search(r'.', b'a')
  TypeError: can't use a string pattern on a bytes-like object

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 1edffc1..e536289 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -576,8 +576,9 @@ def send_results(result, **kwargs):
 
     def get_failure_reason():
         # Output is a tuple (package, version), or None.
-        lastlines = subprocess.Popen(["tail", "-n", "3",
-                                      os.path.join(outputdir, "logfile")], stdout=subprocess.PIPE).communicate()[0].splitlines()
+        lastlines = decode_bytes(subprocess.Popen(
+            ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
+            stdout=subprocess.PIPE).communicate()[0]).splitlines()
 
         import re
         regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
-- 
2.3.2

[Buildroot] [PATCH v2 04/10] autobuild-run, python3: encode/decode mmap data

[André Erdmann]

mmap objects return bytes in Python 3,
which leads to errors when used in str context, for example:

  >>> fh      = open("file", "r")
  >>> fh_mmap = mmap.mmap(fh.fileno(), 0, access=ACCESS_READ)
  >>> fh_mmap.find("word")
  TypeError: 'str' does not support the buffer interface

Adds a new compat function, encode_str(), which is a no-op in Python 2.

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index e536289..40bd657 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -139,12 +139,16 @@ if sys.hexversion >= 0x3000000:
 
     def decode_byte_list(bl):
         return [b.decode() for b in bl]
+
+    def encode_str(s):
+        return s.encode()
 else:
     def _identity(e):
         return e
 
     decode_bytes = _identity
     decode_byte_list = _identity
+    encode_str = _identity
 
 MAX_DURATION = 60 * 60 * 4
 VERSION = 1
@@ -607,10 +611,10 @@ def send_results(result, **kwargs):
             mf = mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ)
             mf.seek(0)
             # Search for first action on the failed package
-            offset = mf.find('>>> %s' % ' '.join(reason))
+            offset = mf.find(encode_str('>>> %s' % ' '.join(reason)))
             if offset != -1:
                 with open(resultfile, "w") as endlog:
-                    endlog.write(mf[offset:])
+                    endlog.write(decode_bytes(mf[offset:]))
             else:
                 # not found, use last 500 lines as fallback
                 extract_last_500_lines()
-- 
2.3.2

[Buildroot] [PATCH v2 05/10] autobuild-run: avoid use of builtin as varname

[André Erdmann]

"file" is a type name in Python 2.

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 40bd657..38e5bae 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -637,10 +637,10 @@ def send_results(result, **kwargs):
         for root, dirs, files in os.walk(srcroot):
             dest = os.path.join(destroot, os.path.relpath(root, srcroot))
 
-            for file in files:
-                if file == 'config.log':
+            for fname in files:
+                if fname == 'config.log':
                     os.makedirs(dest)
-                    shutil.copy(os.path.join(root, file), os.path.join(dest, file))
+                    shutil.copy(os.path.join(root, fname), os.path.join(dest, fname))
 
     copy_config_log_files()
 
-- 
2.3.2

[Buildroot] [PATCH v2 06/10] autobuild-run: reorganize imports

[André Erdmann]

* move "re", "mmap" to the top of the file

* sort imports alphabetically

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 38e5bae..fd294d4 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -111,17 +111,19 @@ __doc__ = doc
 
 import contextlib
 import csv
-from random import randint
-import subprocess
+import docopt
+import errno
+import hashlib
+import mmap
 import multiprocessing
-import signal
 import os
-import errno
+from random import randint
+import re
 import shutil
-from time import localtime, strftime
+import signal
+import subprocess
 import sys
-import hashlib
-import docopt
+from time import localtime, strftime
 
 if sys.hexversion >= 0x3000000:
     import configparser
@@ -584,7 +586,6 @@ def send_results(result, **kwargs):
             ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
             stdout=subprocess.PIPE).communicate()[0]).splitlines()
 
-        import re
         regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
         for line in lastlines:
             m = regexp.search(line)
@@ -606,7 +607,6 @@ def send_results(result, **kwargs):
         if not reason:
             extract_last_500_lines()
         else:
-            import mmap
             f = open(os.path.join(outputdir, "logfile"), 'r')
             mf = mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ)
             mf.seek(0)
-- 
2.3.2

[Buildroot] [PATCH v2 07/10] autobuild-run: move check_requirements() to SystemInfo

[André Erdmann]

* build up the list of (additional) essential programs in main()

* can check for optional progs in fixup_config() without
  having to list them in sysinfo

* it'd be quite easy to dump the prog list to the logfile
  (for name, have_prog in sysinfo.progs.items(): ...)

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 123 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 73 insertions(+), 50 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index fd294d4..409570d 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -47,8 +47,6 @@
 # - Instead of excluding all configurations that have
 #   BR2_PACKAGE_CLASSPATH=y, improve the script to detect whether the
 #   necessary host machine requirements are there to build classpath.
-# - Integrate method check-requirements with the SysInfo class, distinghuishing
-#   between required and optional dependencies.
 # - Extend required dependencies to subversion, mercurial, cpio, wget, python,
 #   etc.
 # - Detect selection of multiple virtual package providers and don't consider it
@@ -166,51 +164,72 @@ def check_version():
         print("ERROR: script version too old, please upgrade.")
         sys.exit(1)
 
-def has_prog(name, flags=os.X_OK, env=os.environ):
-    if not name or name[0] == os.sep: raise ValueError(name)
-
-    prog_path = env.get("PATH", None)
-    # for windows compatibility, we'd need to take PATHEXT into account
-
-    if prog_path:
-        for prog_dir in filter(None, prog_path.split(os.pathsep)):
-            # os.join() not necessary: non-empty prog_dir and name[0] != os.sep
-            prog = prog_dir + os.sep + name
-            if os.access(prog, flags):
-                return True
-    # --
-    return False
-
-def check_requirements(upload=False):
-    needed_progs = ["make", "git", "gcc", "timeout"]
-    missing_requirements = False
-
-    if upload:
-        needed_progs.append("curl")
-
-    for prog in needed_progs:
-        if not has_prog(prog):
-            print("ERROR: your system lacks the '%s' program" % prog)
-            missing_requirements = True
-
-    if missing_requirements:
-        sys.exit(1)
-
 class SystemInfo:
-    def __init__(self):
-        devnull = open(os.devnull, "w")
-        # _grep_gcj :: str -> int
-        _grep_gcj = lambda prog: \
-            subprocess.call("%s -version | grep gcj" % prog, shell=True,
-                            stdout=devnull, stderr=devnull)
-
-        self.has_bzr = has_prog("bzr")
-
-        self.has_java = has_prog("java") and _grep_gcj("java") == 1
+    DEFAULT_NEEDED_PROGS = ["make", "git", "gcc", "timeout"]
+    DEFAULT_OPTIONAL_PROGS = ["bzr", "java", "javac", "jar"]
 
-        self.has_javac = has_prog("javac") and _grep_gcj("javac") == 1
+    def __init__(self):
+        self.needed_progs = list(self.__class__.DEFAULT_NEEDED_PROGS)
+        self.optional_progs = list(self.__class__.DEFAULT_OPTIONAL_PROGS)
+        self.progs = {}
+
+    def find_prog(self, name, flags=os.X_OK, env=os.environ):
+        if not name or name[0] == os.sep: raise ValueError(name)
+
+        prog_path = env.get("PATH", None)
+        # for windows compatibility, we'd need to take PATHEXT into account
+
+        if prog_path:
+            for prog_dir in filter(None, prog_path.split(os.pathsep)):
+                # os.join() not necessary: non-empty prog_dir
+                # and name[0] != os.sep
+                prog = prog_dir + os.sep + name
+                if os.access(prog, flags):
+                    return prog
+        # --
+        return None
 
-        self.has_jar = has_prog("jar")
+    def has(self, prog):
+        """Checks whether a program is available.
+        Lazily evaluates missing entries.
+
+        Returns: None if prog not found, else path to the program [evaluates to True]
+        """
+        try:
+            return self.progs[prog]
+        except KeyError:
+            pass
+
+        have_it = self.find_prog(prog)
+        # java[c] needs special care
+        if have_it and prog in ('java', 'javac'):
+            with open(os.devnull, "w") as devnull:
+                if subprocess.call("%s -version | grep gcj" % prog, shell=True,
+                                   stdout=devnull, stderr=devnull) != 1:
+                    have_it = False
+        # --
+        self.progs[prog] = have_it
+        return have_it
+
+    def check_requirements(self):
+        """Checks program dependencies.
+
+        Returns: True if all mandatory programs are present, else False.
+        """
+        do_check_has_prog = self.has
+
+        missing_requirements = False
+        for prog in self.needed_progs:
+            if not do_check_has_prog(prog):
+                print("ERROR: your system lacks the '%s' program" % prog)
+                missing_requirements = True
+
+        # check optional programs here,
+        # else they'd get checked by each worker instance
+        for prog in self.optional_progs:
+            do_check_has_prog(prog)
+
+        return not missing_requirements
 
 def get_toolchain_configs():
     """Fetch and return the possible toolchain configurations
@@ -338,14 +357,14 @@ def fixup_config(**kwargs):
     if "BR2_PACKAGE_MROUTED=y\n" in configlines and \
        "BR2_TOOLCHAIN_USES_UCLIBC=y\n" in configlines:
         configlines.remove("BR2_PACKAGE_MROUTED=y\n")
-    if "BR2_NEEDS_HOST_JAVA=y\n" in configlines and not sysinfo.has_java:
+    if "BR2_NEEDS_HOST_JAVA=y\n" in configlines and not sysinfo.has("java"):
         return False
-    if "BR2_NEEDS_HOST_JAVAC=y\n" in configlines and not sysinfo.has_javac:
+    if "BR2_NEEDS_HOST_JAVAC=y\n" in configlines and not sysinfo.has("javac"):
         return False
-    if "BR2_NEEDS_HOST_JAR=y\n" in configlines and not sysinfo.has_jar:
+    if "BR2_NEEDS_HOST_JAR=y\n" in configlines and not sysinfo.has("jar"):
         return False
     # python-nfc needs bzr
-    if 'BR2_PACKAGE_PYTHON_NFC=y\n' in configlines and not sysinfo.has_bzr:
+    if 'BR2_PACKAGE_PYTHON_NFC=y\n' in configlines and not sysinfo.has("bzr"):
         return False
     # The ctng toolchain is affected by PR58854
     if 'BR2_PACKAGE_LTTNG_TOOLS=y\n' in configlines and \
@@ -771,11 +790,15 @@ def main():
     # explicitly on None.
     upload = (args['--http-login'] is not None) \
              and (args['--http-password'] is not None)
-    check_requirements(upload)
-    if not upload:
+    if upload:
+        sysinfo.needed_progs.append("curl")
+    else:
         print("WARN: due to the lack of http login/password details, results will not be submitted")
         print("WARN: tarballs of results will be kept locally only")
 
+    if not sysinfo.check_requirements():
+        sys.exit(1)
+
     def sigterm_handler(signum, frame):
         """Kill all children"""
 
-- 
2.3.2

[Buildroot] [PATCH v2 08/10] autobuild-run: encapsulate subprocess calls

[André Erdmann]

Preparation step for passing LANG to worker (sub-)processes,
allows to redirect stdin/stdout/stderr, which all default to devnull now
unless specified otherwise.
This makes the "yes"-pipe in "make oldconfig" redundant.

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 92 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 55 insertions(+), 37 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 409570d..0ee6d57 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -172,6 +172,7 @@ class SystemInfo:
         self.needed_progs = list(self.__class__.DEFAULT_NEEDED_PROGS)
         self.optional_progs = list(self.__class__.DEFAULT_OPTIONAL_PROGS)
         self.progs = {}
+        self.devnull = open(os.devnull, "w")
 
     def find_prog(self, name, flags=os.X_OK, env=os.environ):
         if not name or name[0] == os.sep: raise ValueError(name)
@@ -203,10 +204,8 @@ class SystemInfo:
         have_it = self.find_prog(prog)
         # java[c] needs special care
         if have_it and prog in ('java', 'javac'):
-            with open(os.devnull, "w") as devnull:
-                if subprocess.call("%s -version | grep gcj" % prog, shell=True,
-                                   stdout=devnull, stderr=devnull) != 1:
-                    have_it = False
+            if self.run_cmd("%s -version | grep gcj" % prog, shell=True) != 1:
+                have_it = False
         # --
         self.progs[prog] = have_it
         return have_it
@@ -231,6 +230,27 @@ class SystemInfo:
 
         return not missing_requirements
 
+    def popen(self, cmdv, **kwargs):
+        kwargs.setdefault('stdin', self.devnull)
+        kwargs.setdefault('stdout', self.devnull)
+        kwargs.setdefault('stderr', self.devnull)
+        return subprocess.Popen(cmdv, **kwargs)
+
+    def run_cmd(self, cmdv, **kwargs):
+        proc = self.popen(cmdv, **kwargs)
+        proc.communicate()
+        return proc.poll()
+
+    def run_cmd_write_to(self, outstream, cmdv, **kwargs):
+        kwargs.update(stdout=outstream, stderr=outstream)
+        return self.run_cmd(cmdv, **kwargs)
+
+    def run_cmd_get_stdout(self, cmdv, **kwargs):
+        proc = self.popen(cmdv, stdout=subprocess.PIPE, **kwargs)
+        stdout_data, _ = proc.communicate()
+        return (proc.poll(), decode_bytes(stdout_data) if stdout_data else None)
+
+
 def get_toolchain_configs():
     """Fetch and return the possible toolchain configurations
 
@@ -274,6 +294,7 @@ def prepare_build(**kwargs):
 
     idir = "instance-%d" % kwargs['instance']
     log = kwargs['log']
+    sysinfo = kwargs['sysinfo']
 
     log_write(log, "INFO: preparing a new build")
 
@@ -298,15 +319,15 @@ def prepare_build(**kwargs):
     # didn't exist already.
     srcdir = os.path.join(idir, "buildroot")
     if not os.path.exists(srcdir):
-        ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
-                              stdout=log, stderr=log)
+        ret = sysinfo.run_cmd_write_to(
+            log, ["git", "clone", "git://git.busybox.net/buildroot", srcdir])
         if ret != 0:
             log_write(log, "ERROR: could not clone Buildroot sources")
             return -1
 
     # Update the Buildroot sources.
     abssrcdir = os.path.abspath(srcdir)
-    ret = subprocess.call(["git", "pull"], cwd=abssrcdir, stdout=log, stderr=log)
+    ret = sysinfo.run_cmd_write_to(log, ["git", "pull"],  cwd=abssrcdir)
     if ret != 0:
         log_write(log, "ERROR: could not pull Buildroot sources")
         return -1
@@ -315,7 +336,7 @@ def prepare_build(**kwargs):
     outputdir = os.path.join(idir, "output")
     if os.path.exists(outputdir):
         # shutil.rmtree doesn't remove write-protected files
-        subprocess.call(["rm", "-rf", outputdir])
+        sysinfo.run_cmd(["rm", "-rf", outputdir])
     os.mkdir(outputdir)
 
     return 0
@@ -459,6 +480,7 @@ def gen_config(**kwargs):
 
     idir = "instance-%d" % kwargs['instance']
     log = kwargs['log']
+    sysinfo = kwargs['sysinfo']
 
     # We need the absolute path to use with O=, because the relative
     # path to the output directory here is not relative to the
@@ -493,10 +515,7 @@ def gen_config(**kwargs):
     with open(os.path.join(outputdir, ".config"), "w+") as configf:
         configf.writelines(configlines)
 
-    devnull = open(os.devnull, "w")
-
-    ret = subprocess.call(["yes '' 2>/dev/null| make O=%s -C %s oldconfig" % \
-                           (outputdir, srcdir)], shell=True, stdout=devnull, stderr=devnull)
+    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "oldconfig"])
     if ret != 0:
         log_write(log, "ERROR: cannot oldconfig")
         return -1
@@ -504,23 +523,20 @@ def gen_config(**kwargs):
     # Now, generate the random selection of packages, and fixup
     # things if needed.
     while True:
-        ret = subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir,
-                               "KCONFIG_PROBABILITY=%d" % randint(1,30), "randpackageconfig"],
-                              stdout=devnull, stderr=devnull)
+        ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir,
+                               "KCONFIG_PROBABILITY=%d" % randint(1,30), "randpackageconfig"])
         if ret != 0:
             log_write(log, "ERROR: cannot generate random configuration")
             return -1
         if fixup_config(**kwargs):
             break
 
-    ret = subprocess.call(["yes '' 2>/dev/null| make O=%s -C %s oldconfig" % \
-                           (outputdir, srcdir)], shell=True, stdout=devnull, stderr=devnull)
+    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "oldconfig"])
     if ret != 0:
         log_write(log, "ERROR: cannot oldconfig")
         return -1
 
-    ret = subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir, "savedefconfig"],
-                          stdout=devnull, stderr=devnull)
+    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "savedefconfig"])
     if ret != 0:
         log_write(log, "ERROR: cannot savedefconfig")
         return -1
@@ -532,6 +548,7 @@ def do_build(**kwargs):
 
     idir = "instance-%d" % kwargs['instance']
     log = kwargs['log']
+    sysinfo = kwargs['sysinfo']
 
     # We need the absolute path to use with O=, because the relative
     # path to the output directory here is not relative to the
@@ -547,7 +564,7 @@ def do_build(**kwargs):
             "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
             "BR2_JLEVEL=%s" % kwargs['njobs']] \
           + kwargs['make_opts'].split()
-    sub = subprocess.Popen(cmd, stdout=f, stderr=f)
+    sub = sysinfo.popen(cmd, stdout=f, stderr=f)
     kwargs['buildpid'][kwargs['instance']] = sub.pid
     ret = sub.wait()
     kwargs['buildpid'][kwargs['instance']] = 0
@@ -560,7 +577,7 @@ def do_build(**kwargs):
     if ret != 0:
         log_write(log, "INFO: build failed")
         return -1
-    ret = subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir], stdout=f, stderr=f)
+    ret = sysinfo.run_cmd_write_to(f, ["make", "O=%s" % outputdir, "-C", srcdir])
     if ret != 0:
         log_write(log, "INFO: build failed during legal-info")
         return -1
@@ -577,6 +594,7 @@ def send_results(result, **kwargs):
 
     idir = "instance-%d" % kwargs['instance']
     log = kwargs['log']
+    sysinfo = kwargs['sysinfo']
 
     outputdir = os.path.abspath(os.path.join(idir, "output"))
     srcdir = os.path.join(idir, "buildroot")
@@ -595,15 +613,14 @@ def send_results(result, **kwargs):
         shutil.copyfile(os.path.join(outputdir, "legal-info", "manifest.csv"),
                         os.path.join(resultdir, "licenses-manifest.csv"))
 
-    subprocess.call(["git log master -n 1 --pretty=format:%%H > %s" % \
+    sysinfo.run_cmd(["git log master -n 1 --pretty=format:%%H > %s" % \
                      os.path.join(resultdir, "gitid")],
-                    shell=True, cwd=srcdir)
+                    shell=True, cwd=srcdir, stderr=None)
 
     def get_failure_reason():
         # Output is a tuple (package, version), or None.
-        lastlines = decode_bytes(subprocess.Popen(
-            ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
-            stdout=subprocess.PIPE).communicate()[0]).splitlines()
+        lastlines = sysinfo.run_cmd_get_stdout(
+            ["tail", "-n", "3", os.path.join(outputdir, "logfile")])[1].splitlines()
 
         regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
         for line in lastlines:
@@ -618,9 +635,9 @@ def send_results(result, **kwargs):
         """Save the last part of the build log, starting from the failed package"""
 
         def extract_last_500_lines():
-            subprocess.call(["tail -500 %s > %s" % \
+            sysinfo.run_cmd(["tail -500 %s > %s" % \
                              (os.path.join(outputdir, "logfile"), resultfile)],
-                            shell=True)
+                            shell=True, stderr=None)
 
         reason = get_failure_reason()
         if not reason:
@@ -677,8 +694,8 @@ def send_results(result, **kwargs):
 
     # Yes, shutil.make_archive() would be nice, but it doesn't exist
     # in Python 2.6.
-    ret = subprocess.call(["tar", "cjf", "results.tar.bz2", "results"],
-                          cwd=outputdir, stdout=log, stderr=log)
+    ret = sysinfo.run_cmd_write_to(
+        log, ["tar", "cjf", "results.tar.bz2", "results"], cwd=outputdir)
     if ret != 0:
         log_write(log, "ERROR: could not make results tarball")
         sys.exit(1)
@@ -687,13 +704,14 @@ def send_results(result, **kwargs):
         # Submit results. Yes, Python has some HTTP libraries, but
         # none of the ones that are part of the standard library can
         # upload a file without writing dozens of lines of code.
-        ret = subprocess.call(["curl", "-u",
-                               "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
-                               "-H", "Expect:",
-                               "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
-                               "-F", "uploadsubmit=1",
-                               "http://autobuild.buildroot.org/submit/"],
-                              stdout=log, stderr=log)
+        ret = sysinfo.run_cmd_write_to(
+            log,
+            ["curl", "-u",
+             "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
+             "-H", "Expect:",
+             "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
+             "-F", "uploadsubmit=1",
+             "http://autobuild.buildroot.org/submit/"])
         if ret != 0:
             log_write(log, "INFO: results could not be submitted, %d" % ret)
         else:
-- 
2.3.2

[Buildroot] [PATCH v2 09/10] autobuild-run: control subprocess env

[André Erdmann]

Preparation step for passing LANG to worker (sub-)processes,
allows to control execution environment of subprocesses
without affecting the "autobuild-run" process itself.

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---
 scripts/autobuild-run | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 0ee6d57..e1a3398 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -173,11 +173,12 @@ class SystemInfo:
         self.optional_progs = list(self.__class__.DEFAULT_OPTIONAL_PROGS)
         self.progs = {}
         self.devnull = open(os.devnull, "w")
+        self.env = os.environ.copy()
 
-    def find_prog(self, name, flags=os.X_OK, env=os.environ):
+    def find_prog(self, name, flags=os.X_OK):
         if not name or name[0] == os.sep: raise ValueError(name)
 
-        prog_path = env.get("PATH", None)
+        prog_path = self.env.get("PATH", None)
         # for windows compatibility, we'd need to take PATHEXT into account
 
         if prog_path:
@@ -234,6 +235,7 @@ class SystemInfo:
         kwargs.setdefault('stdin', self.devnull)
         kwargs.setdefault('stdout', self.devnull)
         kwargs.setdefault('stderr', self.devnull)
+        kwargs['env'] = self.env
         return subprocess.Popen(cmdv, **kwargs)
 
     def run_cmd(self, cmdv, **kwargs):
-- 
2.3.2

[Buildroot] [PATCH v2 10/10] autobuild-run: set locale to en_US or C

[André Erdmann]

some python scripts break if the locale is set to C, try en_US{.UTF-8,} first

Additionally, drop all locale env vars (LC_*, LANG[GUAGE]) when
setting the new locale.

Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
---

Just for reference, a small python example that breaks if LANG is set to C:

  $ LANG=C python -c "print(u'Andr\xe9')"
  UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 4: ordinal not in range(128)

---
 scripts/autobuild-run | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index e1a3398..a1e947d 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -168,6 +168,12 @@ class SystemInfo:
     DEFAULT_NEEDED_PROGS = ["make", "git", "gcc", "timeout"]
     DEFAULT_OPTIONAL_PROGS = ["bzr", "java", "javac", "jar"]
 
+    # list of default locales (in lowercase, without "-", descending order)
+    #  some python scripts break if the locale is set to C, try en_US first
+    TRY_LOCALES = ['en_us.utf8', 'en_us', 'c']
+    # list of locale environment variables that should be (re-)set by set_locale()
+    LOCALE_KEYS = ['LANG']
+
     def __init__(self):
         self.needed_progs = list(self.__class__.DEFAULT_NEEDED_PROGS)
         self.optional_progs = list(self.__class__.DEFAULT_OPTIONAL_PROGS)
@@ -231,6 +237,41 @@ class SystemInfo:
 
         return not missing_requirements
 
+    def set_locale(self):
+        def is_locale_env_varname(w):
+            # w[:4] == 'LANG' and (not w[4:] or w[4:] == 'UAGE') ...
+            return w[:3] == 'LC_' or w == 'LANG' or w == 'LANGUAGE'
+
+        ret, locales_str = self.run_cmd_get_stdout(["locale", "-a"])
+        if ret != os.EX_OK:
+            return False
+
+        # create a dict
+        #   <locale identifier> (as listed in TRY_LOCALES) => <locale env name>
+        locales = dict((
+            (k.lower().replace("-", ""), k) for k in locales_str.split(None)
+        ))
+
+        for loc_key in filter(lambda x: x in locales, self.TRY_LOCALES):
+            # cannot modify self.env while iterating over it,
+            #  create intermediate list
+            env_old_locale_keys = [
+                k for k in self.env.keys() if is_locale_env_varname(k)
+            ]
+            for k in env_old_locale_keys:
+                del self.env[k]
+
+            # set new locale once
+            for vname in self.LOCALE_KEYS:
+                self.env[vname] = locales[loc_key]
+            return True
+        # -- end for
+        # practically impossible to reach this return if 'c' is in TRY_LOCALES:
+        return None
+
+    def sanitize_env(self):
+        self.set_locale()
+
     def popen(self, cmdv, **kwargs):
         kwargs.setdefault('stdin', self.devnull)
         kwargs.setdefault('stdout', self.devnull)
@@ -789,12 +830,11 @@ def merge(dict_1, dict_2):
 
 def main():
 
-    # Avoid locale settings of autobuilder machine leaking in, for example
-    # showing error messages in another language.
-    os.environ['LC_ALL'] = 'C'
-
     check_version()
     sysinfo = SystemInfo()
+    # Avoid locale settings of autobuilder machine leaking in, for example
+    # showing error messages in another language.
+    sysinfo.sanitize_env()
 
     args = docopt.docopt(doc, version=VERSION)
 
-- 
2.3.2

[Buildroot] [PATCH v2 00/10] autobuild-run: python3 compat and misc improvements

[Thomas Petazzoni]

Dear Andr? Erdmann,

On Wed, 18 Mar 2015 16:50:33 +0100, Andr? Erdmann wrote:

> Andr? Erdmann (10):
>   autobuild-run: fixup imports
>   autobuild-run, python3: dict.iteritems()->items()
>   autobuild-run, python3: decode subprocess output
>   autobuild-run, python3: encode/decode mmap data
>   autobuild-run: avoid use of builtin as varname
>   autobuild-run: reorganize imports
>   autobuild-run: move check_requirements() to SystemInfo

Those 7 first patches applied.

>   autobuild-run: encapsulate subprocess calls
>   autobuild-run: control subprocess env
>   autobuild-run: set locale to en_US or C

I'll need more time (or help from someone else) to review these changes.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2 08/10] autobuild-run: encapsulate subprocess calls

[Samuel Martin]

Hi Andr?, Thomas, all,

On Wed, Mar 18, 2015 at 4:50 PM, Andr? Erdmann <dywi@mailerd.de> wrote:
> Preparation step for passing LANG to worker (sub-)processes,
> allows to redirect stdin/stdout/stderr, which all default to devnull now
> unless specified otherwise.
> This makes the "yes"-pipe in "make oldconfig" redundant.
>
> Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
[...]
>
>      return 0
> @@ -459,6 +480,7 @@ def gen_config(**kwargs):
>
>      idir = "instance-%d" % kwargs['instance']
>      log = kwargs['log']
> +    sysinfo = kwargs['sysinfo']
>
>      # We need the absolute path to use with O=, because the relative
>      # path to the output directory here is not relative to the
> @@ -493,10 +515,7 @@ def gen_config(**kwargs):
>      with open(os.path.join(outputdir, ".config"), "w+") as configf:
>          configf.writelines(configlines)
>
> -    devnull = open(os.devnull, "w")
> -
> -    ret = subprocess.call(["yes '' 2>/dev/null| make O=%s -C %s oldconfig" % \
> -                           (outputdir, srcdir)], shell=True, stdout=devnull, stderr=devnull)
> +    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "oldconfig"])
Any reason to drop the yes pipe in the command?

>      if ret != 0:
>          log_write(log, "ERROR: cannot oldconfig")
>          return -1
> @@ -504,23 +523,20 @@ def gen_config(**kwargs):
>      # Now, generate the random selection of packages, and fixup
>      # things if needed.
>      while True:
> -        ret = subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir,
> -                               "KCONFIG_PROBABILITY=%d" % randint(1,30), "randpackageconfig"],
> -                              stdout=devnull, stderr=devnull)
> +        ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir,
> +                               "KCONFIG_PROBABILITY=%d" % randint(1,30), "randpackageconfig"])
>          if ret != 0:
>              log_write(log, "ERROR: cannot generate random configuration")
>              return -1
>          if fixup_config(**kwargs):
>              break
>
> -    ret = subprocess.call(["yes '' 2>/dev/null| make O=%s -C %s oldconfig" % \
> -                           (outputdir, srcdir)], shell=True, stdout=devnull, stderr=devnull)
> +    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "oldconfig"])
ditto

>      if ret != 0:
>          log_write(log, "ERROR: cannot oldconfig")
>          return -1
>
> -    ret = subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir, "savedefconfig"],
> -                          stdout=devnull, stderr=devnull)
> +    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "savedefconfig"])
>      if ret != 0:
>          log_write(log, "ERROR: cannot savedefconfig")
>          return -1
[...]
> @@ -595,15 +613,14 @@ def send_results(result, **kwargs):
>          shutil.copyfile(os.path.join(outputdir, "legal-info", "manifest.csv"),
>                          os.path.join(resultdir, "licenses-manifest.csv"))
>
> -    subprocess.call(["git log master -n 1 --pretty=format:%%H > %s" % \
> +    sysinfo.run_cmd(["git log master -n 1 --pretty=format:%%H > %s" % \
>                       os.path.join(resultdir, "gitid")],
> -                    shell=True, cwd=srcdir)
> +                    shell=True, cwd=srcdir, stderr=None)
IIRC, stderr=None dumps the error output in the process error output,
so only available for the autobuilder owner.
So, how about log stderr in the log file using
sysinfo.run_cmd_get_output (stdout would be anyway redirected to
gitid, and stderr would be part of the log)?

>
>      def get_failure_reason():
>          # Output is a tuple (package, version), or None.
> -        lastlines = decode_bytes(subprocess.Popen(
> -            ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
> -            stdout=subprocess.PIPE).communicate()[0]).splitlines()
> +        lastlines = sysinfo.run_cmd_get_stdout(
> +            ["tail", "-n", "3", os.path.join(outputdir, "logfile")])[1].splitlines()
>
>          regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
>          for line in lastlines:
> @@ -618,9 +635,9 @@ def send_results(result, **kwargs):
>          """Save the last part of the build log, starting from the failed package"""
>
>          def extract_last_500_lines():
> -            subprocess.call(["tail -500 %s > %s" % \
> +            sysinfo.run_cmd(["tail -500 %s > %s" % \
>                               (os.path.join(outputdir, "logfile"), resultfile)],
> -                            shell=True)
> +                            shell=True, stderr=None)
ditto

>
>          reason = get_failure_reason()
>          if not reason:
> @@ -677,8 +694,8 @@ def send_results(result, **kwargs):
>
>      # Yes, shutil.make_archive() would be nice, but it doesn't exist
>      # in Python 2.6.
> -    ret = subprocess.call(["tar", "cjf", "results.tar.bz2", "results"],
> -                          cwd=outputdir, stdout=log, stderr=log)
> +    ret = sysinfo.run_cmd_write_to(
> +        log, ["tar", "cjf", "results.tar.bz2", "results"], cwd=outputdir)
>      if ret != 0:
>          log_write(log, "ERROR: could not make results tarball")
>          sys.exit(1)
> @@ -687,13 +704,14 @@ def send_results(result, **kwargs):
>          # Submit results. Yes, Python has some HTTP libraries, but
>          # none of the ones that are part of the standard library can
>          # upload a file without writing dozens of lines of code.
> -        ret = subprocess.call(["curl", "-u",
> -                               "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
> -                               "-H", "Expect:",
> -                               "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
> -                               "-F", "uploadsubmit=1",
> -                               "http://autobuild.buildroot.org/submit/"],
> -                              stdout=log, stderr=log)
> +        ret = sysinfo.run_cmd_write_to(
> +            log,
> +            ["curl", "-u",
> +             "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
> +             "-H", "Expect:",
> +             "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
> +             "-F", "uploadsubmit=1",
> +             "http://autobuild.buildroot.org/submit/"])
>          if ret != 0:
>              log_write(log, "INFO: results could not be submitted, %d" % ret)
>          else:
> --
> 2.3.2
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Regards,

-- 
Samuel

[Buildroot] [PATCH v2 09/10] autobuild-run: control subprocess env

[Samuel Martin]

Hi Andr?, Thomas, all,

On Wed, Mar 18, 2015 at 4:50 PM, Andr? Erdmann <dywi@mailerd.de> wrote:
> Preparation step for passing LANG to worker (sub-)processes,
> allows to control execution environment of subprocesses
> without affecting the "autobuild-run" process itself.
>
> Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>

Regards,

-- 
Samuel

[Buildroot] [PATCH v2 10/10] autobuild-run: set locale to en_US or C

[Samuel Martin]

Hi Andr?, Thomas, all,

On Wed, Mar 18, 2015 at 4:50 PM, Andr? Erdmann <dywi@mailerd.de> wrote:
> some python scripts break if the locale is set to C, try en_US{.UTF-8,} first
Could you give an example of such a script?

>
> Additionally, drop all locale env vars (LC_*, LANG[GUAGE]) when
> setting the new locale.
>
> Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
> ---
>
> Just for reference, a small python example that breaks if LANG is set to C:
>
>   $ LANG=C python -c "print(u'Andr\xe9')"
>   UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 4: ordinal not in range(128)
>
> ---
>  scripts/autobuild-run | 48 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index e1a3398..a1e947d 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -168,6 +168,12 @@ class SystemInfo:
>      DEFAULT_NEEDED_PROGS = ["make", "git", "gcc", "timeout"]
>      DEFAULT_OPTIONAL_PROGS = ["bzr", "java", "javac", "jar"]
>
> +    # list of default locales (in lowercase, without "-", descending order)
> +    #  some python scripts break if the locale is set to C, try en_US first
> +    TRY_LOCALES = ['en_us.utf8', 'en_us', 'c']
> +    # list of locale environment variables that should be (re-)set by set_locale()
> +    LOCALE_KEYS = ['LANG']
> +
>      def __init__(self):
>          self.needed_progs = list(self.__class__.DEFAULT_NEEDED_PROGS)
>          self.optional_progs = list(self.__class__.DEFAULT_OPTIONAL_PROGS)
> @@ -231,6 +237,41 @@ class SystemInfo:
>
>          return not missing_requirements
>
> +    def set_locale(self):
> +        def is_locale_env_varname(w):
> +            # w[:4] == 'LANG' and (not w[4:] or w[4:] == 'UAGE') ...
Hmm... I must say the comment is rather confusing!

> +            return w[:3] == 'LC_' or w == 'LANG' or w == 'LANGUAGE'
or: return w.startswith('LC_') or w in ['LANG', 'LANGUAGE']

> +
> +        ret, locales_str = self.run_cmd_get_stdout(["locale", "-a"])[
> +        if ret != os.EX_OK:
> +            return False
> +
> +        # create a dict
> +        #   <locale identifier> (as listed in TRY_LOCALES) => <locale env name>
> +        locales = dict((
> +            (k.lower().replace("-", ""), k) for k in locales_str.split(None)
s/None//

> +        ))
> +
> +        for loc_key in filter(lambda x: x in locales, self.TRY_LOCALES):
Is a for loop really needed here?
You could just do:
  try:[
    loc_key = filter(lambda x: x in locales, self.TRY_LOCALES)[0]
  except (TypeError, IndexError):
    # set "C" as locale
    loc_key = self.TRY_LOCALES[-1]

> +            # cannot modify self.env while iterating over it,
> +            #  create intermediate list
> +            env_old_locale_keys = [
> +                k for k in self.env.keys() if is_locale_env_varname(k)
> +            ]
> +            for k in env_old_locale_keys:
> +                del self.env[k]
Setting the new value will automatically override the old value, so
deleting it before looks a bit overkill...

> +
> +            # set new locale once
> +            for vname in self.LOCALE_KEYS:
> +                self.env[vname] = locales[loc_key]
> +            return True
> +        # -- end for
> +        # practically impossible to reach this return if 'c' is in TRY_LOCALES:
> +        return None
> +
> +    def sanitize_env(self):
> +        self.set_locale()
> +
>      def popen(self, cmdv, **kwargs):
>          kwargs.setdefault('stdin', self.devnull)
>          kwargs.setdefault('stdout', self.devnull)
> @@ -789,12 +830,11 @@ def merge(dict_1, dict_2):
>
>  def main():
>
> -    # Avoid locale settings of autobuilder machine leaking in, for example
> -    # showing error messages in another language.
> -    os.environ['LC_ALL'] = 'C'
> -
>      check_version()
>      sysinfo = SystemInfo()
> +    # Avoid locale settings of autobuilder machine leaking in, for example
> +    # showing error messages in another language.
> +    sysinfo.sanitize_env()
>
>      args = docopt.docopt(doc, version=VERSION)
>
> --
> 2.3.2
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Regards,

-- 
Samuel

[Buildroot] [PATCH v2 08/10] autobuild-run: encapsulate subprocess calls

[André Erdmann]

Hi,

2015-04-12 9:51 GMT+02:00 Samuel Martin <s.martin49@gmail.com>:
> Hi Andr?, Thomas, all,
>
> On Wed, Mar 18, 2015 at 4:50 PM, Andr? Erdmann <dywi@mailerd.de> wrote:
>> Preparation step for passing LANG to worker (sub-)processes,
>> allows to redirect stdin/stdout/stderr, which all default to devnull now
>> unless specified otherwise.
>> This makes the "yes"-pipe in "make oldconfig" redundant.
>>
>> Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
> [...]
>>
>>      return 0
>> @@ -459,6 +480,7 @@ def gen_config(**kwargs):
>>
>>      idir = "instance-%d" % kwargs['instance']
>>      log = kwargs['log']
>> +    sysinfo = kwargs['sysinfo']
>>
>>      # We need the absolute path to use with O=, because the relative
>>      # path to the output directory here is not relative to the
>> @@ -493,10 +515,7 @@ def gen_config(**kwargs):
>>      with open(os.path.join(outputdir, ".config"), "w+") as configf:
>>          configf.writelines(configlines)
>>
>> -    devnull = open(os.devnull, "w")
>> -
>> -    ret = subprocess.call(["yes '' 2>/dev/null| make O=%s -C %s oldconfig" % \
>> -                           (outputdir, srcdir)], shell=True, stdout=devnull, stderr=devnull)
>> +    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "oldconfig"])
> Any reason to drop the yes pipe in the command?
>

It's not needed anymore: SystemInfo::run_cmd() redirects stdin to
/dev/null, so this command reads as "make ... oldconfig < /dev/null",
which behaves identical to "yes '' | make ... oldconfig".

>>      if ret != 0:
>>          log_write(log, "ERROR: cannot oldconfig")
>>          return -1
>> @@ -504,23 +523,20 @@ def gen_config(**kwargs):
>>      # Now, generate the random selection of packages, and fixup
>>      # things if needed.
>>      while True:
>> -        ret = subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir,
>> -                               "KCONFIG_PROBABILITY=%d" % randint(1,30), "randpackageconfig"],
>> -                              stdout=devnull, stderr=devnull)
>> +        ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir,
>> +                               "KCONFIG_PROBABILITY=%d" % randint(1,30), "randpackageconfig"])
>>          if ret != 0:
>>              log_write(log, "ERROR: cannot generate random configuration")
>>              return -1
>>          if fixup_config(**kwargs):
>>              break
>>
>> -    ret = subprocess.call(["yes '' 2>/dev/null| make O=%s -C %s oldconfig" % \
>> -                           (outputdir, srcdir)], shell=True, stdout=devnull, stderr=devnull)
>> +    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "oldconfig"])
> ditto
>
>>      if ret != 0:
>>          log_write(log, "ERROR: cannot oldconfig")
>>          return -1
>>
>> -    ret = subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir, "savedefconfig"],
>> -                          stdout=devnull, stderr=devnull)
>> +    ret = sysinfo.run_cmd(["make", "O=%s" % outputdir, "-C", srcdir, "savedefconfig"])
>>      if ret != 0:
>>          log_write(log, "ERROR: cannot savedefconfig")
>>          return -1
> [...]
>> @@ -595,15 +613,14 @@ def send_results(result, **kwargs):
>>          shutil.copyfile(os.path.join(outputdir, "legal-info", "manifest.csv"),
>>                          os.path.join(resultdir, "licenses-manifest.csv"))
>>
>> -    subprocess.call(["git log master -n 1 --pretty=format:%%H > %s" % \
>> +    sysinfo.run_cmd(["git log master -n 1 --pretty=format:%%H > %s" % \
>>                       os.path.join(resultdir, "gitid")],
>> -                    shell=True, cwd=srcdir)
>> +                    shell=True, cwd=srcdir, stderr=None)
> IIRC, stderr=None dumps the error output in the process error output,
> so only available for the autobuilder owner.
> So, how about log stderr in the log file using
> sysinfo.run_cmd_get_output (stdout would be anyway redirected to
> gitid, and stderr would be part of the log)?
>

Sure! But I'd rather do that in a follow-up patch since this one tries
to keep the functional changes at a minimum, except for the
stdin-/dev/null redirection. (The "git log" command used to write to
console before this series, same applies to the "tar" command below.)

>>
>>      def get_failure_reason():
>>          # Output is a tuple (package, version), or None.
>> -        lastlines = decode_bytes(subprocess.Popen(
>> -            ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
>> -            stdout=subprocess.PIPE).communicate()[0]).splitlines()
>> +        lastlines = sysinfo.run_cmd_get_stdout(
>> +            ["tail", "-n", "3", os.path.join(outputdir, "logfile")])[1].splitlines()
>>
>>          regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
>>          for line in lastlines:
>> @@ -618,9 +635,9 @@ def send_results(result, **kwargs):
>>          """Save the last part of the build log, starting from the failed package"""
>>
>>          def extract_last_500_lines():
>> -            subprocess.call(["tail -500 %s > %s" % \
>> +            sysinfo.run_cmd(["tail -500 %s > %s" % \
>>                               (os.path.join(outputdir, "logfile"), resultfile)],
>> -                            shell=True)
>> +                            shell=True, stderr=None)
> ditto
>
>>
>>          reason = get_failure_reason()
>>          if not reason:
>> @@ -677,8 +694,8 @@ def send_results(result, **kwargs):
>>
>>      # Yes, shutil.make_archive() would be nice, but it doesn't exist
>>      # in Python 2.6.
>> -    ret = subprocess.call(["tar", "cjf", "results.tar.bz2", "results"],
>> -                          cwd=outputdir, stdout=log, stderr=log)
>> +    ret = sysinfo.run_cmd_write_to(
>> +        log, ["tar", "cjf", "results.tar.bz2", "results"], cwd=outputdir)
>>      if ret != 0:
>>          log_write(log, "ERROR: could not make results tarball")
>>          sys.exit(1)
>> @@ -687,13 +704,14 @@ def send_results(result, **kwargs):
>>          # Submit results. Yes, Python has some HTTP libraries, but
>>          # none of the ones that are part of the standard library can
>>          # upload a file without writing dozens of lines of code.
>> -        ret = subprocess.call(["curl", "-u",
>> -                               "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
>> -                               "-H", "Expect:",
>> -                               "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
>> -                               "-F", "uploadsubmit=1",
>> -                               "http://autobuild.buildroot.org/submit/"],
>> -                              stdout=log, stderr=log)
>> +        ret = sysinfo.run_cmd_write_to(
>> +            log,
>> +            ["curl", "-u",
>> +             "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
>> +             "-H", "Expect:",
>> +             "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
>> +             "-F", "uploadsubmit=1",
>> +             "http://autobuild.buildroot.org/submit/"])
>>          if ret != 0:
>>              log_write(log, "INFO: results could not be submitted, %d" % ret)
>>          else:
>> --
>> 2.3.2
>>


-- 
Andr?

[Buildroot] [PATCH v2 10/10] autobuild-run: set locale to en_US or C

[André Erdmann]

Hi,

2015-04-12 10:31 GMT+02:00 Samuel Martin <s.martin49@gmail.com>:
> Hi Andr?, Thomas, all,
>
> On Wed, Mar 18, 2015 at 4:50 PM, Andr? Erdmann <dywi@mailerd.de> wrote:
>> some python scripts break if the locale is set to C, try en_US{.UTF-8,} first
> Could you give an example of such a script?
>

Apart from the small snippet I've included below the commit message:
no, I don't have any real-world script at hand that exposes this issue
and is sufficiently small. In general, it occurs when a script relies
on the default system encoding derived from the LANG/LC_ and has to
deal with non-ascii strings, e.g. when using open() and not
io.open()/codecs.open() to read a file.

>>
>> Additionally, drop all locale env vars (LC_*, LANG[GUAGE]) when
>> setting the new locale.
>>
>> Signed-off-by: Andr? Erdmann <dywi@mailerd.de>
>> ---
>>
>> Just for reference, a small python example that breaks if LANG is set to C:
>>
>>   $ LANG=C python -c "print(u'Andr\xe9')"
>>   UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 4: ordinal not in range(128)
>>
>> ---
>>  scripts/autobuild-run | 48 ++++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>> index e1a3398..a1e947d 100755
>> --- a/scripts/autobuild-run
>> +++ b/scripts/autobuild-run
>> @@ -168,6 +168,12 @@ class SystemInfo:
>>      DEFAULT_NEEDED_PROGS = ["make", "git", "gcc", "timeout"]
>>      DEFAULT_OPTIONAL_PROGS = ["bzr", "java", "javac", "jar"]
>>
>> +    # list of default locales (in lowercase, without "-", descending order)
>> +    #  some python scripts break if the locale is set to C, try en_US first
>> +    TRY_LOCALES = ['en_us.utf8', 'en_us', 'c']
>> +    # list of locale environment variables that should be (re-)set by set_locale()
>> +    LOCALE_KEYS = ['LANG']
>> +
>>      def __init__(self):
>>          self.needed_progs = list(self.__class__.DEFAULT_NEEDED_PROGS)
>>          self.optional_progs = list(self.__class__.DEFAULT_OPTIONAL_PROGS)
>> @@ -231,6 +237,41 @@ class SystemInfo:
>>
>>          return not missing_requirements
>>
>> +    def set_locale(self):
>> +        def is_locale_env_varname(w):
>> +            # w[:4] == 'LANG' and (not w[4:] or w[4:] == 'UAGE') ...
> Hmm... I must say the comment is rather confusing!
>

I'll drop it.

I usually take some "this could be optimized" notes while coding, this
one aims at reducing the str parts that need to be matched more than
once, because the word w matches "LANG" or "LANGUAGE" if the first 4
chars match "LANG" and the remainder is either empty or "UAGE".

>> +            return w[:3] == 'LC_' or w == 'LANG' or w == 'LANGUAGE'
> or: return w.startswith('LC_') or w in ['LANG', 'LANGUAGE']
>
>> +
>> +        ret, locales_str = self.run_cmd_get_stdout(["locale", "-a"])[
>> +        if ret != os.EX_OK:
>> +            return False
>> +
>> +        # create a dict
>> +        #   <locale identifier> (as listed in TRY_LOCALES) => <locale env name>
>> +        locales = dict((
>> +            (k.lower().replace("-", ""), k) for k in locales_str.split(None)
> s/None//
>
>> +        ))
>> +
>> +        for loc_key in filter(lambda x: x in locales, self.TRY_LOCALES):
> Is a for loop really needed here?
> You could just do:
>   try:[
>     loc_key = filter(lambda x: x in locales, self.TRY_LOCALES)[0]
>   except (TypeError, IndexError):
>     # set "C" as locale
>     loc_key = self.TRY_LOCALES[-1]
>

I've chosen a for-loop here because it works with both python 2 and
python 3 equally well:

* in python 2: filter() returns a list  =>  "filter(...)[0]" enclosed
by a "try/except IndexError" block
* in python 3: filter() returns a generator object  =>
"next(filter(...))" enclosed by a "try/except StopIteration" block

None of these variants are compatible with each other, because you
can't access generator items by index (filter(...)[0] raises a
TypeError in py3), and lists are not iterators (next(filter(...))
raises a TypeError in py2).

What would work for both py2/py3 is "next(iter(filter(...)))" enclosed
by a "try/except StopIteration" block, but the for-loop is more
readable.


>> +            # cannot modify self.env while iterating over it,
>> +            #  create intermediate list
>> +            env_old_locale_keys = [
>> +                k for k in self.env.keys() if is_locale_env_varname(k)
>> +            ]
>> +            for k in env_old_locale_keys:
>> +                del self.env[k]
> Setting the new value will automatically override the old value, so
> deleting it before looks a bit overkill...
>

Deleting/Inserting is asymmetrical:

* drop any env var matching LANG, LANGUAGE or LC_* (LC_MESSAGES et al)
* set LANG to the new locale

We could also simply set LC_ALL instead of deleting LC_*, but that
opens up the possibility to leak LC_ env vars when LC_ALL gets
unset/emptied:

$ env LC_MESSAGES=en_US.UTF-8 LC_ALL=de_DE.UTF-8  sh -c 'locale | grep
LC_MES; export LC_ALL=; locale | grep LC_MES;'

LC_MESSAGES="de_DE.UTF-8"
LC_MESSAGES=en_US.UTF-8

>> +
>> +            # set new locale once
>> +            for vname in self.LOCALE_KEYS:
>> +                self.env[vname] = locales[loc_key]
>> +            return True
>> +        # -- end for
>> +        # practically impossible to reach this return if 'c' is in TRY_LOCALES:
>> +        return None
>> +
>> +    def sanitize_env(self):
>> +        self.set_locale()
>> +
>>      def popen(self, cmdv, **kwargs):
>>          kwargs.setdefault('stdin', self.devnull)
>>          kwargs.setdefault('stdout', self.devnull)
>> @@ -789,12 +830,11 @@ def merge(dict_1, dict_2):
>>
>>  def main():
>>
>> -    # Avoid locale settings of autobuilder machine leaking in, for example
>> -    # showing error messages in another language.
>> -    os.environ['LC_ALL'] = 'C'
>> -
>>      check_version()
>>      sysinfo = SystemInfo()
>> +    # Avoid locale settings of autobuilder machine leaking in, for example
>> +    # showing error messages in another language.
>> +    sysinfo.sanitize_env()
>>
>>      args = docopt.docopt(doc, version=VERSION)
>>
>> --
>> 2.3.2
>>


-- 
Andr?