autobuild-run: python3 compat and misc improvements

PATCH New André Erdmann 3 participants 23 messages Feb 25, 2015 – Mar 15, 2015

lore

Share & tools

lore

André Erdmann

author

2015-02-25 21:17 UTC #0 lore raw

cover autobuild-run: python3 compat and misc improvements

Patches 1-4 make autobuild-run compatible with both python >=2.6 and 3.

Patches 5-9 are minor enhancements (mostly cleanup).

Patches 10- are more invasive:
* move the check_requirements() logic to the SystemInfo class
* control environment(->locale), std{in,out,err} of all called commands

Tested w/ python2.7 and python3.4, but not python2.6 (it should work, though).

This changeset should be mostly compatible with the
"autobuild-run improvements" patchset from Thomas De Schampheleire, except for:

* autobuild-run: extend TODO list

* implements "Integrate method check-requirements with the SysInfo class, distinghuishing between required and optional dependencies.", so this item can be removed from the TODO list

* autobuild-run: check-requirements does not need to know the login details

* dep on "curl" gets set in main()

* autobuild-run: set LC_ALL=C to not use locale settings of host machine

* SystemInfo maintains its own env dict and passes it to subprocess.call()
* ^ tries to find a "comparable" locale: either en_US[.UTF-8] or C (My experience is that some python (build) scripts fail if the locale is not *.utf-8.)
* remove all LANG, LC_* vars from the env if a "comparable" locale is found and set LANG=<locale>

Andr? Erdmann (13):
autobuild-run, python3: print is a function autobuild-run, python3: urllib.request<>urllib2 autobuild-run, python3: bytes<>str, decode() autobuild-run, python3: configparser<>ConfigParser
autobuild-run: remove unneeded vars
autobuild-run: explicitly close web file handles
autobuild-run: get host arch once
autobuild-run: sort imports alphabetically
autobuild-run: unify "which <prog>" code
autobuild-run: use a built-in has_prog() implementation
autobuild-run: move check_requirements() to SystemInfo
autobuild-run: encapsulate subprocess calls
autobuild-run: set locale to en_US or C

 scripts/autobuild-run | 321 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 218 insertions(+), 103 deletions(-)

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #1 lore raw

PATCH 1/13 autobuild-run, python3: print is a function

Commands used for editing:
2to ./scripts/autobuild-run -f print -w

Py2k compatibility: via __future__ import

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

scripts/autobuild-run +8 -6

index a75006f..2354893 100755
@@ -57,6 +57,8 @@
 #   BR2_PACKAGE_CLASSPATH=y, improve the script to detect whether the
 #   necessary host machine requirements are there to build classpath.
 
+from __future__ import print_function
+
 import urllib2
 import csv
 from random import randint

@@ -82,7 +84,7 @@ def check_version():
     r = urllib2.urlopen('http://autobuild.buildroot.org/version')
     version = int(r.readline().strip())
     if version > VERSION:
-        print "ERROR: script version too old, please upgrade."
+        print("ERROR: script version too old, please upgrade.")
         sys.exit(1)
 
 def check_requirements(http_login, http_password):

@@ -96,7 +98,7 @@ def check_requirements(http_login, http_password):
     for prog in needed_progs:
         ret = subprocess.call(["which", prog], stdout=devnull, stderr=devnull)
         if ret != 0:
-            print "ERROR: your system lacks the '%s' program" % prog
+            print("ERROR: your system lacks the '%s' program" % prog)
             missing_requirements = True
 
     if missing_requirements:

@@ -576,11 +578,11 @@ Format of the configuration file:
 
     if args.config:
         if not os.path.exists(args.config):
-            print "ERROR: configuration file %s does not exist" % args.config
+            print("ERROR: configuration file %s does not exist" % args.config)
             sys.exit(1)
         parser = ConfigParser.RawConfigParser()
         if not parser.read([args.config]):
-            print "ERROR: cannot parse configuration file %s" % args.config
+            print("ERROR: cannot parse configuration file %s" % args.config)
             sys.exit(1)
         if parser.has_option('main', 'ninstances'):
             ninstances = parser.getint('main', 'ninstances')

@@ -608,8 +610,8 @@ def main():
     (ninstances, njobs, http_login, http_password, submitter) = config_get()
     check_requirements(http_login, http_password)
     if http_login is None or http_password is None:
-        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"
+        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")
     def sigterm_handler(signum, frame):
         os.killpg(os.getpgid(os.getpid()), signal.SIGTERM)
         sys.exit(1)

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #2 lore raw

PATCH 2/13 autobuild-run, python3: urllib.request<>urllib2

In reply to André Erdmann

urllib2 has been split into several modules,
urlopen() is now part of urllib.request.

Py2k compatibility: import urllib2 if python version < 3

Signed-off-by Andr? Erdmann <[email protected]>

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

scripts/autobuild-run +10 -4

index 2354893..be2f482 100755
@@ -59,7 +59,6 @@
 
 from __future__ import print_function
 
-import urllib2
 import csv
 from random import randint
 import subprocess

@@ -73,6 +72,13 @@ import hashlib
 import argparse
 import ConfigParser
 
+if sys.hexversion >= 0x3000000:
+    import urllib.request as _urllib
+else:
+    import urllib2 as _urllib
+
+urlopen = _urllib.urlopen
+
 MAX_DURATION = 60 * 60 * 4
 VERSION = 1

@@ -81,7 +87,7 @@ def log_write(logf, msg):
     logf.flush()
 
 def check_version():
-    r = urllib2.urlopen('http://autobuild.buildroot.org/version')
+    r = urlopen('http://autobuild.buildroot.org/version')
     version = int(r.readline().strip())
     if version > VERSION:
         print("ERROR: script version too old, please upgrade.")

@@ -136,7 +142,7 @@ def get_toolchain_configs():
         - contents: an array of lines of the defconfig
     """
 
-    r = urllib2.urlopen('http://autobuild.buildroot.org/toolchains/configs/toolchain-configs.csv')
+    r = urlopen('http://autobuild.buildroot.org/toolchains/configs/toolchain-configs.csv')
     l = r.readlines()
     configs = []
     for row in csv.reader(l):

@@ -151,7 +157,7 @@ def get_toolchain_configs():
         if hostarch != config["hostarch"]:
             continue
         config["libc"] = row[2]
-        r = urllib2.urlopen(config["url"])
+        r = urlopen(config["url"])
         config["contents"] = r.readlines()
         configs.append(config)
     return configs

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #3 lore raw

PATCH 3/13 autobuild-run, python3: bytes<>str, decode()

In reply to André Erdmann

urlopen.read[lines]() returns bytes in py3 and text in py2. Decode the bytes if necessary.

Py2k compatibility: no-op decode_bytes(), decode_byte_list() functions

Note: The Py2k variant of the decode_byte_list() function returns the
input list, whereas the python3 variant creates a new list.

Note2: This commit does not add encode() functions, which could be necessary
when writing non-ascii chars to files -- in Python3 only.

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

scripts/autobuild-run +16 -3

index be2f482..3a3b8de 100755
@@ -79,6 +79,19 @@ else:
 
 urlopen = _urllib.urlopen
 
+if sys.hexversion >= 0x3000000:
+    def decode_bytes(b):
+        return b.decode()
+
+    def decode_byte_list(bl):
+        return [b.decode() for b in bl]
+else:
+    def _identity(e):
+        return e
+
+    decode_bytes = _identity
+    decode_byte_list = _identity
+
 MAX_DURATION = 60 * 60 * 4
 VERSION = 1

@@ -88,7 +101,7 @@ def log_write(logf, msg):
 
 def check_version():
     r = urlopen('http://autobuild.buildroot.org/version')
-    version = int(r.readline().strip())
+    version = int(decode_bytes(r.readline()).strip())
     if version > VERSION:
         print("ERROR: script version too old, please upgrade.")
         sys.exit(1)

@@ -143,7 +156,7 @@ def get_toolchain_configs():
     """
 
     r = urlopen('http://autobuild.buildroot.org/toolchains/configs/toolchain-configs.csv')
-    l = r.readlines()
+    l = decode_byte_list(r.readlines())
     configs = []
     for row in csv.reader(l):
         config = {}

@@ -158,7 +171,7 @@ def get_toolchain_configs():
             continue
         config["libc"] = row[2]
         r = urlopen(config["url"])
-        config["contents"] = r.readlines()
+        config["contents"] = decode_byte_list(r.readlines())
         configs.append(config)
     return configs

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #4 lore raw

PATCH 4/13 autobuild-run, python3: configparser<>ConfigParser

In reply to André Erdmann

The ConfigParser module has been renamed to configparser.

Py2k compatibility: import ConfigParser if version < 3

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

scripts/autobuild-run +3 -2

index 3a3b8de..236597d 100755
@@ -70,11 +70,12 @@ from time import localtime, strftime
 import sys
 import hashlib
 import argparse
-import ConfigParser
 
 if sys.hexversion >= 0x3000000:
+    import configparser
     import urllib.request as _urllib
 else:
+    import ConfigParser as configparser
     import urllib2 as _urllib
 
 urlopen = _urllib.urlopen

@@ -599,7 +600,7 @@ Format of the configuration file:
         if not os.path.exists(args.config):
             print("ERROR: configuration file %s does not exist" % args.config)
             sys.exit(1)
-        parser = ConfigParser.RawConfigParser()
+        parser = configparser.RawConfigParser()
         if not parser.read([args.config]):
             print("ERROR: cannot parse configuration file %s" % args.config)
             sys.exit(1)

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #5 lore raw

PATCH 5/13 autobuild-run: remove unneeded vars

In reply to André Erdmann

And use cwd=abssrcdir (instead of srcdir) in the "Update the Buildroot sources" git pull command as that seems the intended purpose of $abssrcdir.

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

scripts/autobuild-run +1 -4

index 236597d..5db21b1 100755
@@ -205,8 +205,6 @@ def prepare_build(instance, log):
         log_write(log, "INFO: removing %s from downloads" % f)
         os.remove(os.path.join(dldir, f))
 
-    devnull = open(os.devnull, "w")
-
     # Clone Buildroot. This only happens if the source directory
     # didn't exist already.
     srcdir = os.path.join(idir, "buildroot")

@@ -219,7 +217,7 @@ def prepare_build(instance, log):
 
     # Update the Buildroot sources.
     abssrcdir = os.path.abspath(srcdir)
-    ret = subprocess.call(["git", "pull"], cwd=srcdir, stdout=log, stderr=log)
+    ret = subprocess.call(["git", "pull"], cwd=abssrcdir, stdout=log, stderr=log)
     if ret != 0:
         log_write(log, "ERROR: could not pull Buildroot sources")
         return -1

@@ -361,7 +359,6 @@ def gen_config(instance, log, sysinfo):
     """
 
     idir = "instance-%d" % instance
-    dldir = os.path.join(idir, "dl")
     # We need the absolute path to use with O=, because the relative
     # path to the output directory here is not relative to the
     # Buildroot sources, but to the location of the autobuilder

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #6 lore raw

PATCH 6/13 autobuild-run: explicitly close web file handles

In reply to André Erdmann

Close urlopen() file handles as soon as possible rather than waiting until their refcount drops to zero.

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

scripts/autobuild-run +9 -6

index 5db21b1..fee68b7 100755
@@ -59,6 +59,7 @@
 
 from __future__ import print_function
 
+import contextlib
 import csv
 from random import randint
 import subprocess

@@ -79,6 +80,7 @@ else:
     import urllib2 as _urllib
 
 urlopen = _urllib.urlopen
+urlopen_closing = lambda uri: contextlib.closing(urlopen(uri))
 
 if sys.hexversion >= 0x3000000:
     def decode_bytes(b):

@@ -101,8 +103,8 @@ def log_write(logf, msg):
     logf.flush()
 
 def check_version():
-    r = urlopen('http://autobuild.buildroot.org/version')
-    version = int(decode_bytes(r.readline()).strip())
+    with urlopen_closing('http://autobuild.buildroot.org/version') as r:
+        version = int(decode_bytes(r.readline()).strip())
     if version > VERSION:
         print("ERROR: script version too old, please upgrade.")
         sys.exit(1)

@@ -155,9 +157,10 @@ def get_toolchain_configs():
         - hostarch: the host architecture for which the toolchain is built
         - contents: an array of lines of the defconfig
     """
+    tc_cfg_uri = 'http://autobuild.buildroot.org/toolchains/configs/toolchain-configs.csv'
 
-    r = urlopen('http://autobuild.buildroot.org/toolchains/configs/toolchain-configs.csv')
-    l = decode_byte_list(r.readlines())
+    with urlopen_closing(tc_cfg_uri) as r:
+        l = decode_byte_list(r.readlines())
     configs = []
     for row in csv.reader(l):
         config = {}

@@ -171,8 +174,8 @@ def get_toolchain_configs():
         if hostarch != config["hostarch"]:
             continue
         config["libc"] = row[2]
-        r = urlopen(config["url"])
-        config["contents"] = decode_byte_list(r.readlines())
+        with urlopen_closing(config["url"]) as r:
+            config["contents"] = decode_byte_list(r.readlines())
         configs.append(config)
     return configs

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #7 lore raw

PATCH 7/13 autobuild-run: get host arch once

In reply to André Erdmann

in get_toolchain_configs()

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

scripts/autobuild-run +5 -3

index fee68b7..e0971cb 100755
@@ -162,15 +162,17 @@ def get_toolchain_configs():
     with urlopen_closing(tc_cfg_uri) as r:
         l = decode_byte_list(r.readlines())
     configs = []
+
+    (_, _, _, _, hostarch) = os.uname()
+    if hostarch == 'i686' or hostarch == 'x86_64':
+        hostarch = 'x86'
+
     for row in csv.reader(l):
         config = {}
         config["url"] = row[0]
         config["hostarch"] = row[1]
         # Ignore toolchains that are not built for the appropriate
         # host architecture
-        (_, _, _, _, hostarch) = os.uname()
-        if hostarch == 'i686' or hostarch == 'x86_64':
-            hostarch = 'x86'
         if hostarch != config["hostarch"]:
             continue
         config["libc"] = row[2]

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #8 lore raw

PATCH 8/13 autobuild-run: sort imports alphabetically

In reply to André Erdmann

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

scripts/autobuild-run +6 -6

index e0971cb..52577e7 100755
@@ -59,18 +59,18 @@
 
 from __future__ import print_function
 
+import argparse
 import contextlib
 import csv
-from random import randint
-import subprocess
+import hashlib
 from multiprocessing import Process
-import signal
 import os
+from random import randint
 import shutil
-from time import localtime, strftime
+import signal
+import subprocess
 import sys
-import hashlib
-import argparse
+from time import localtime, strftime
 
 if sys.hexversion >= 0x3000000:
     import configparser

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #9 lore raw

PATCH 9/13 autobuild-run: unify "which <prog>" code

In reply to André Erdmann

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

scripts/autobuild-run +13 ~~-17~~

index 52577e7..9c3e36c 100755
@@ -109,8 +109,11 @@ def check_version():
         print("ERROR: script version too old, please upgrade.")
         sys.exit(1)
 
+def has_prog(name):
+    with open(os.devnull, "w") as devnull:
+        return subprocess.call(["which", name], stdout=devnull, stderr=devnull) == os.EX_OK
+
 def check_requirements(http_login, http_password):
-    devnull = open(os.devnull, "w")
     needed_progs = ["make", "git", "gcc", "timeout"]
     missing_requirements = False

@@ -118,8 +121,7 @@ def check_requirements(http_login, http_password):
         needed_progs.append("curl")
 
     for prog in needed_progs:
-        ret = subprocess.call(["which", prog], stdout=devnull, stderr=devnull)
-        if ret != 0:
+        if not has_prog(prog):
             print("ERROR: your system lacks the '%s' program" % prog)
             missing_requirements = True

@@ -129,24 +131,18 @@ def check_requirements(http_login, http_password):
 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 = \
-          subprocess.call(["which", "bzr"], stdout=devnull, stderr=devnull) == 0
+        self.has_bzr = has_prog("bzr")
 
-        self.has_java = False
-        if subprocess.call(["which", "java"], stdout=devnull, stderr=devnull) == 0:
-            if subprocess.call("java -version | grep gcj", shell=True,
-                               stdout=devnull, stderr=devnull) == 1:
-                self.has_java = True
+        self.has_java = has_prog("java") and _grep_gcj("java") == 1
 
-        self.has_javac = False
-        if subprocess.call(["which", "javac"], stdout=devnull, stderr=devnull) == 0:
-            if subprocess.call("javac -version | grep gcj", shell=True,
-                               stdout=devnull, stderr=devnull) == 1:
-                self.has_javac = True
+        self.has_javac = has_prog("javac") and _grep_gcj("javac") == 1
 
-        self.has_jar = \
-          subprocess.call(["which", "jar"], stdout=devnull, stderr=devnull) == 0
+        self.has_jar = has_prog("jar")
 
 def get_toolchain_configs():
     """Fetch and return the possible toolchain configurations

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #10 lore raw

PATCH 10/13 autobuild-run: use a built-in has_prog() implementation

In reply to André Erdmann

No need to call subprocess.call(["which",name]).

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

scripts/autobuild-run +14 -3

index 9c3e36c..29d1bbb 100755
@@ -109,9 +109,20 @@ def check_version():
         print("ERROR: script version too old, please upgrade.")
         sys.exit(1)
 
-def has_prog(name):
-    with open(os.devnull, "w") as devnull:
-        return subprocess.call(["which", name], stdout=devnull, stderr=devnull) == os.EX_OK
+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(http_login, http_password):
     needed_progs = ["make", "git", "gcc", "timeout"]

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #11 lore raw

PATCH 11/13 autobuild-run: move check_requirements() to SystemInfo

In reply to André Erdmann

* build up the list of (additional) essential programs in main() rather than passing the http credentials to check_requirements()

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

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

scripts/autobuild-run +72 ~~-48~~

index 29d1bbb..9c78cc5 100755
@@ -109,51 +109,69 @@ 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(http_login, http_password):
-    needed_progs = ["make", "git", "gcc", "timeout"]
-    missing_requirements = False
-
-    if http_login and http_password:
-        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)
+    DEFAULT_NEEDED_PROGS = ["make", "git", "gcc", "timeout"]
+    DEFAULT_OPTIONAL_PROGS = ["bzr", "java", "javac", "jar"]
 
-        self.has_bzr = has_prog("bzr")
-
-        self.has_java = has_prog("java") and _grep_gcj("java") == 1
-
-        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
+
+    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):
+        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
 
-        self.has_jar = has_prog("jar")
 
 def get_toolchain_configs():
     """Fetch and return the possible toolchain configurations

@@ -279,14 +297,14 @@ def fixup_config(instance, sysinfo):
     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 \

@@ -637,10 +655,16 @@ def main():
     check_version()
     sysinfo = SystemInfo()
     (ninstances, njobs, http_login, http_password, submitter) = config_get()
-    check_requirements(http_login, http_password)
-    if http_login is None or http_password is None:
+
+    if http_login and http_password:
+        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):
         os.killpg(os.getpgid(os.getpid()), signal.SIGTERM)
         sys.exit(1)

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #12 lore raw

PATCH 12/13 autobuild-run: encapsulate subprocess calls

In reply to André Erdmann

Preparation step for passing LANG to worker (sub-)processes,

* always set stdout and stderr to devnull if not specified otherwise
* also default stdin to devnull, makes the "yes"-pipe in "make oldconfig" commands redundant

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 87 +++++++++++++++++++++++++++------------------------
 1 file changed, 46 insertions(+), 41 deletions(-)

scripts/autobuild-run +46 ~~-41~~

index 9c78cc5..7980879 100755
@@ -117,6 +117,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)

@@ -148,10 +149,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

@@ -172,6 +171,16 @@ class SystemInfo:
 
         return not missing_requirements
 
+    def run_cmd(self, cmdv, **kwargs):
+        kwargs.setdefault('stdin', self.devnull)
+        kwargs.setdefault('stdout', self.devnull)
+        kwargs.setdefault('stderr', self.devnull)
+        return subprocess.call(cmdv, **kwargs)
+
+    def run_cmd_write_to(self, outstream, cmdv, **kwargs):
+        kwargs.update(stdout=outstream, stderr=outstream)
+        return self.run_cmd(cmdv, **kwargs)
+
 
 def get_toolchain_configs():
     """Fetch and return the possible toolchain configurations

@@ -206,7 +215,7 @@ def get_toolchain_configs():
         configs.append(config)
     return configs
 
-def prepare_build(instance, log):
+def prepare_build(instance, log, sysinfo):
     """Prepare for the next build of the specified instance
 
     This function prepares the build by making sure all the needed

@@ -239,15 +248,15 @@ def prepare_build(instance, log):
     # 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

@@ -256,7 +265,7 @@ def prepare_build(instance, log):
     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

@@ -422,10 +431,7 @@ def gen_config(instance, log, sysinfo):
     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

@@ -433,30 +439,27 @@ def gen_config(instance, log, sysinfo):
     # 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(instance, sysinfo):
             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
 
     return 0
 
-def do_build(instance, njobs, log):
+def do_build(instance, njobs, log, sysinfo):
     """Run the build itself"""
 
     idir = "instance-%d" % instance

@@ -469,8 +472,9 @@ def do_build(instance, njobs, log):
     srcdir = os.path.join(idir, "buildroot")
     f = open(os.path.join(outputdir, "logfile"), "w+")
     log_write(log, "INFO: build started")
-    ret = subprocess.call(["timeout", str(MAX_DURATION), "make", "O=%s" % outputdir, "-C", srcdir,
-                           "BR2_DL_DIR=%s" % dldir, "BR2_JLEVEL=%s" % njobs], stdout=f, stderr=f)
+    ret = sysinfo.run_cmd_write_to(
+        f, ["timeout", str(MAX_DURATION), "make", "O=%s" % outputdir,
+            "-C", srcdir, "BR2_DL_DIR=%s" % dldir, "BR2_JLEVEL=%s" % njobs])
     # 124 is a special error code that indicates we have reached the
     # timeout
     if ret == 124:

@@ -479,14 +483,14 @@ def do_build(instance, njobs, log):
     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
     log_write(log, "INFO: build successful")
     return 0
 
-def send_results(instance, http_login, http_password, submitter, log, result):
+def send_results(instance, http_login, http_password, submitter, log, result, sysinfo):
     """Prepare and store/send tarball with results
 
     This function prepares the tarball with the results, and either

@@ -512,12 +516,12 @@ def send_results(instance, http_login, http_password, submitter, log, result):
         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)
-    subprocess.call(["tail -500 %s > %s" % \
-                     (os.path.join(outputdir, "logfile"), os.path.join(resultdir, "build-end.log"))],
-                    shell=True)
+                    shell=True, cwd=srcdir, stderr=None)
+    sysinfo.run_cmd(["tail -500 %s > %s" % (os.path.join(outputdir, "logfile"),
+                     os.path.join(resultdir, "build-end.log"))],
+                    shell=True, stderr=None)
 
     resultf = open(os.path.join(resultdir, "status"), "w+")
     if result == 0:

@@ -533,8 +537,8 @@ def send_results(instance, http_login, http_password, submitter, log, result):
 
     # 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)

@@ -543,12 +547,13 @@ def send_results(instance, http_login, http_password, submitter, log, result):
         # 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" % (http_login, 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" % (http_login, 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:

@@ -579,7 +584,7 @@ def run_instance(instance, njobs, http_login, http_password, submitter, sysinfo)
     while True:
         check_version()
 
-        ret = prepare_build(instance, instance_log)
+        ret = prepare_build(instance, instance_log, sysinfo)
         if ret != 0:
             continue

@@ -587,8 +592,8 @@ def run_instance(instance, njobs, http_login, http_password, submitter, sysinfo)
         if ret != 0:
             continue
 
-        ret = do_build(instance, njobs, instance_log)
-        send_results(instance, http_login, http_password, submitter, instance_log, ret)
+        ret = do_build(instance, njobs, instance_log, sysinfo)
+        send_results(instance, http_login, http_password, submitter, instance_log, ret, sysinfo)
 
 def config_get():
     """Get configuration parameters, either from the command line or the config file"""

Signature · 1 line

2.3.0

André Erdmann

author

2015-02-25 21:17 UTC #13 lore raw

PATCH 13/13 autobuild-run: set locale to en_US or C

In reply to André Erdmann

some python scripts break if the locale is set to C, try en_US first

Signed-off-by Andr? Erdmann <[email protected]>

---
 scripts/autobuild-run | 59 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

scripts/autobuild-run +57 -2

index 7980879..b9440b6 100755
@@ -95,6 +95,20 @@ else:
     decode_bytes = _identity
     decode_byte_list = _identity
 
+if sys.hexversion >= 0x2700000:
+    _run_proc_check_output = subprocess.check_output
+else:
+    # python 2.7 lacks subprocess.check_output(), need to define it here.
+    # It _has_ to raise CalledProcessError if the exitcode is not 0
+    def _run_proc_check_output(args, **kwargs):
+        assert 'stdout' not in kwargs
+        proc = subprocess.Popen(args, stdout=subprocess.PIPE, **kwargs)
+        stdout_data, _ = proc.communicate()
+        ret = proc.poll()
+        if ret != os.EX_OK:
+            raise subprocess.CalledProcessError(ret, args)
+        return stdout_data
+
 MAX_DURATION = 60 * 60 * 4
 VERSION = 1

@@ -113,16 +127,23 @@ 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)
         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:

@@ -171,10 +192,43 @@ class SystemInfo:
 
         return not missing_requirements
 
+    def set_locale(self):
+        try:
+            locales_str = _run_proc_check_output(
+                ["locale", "-a"], env=self.env,
+                stdin=self.devnull, stderr=self.devnull)
+        except subprocess.CalledProcessError:
+            return False
+
+        locales = dict((
+            (k.lower().replace("-", ""), k)
+            for k in decode_bytes(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 (k == 'LANG' or k[:3] == 'LC_')
+            ]
+            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
+        return None
+
+    def sanitize_env(self):
+        self.set_locale()
+
     def run_cmd(self, cmdv, **kwargs):
         kwargs.setdefault('stdin', self.devnull)
         kwargs.setdefault('stdout', self.devnull)
         kwargs.setdefault('stderr', self.devnull)
+        kwargs['env'] = self.env
         return subprocess.call(cmdv, **kwargs)
 
     def run_cmd_write_to(self, outstream, cmdv, **kwargs):

@@ -659,6 +713,7 @@ Format of the configuration file:
 def main():
     check_version()
     sysinfo = SystemInfo()
+    sysinfo.sanitize_env()
     (ninstances, njobs, http_login, http_password, submitter) = config_get()
 
     if http_login and http_password:

Signature · 1 line

2.3.0

Thomas Petazzoni

2015-02-26 10:08 UTC #14 lore raw

cover autobuild-run: python3 compat and misc improvements

In reply to André Erdmann

Dear Andr? Erdmann,

Quoted 7 lines · Andr? Erdmann

Patches 1-4 make autobuild-run compatible with both python >=2.6 and 3.
Patches 5-9 are minor enhancements (mostly cleanup).
Patches 10- are more invasive:
* move the check_requirements() logic to the SystemInfo class
* control environment(->locale), std{in,out,err} of all called commands

Thanks a lot all those patches. Overall they look very useful!

I really need to spend some time to merge the patches proposed by Thomas DS, and then your patches. I would still like to give the priority to Thomas DS patches, since they have been in the queue for a very long time.

Again, thanks!

Thomas

Signature · 3 lines

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

Thomas De Schampheleire

2015-02-26 19:25 UTC #15 lore raw

cover autobuild-run: python3 compat and misc improvements

Quoted 11 lines · Thomas Petazzoni

Dear Andr? Erdmann,
Quoted 1 line · Andr? Erdmann
Patches 1-4 make autobuild-run compatible with both python >=2.6 and
3.
Quoted 6 lines
Patches 5-9 are minor enhancements (mostly cleanup).
Patches 10- are more invasive:
* move the check_requirements() logic to the SystemInfo class
* control environment(->locale), std{in,out,err} of all called

Quoted 9 lines

commands
Thanks a lot all those patches. Overall they look very useful!
I really need to spend some time to merge the patches proposed by Thomas DS, and then your patches. I would still like to give the priority to Thomas DS patches, since they have been in the queue for a very long time.

Andr?,

Could you give your personal feedback on the usage of docopt in my patches? Do
you think it is an improvement or rather an unnecessary dependency?

ThomasP has a different opinion than me, and there haven't been many other
opinions so far. Your input is this most definitely welcome, no matter which side you choose.

Thanks,
Thomas

André Erdmann

author

2015-03-01 00:09 UTC #16 lore raw

cover autobuild-run: python3 compat and misc improvements

2015/2/28 Thomas De Schampheleire <[email protected]>:

Quoted 22 lines · Thomas Petazzoni

Quoted 11 lines
Dear Andr? Erdmann,
Quoted 1 line · Andr? Erdmann
Patches 1-4 make autobuild-run compatible with both python >=2.6 and
3.
Quoted 6 lines
Patches 5-9 are minor enhancements (mostly cleanup).
Patches 10- are more invasive:
* move the check_requirements() logic to the SystemInfo class
* control environment(->locale), std{in,out,err} of all called
Quoted 9 lines
commands
Thanks a lot all those patches. Overall they look very useful!
I really need to spend some time to merge the patches proposed by Thomas DS, and then your patches. I would still like to give the priority to Thomas DS patches, since they have been in the queue for a very long time.

Quoted 9 lines

Andr?,
Could you give your personal feedback on the usage of docopt in my patches? Do
you think it is an improvement or rather an unnecessary dependency?
ThomasP has a different opinion than me, and there haven't been many other
opinions so far. Your input is this most definitely welcome, no matter which side you choose.

The advantage of docopt is that it's really simple and you always have the usage message in the script, but I'd prefer argparse primarily because it's a built-in module (in py >= 2.7). It also features arg validation (via type=<function|type>). There's much more like subparsers, call a <function> each time an <arg> is encountered and whatnot, but that's not of interest here.

The major improvement of the docopt approach is reading the config (argv/file) into a few dicts that can be easily merged rather than doing a 3-way merge manually (defaults as locals X ini file parser X argparser namedtuple-like object). That's a good idea. It's doable with argparse as well, "vars(parser.parse_args())" returns the parsed args as dict.

So, the ((untested)) prototype of a "dict-improved" config_get() argparse variant would be:

def config_get():
def nonempty(val):
if val:
return val
raise argparse.ArgumentTypeError("must be a non-empty value.")
# --- end of nonempty (...) ---

def positive_int(val):
ival = int(val)
if ival > 0:
return ival
raise argparse.ArgumentTypeError("must be > 0 : %s" % val)
# --- end of positive_int (...) ---

   ## this is the dict that config_get() will return later on
   config = {
      "ninstances": 1, "njobs": 1, "http_login": None, "http_password": None, submitter: "N/A"
   }

## build up the arg parser
parser = argparse.ArgumentParser(...)

## note the use of SUPPRESS:
## the option won't make it into the (namespace object =>) dict if it is not specified on the cmdline parser.add_argument("--ninstances", "-n", default=argparse.SUPPRESS, type=positive_int, help=...) ...more args...
parser.add_argument("--submitter", "-s", default=argparse.SUPPRESS, type=nonempty, ...) parser.add_argument("--config", "-c", default=None, ...)

## parse args, store key/values in a dict
argv_config = vars(parser.parse_args())

## load config file, if any
if argv_config.get("config"):
## load_ini_config() must return a "usable" dict
## (e.g. convert "a-b" keys to "a_b", drop empty values, use parser.getint() where appropriate) config_from_file = load_ini_config(argv_config["config"])

## transfer config from file to the result dict ## (overrides default entries) config.update(config_from_file)
# -- end if

## drop the "config" key from argv config, no longer needed argv_config.pop("config",None)

## transfer argv config to the result dict
## (overrides entries from defaults, config_from_file) config.update(argv_config)

return config
# --- end of config_get (...) ---

config_get() would then return a dict that can be passed around (multiprocessing.Process(target=run_instance, args=(i, config, sysinfo)), do_build(instance, log, config), send_results(instance, log, result, config) a.s.o.)

Signature · 1 line

Andr?

Thomas De Schampheleire

2015-03-01 21:17 UTC #17 lore raw

cover autobuild-run: python3 compat and misc improvements

Hi Andr?,

Quoted 99 lines · Andr? Erdmann

2015/2/28 Thomas De Schampheleire <[email protected]>:
Quoted 22 lines · Thomas Petazzoni
Quoted 11 lines
Dear Andr? Erdmann,
Quoted 1 line · Andr? Erdmann
Patches 1-4 make autobuild-run compatible with both python >=2.6 and
3.
Quoted 6 lines
Patches 5-9 are minor enhancements (mostly cleanup).
Patches 10- are more invasive:
* move the check_requirements() logic to the SystemInfo class
* control environment(->locale), std{in,out,err} of all called
Quoted 9 lines
commands
Thanks a lot all those patches. Overall they look very useful!
I really need to spend some time to merge the patches proposed by Thomas DS, and then your patches. I would still like to give the priority to Thomas DS patches, since they have been in the queue for a very long time.
Quoted 9 lines
Andr?,
Could you give your personal feedback on the usage of docopt in my patches? Do
you think it is an improvement or rather an unnecessary dependency?
ThomasP has a different opinion than me, and there haven't been many other
opinions so far. Your input is this most definitely welcome, no matter which side you choose.
The advantage of docopt is that it's really simple and you always have the usage message in the script, but I'd prefer argparse primarily because it's a built-in module (in py >= 2.7). It also features arg validation (via type=<function|type>). There's much more like subparsers, call a <function> each time an <arg> is encountered and whatnot, but that's not of interest here.
The major improvement of the docopt approach is reading the config (argv/file) into a few dicts that can be easily merged rather than doing a 3-way merge manually (defaults as locals X ini file parser X argparser namedtuple-like object). That's a good idea. It's doable with argparse as well, "vars(parser.parse_args())" returns the parsed args as dict.
So, the ((untested)) prototype of a "dict-improved" config_get() argparse variant would be:
def config_get():
def nonempty(val):
if val:
return val
raise argparse.ArgumentTypeError("must be a non-empty value.")
# --- end of nonempty (...) ---
def positive_int(val):
ival = int(val)
if ival > 0:
return ival
raise argparse.ArgumentTypeError("must be > 0 : %s" % val)
# --- end of positive_int (...) ---
   ## this is the dict that config_get() will return later on
   config = {
      "ninstances": 1, "njobs": 1, "http_login": None, "http_password": None, submitter: "N/A"
   }
## build up the arg parser
parser = argparse.ArgumentParser(...)
## note the use of SUPPRESS:
## the option won't make it into the (namespace object =>) dict if it is not specified on the cmdline parser.add_argument("--ninstances", "-n", default=argparse.SUPPRESS, type=positive_int, help=...) ...more args...
parser.add_argument("--submitter", "-s", default=argparse.SUPPRESS, type=nonempty, ...) parser.add_argument("--config", "-c", default=None, ...)
## parse args, store key/values in a dict
argv_config = vars(parser.parse_args())
## load config file, if any
if argv_config.get("config"):
## load_ini_config() must return a "usable" dict
## (e.g. convert "a-b" keys to "a_b", drop empty values, use parser.getint() where appropriate) config_from_file = load_ini_config(argv_config["config"])
## transfer config from file to the result dict ## (overrides default entries) config.update(config_from_file)
# -- end if
## drop the "config" key from argv config, no longer needed argv_config.pop("config",None)
## transfer argv config to the result dict
## (overrides entries from defaults, config_from_file) config.update(argv_config)

Quoted 8 lines

return config
# --- end of config_get (...) ---
config_get() would then return a dict that can be passed around (multiprocessing.Process(target=run_instance, args=(i, config, sysinfo)), do_build(instance, log, config), send_results(instance, log, result, config) a.s.o.)

Thanks a lot for your feedback.

Maybe it's because I haven't seen the full solution yet, but this handling seems much more involved than the docopt approach. In particular it's odd that there would need to be explicit handling of things like nonempty or positive_int, which isn't necessary with standard argparse nor docopt.
I'm biased, of course, but this does not look more desirable to me than having an extra docopt dependency.

Best regards,
Thomas

André Erdmann

author

2015-03-01 21:37 UTC #18 lore raw

cover autobuild-run: python3 compat and misc improvements

2015-03-01 22:17 GMT+01:00 Thomas De Schampheleire <[email protected]>:

Quoted 55 lines

Hi Andr?,
Quoted 46 lines · Andr? Erdmann
2015/2/28 Thomas De Schampheleire <[email protected]>:
Quoted 22 lines · Thomas Petazzoni
Quoted 11 lines
Dear Andr? Erdmann,
Quoted 1 line · Andr? Erdmann
Patches 1-4 make autobuild-run compatible with both python >=2.6 and
3.
Quoted 6 lines
Patches 5-9 are minor enhancements (mostly cleanup).
Patches 10- are more invasive:
* move the check_requirements() logic to the SystemInfo class
* control environment(->locale), std{in,out,err} of all called
Quoted 9 lines
commands
Thanks a lot all those patches. Overall they look very useful!
I really need to spend some time to merge the patches proposed by Thomas DS, and then your patches. I would still like to give the priority to Thomas DS patches, since they have been in the queue for a very long time.
Quoted 9 lines
Andr?,
Could you give your personal feedback on the usage of docopt in my patches? Do
you think it is an improvement or rather an unnecessary dependency?
ThomasP has a different opinion than me, and there haven't been many other
opinions so far. Your input is this most definitely welcome, no matter which side you choose.
The advantage of docopt is that it's really simple and you always have the usage message in the script, but I'd prefer argparse primarily because it's a built-in module (in py >= 2.7). It also features arg validation (via type=<function|type>). There's much more like subparsers, call a <function> each time an <arg> is encountered and whatnot, but that's not of interest here.
The major improvement of the docopt approach is reading the config (argv/file) into a few dicts that can be easily merged rather than doing a 3-way merge manually (defaults as locals X ini file parser X argparser namedtuple-like object). That's a good idea. It's doable with argparse as well, "vars(parser.parse_args())" returns the parsed args as dict.
Quoted 5 lines
So, the ((untested)) prototype of a "dict-improved" config_get() argparse variant would be:
[...]

Quoted 7 lines

Thanks a lot for your feedback.
Maybe it's because I haven't seen the full solution yet, but this handling seems much more involved than the docopt approach. In particular it's odd that there would need to be explicit handling of things like nonempty or positive_int, which isn't necessary with standard argparse nor docopt.

They're purely optional - I've just put them in there to demonstrate how the input can be restricted so that "--ninstances hello" would get rejected by the parser (which docopt would not). You could also set "type=int" instead of "type=positive_int", which is shorter code-wise, but slighlty less correct.

Quoted 5 lines

I'm biased, of course, but this does not look more desirable to me than having an extra docopt dependency.
Best regards,
Thomas

Signature · 1 line

Andr?

Thomas De Schampheleire

2015-03-01 21:52 UTC #19 lore raw

cover autobuild-run: python3 compat and misc improvements

On March 1, 2015 10:37:09 PM CET, "Andr? Erdmann" <[email protected]> wrote:
...

Quoted 36 lines

Quoted 2 lines
Quoted 2 lines
The advantage of docopt is that it's really simple and you always
have the
Quoted 1 line
Quoted 1 line
usage message in the script, but I'd prefer argparse primarily
because it's a built-in
Quoted 1 line
Quoted 1 line
module (in py >= 2.7). It also features arg validation (via
type=<function|type>).
Quoted 1 line
Quoted 1 line
There's much more like subparsers, call a <function> each time an
<arg> is encountered
Quoted 3 lines
Quoted 3 lines
and whatnot, but that's not of interest here.
The major improvement of the docopt approach is reading the config
(argv/file)
Quoted 1 line
Quoted 1 line
into a few dicts that can be easily merged rather than doing a 3-way
merge manually
Quoted 1 line
Quoted 1 line
(defaults as locals X ini file parser X argparser namedtuple-like
object). That's a
Quoted 1 line
Quoted 1 line
good idea. It's doable with argparse as well,
"vars(parser.parse_args())" returns
Quoted 3 lines
Quoted 3 lines
the parsed args as dict.
So, the ((untested)) prototype of a "dict-improved" config_get()
argparse variant would be:
Quoted 5 lines
Quoted 4 lines
[...]
Quoted 7 lines
Thanks a lot for your feedback.
Maybe it's because I haven't seen the full solution yet, but this handling seems much more involved than the docopt approach. In particular it's odd that there would need to be explicit handling of things like nonempty or positive_int, which isn't necessary with standard argparse nor docopt.

Quoted 5 lines

They're purely optional - I've just put them in there to demonstrate how the input can be restricted so that "--ninstances hello" would get rejected by the parser (which docopt would not). You could also set "type=int" instead of "type=positive_int", which is shorter code-wise, but slighlty less correct.

Thanks for clarifying, it makes more sense to me now. Keeping argparse but using the
dict-merging isn't too bad, it would address my main concern with the original code (the not-so-beautiful handling of the arguments).

So Thomas P: it's your call now: I'm just as ok with going for the approach proposed
by Andr?.

Best regards,
Thomas

Thomas Petazzoni

2015-03-02 08:36 UTC #20 lore raw

cover autobuild-run: python3 compat and misc improvements

Dear Thomas De Schampheleire,

Quoted 6 lines · Thomas De Schampheleire

Thanks for clarifying, it makes more sense to me now. Keeping argparse but using the
dict-merging isn't too bad, it would address my main concern with the original code (the not-so-beautiful handling of the arguments).
So Thomas P: it's your call now: I'm just as ok with going for the approach proposed
by Andr?.

I really don't know to be honest. Originally, I was really in favor of keeping a solution that doesn't require external modules, but you pushed docopt, and I ended up being OK with it. So if now you tell me there is a reasonable solution without docopt, why not.

Who would do the rework? Andr?, are you willing to do it?

The thing is that there is still a number of very useful improvements in the branch provided by Thomas DS. So in fact, I believe I might be applying Thomas DS patches first, and then if someone wants to revert back to argparse for argument parsing, a follow-up series should be sent.

Best regards,

Thomas

Signature · 3 lines

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

André Erdmann

author

2015-03-02 20:00 UTC #21 lore raw

cover autobuild-run: python3 compat and misc improvements

2015-03-02 9:36 GMT+01:00 Thomas Petazzoni
<[email protected]>:

Quoted 11 lines

Dear Thomas De Schampheleire,
Quoted 6 lines · Thomas De Schampheleire
Thanks for clarifying, it makes more sense to me now. Keeping argparse but using the
dict-merging isn't too bad, it would address my main concern with the original code (the not-so-beautiful handling of the arguments).
So Thomas P: it's your call now: I'm just as ok with going for the approach proposed
by Andr?.

Quoted 7 lines

I really don't know to be honest. Originally, I was really in favor of keeping a solution that doesn't require external modules, but you pushed docopt, and I ended up being OK with it. So if now you tell me there is a reasonable solution without docopt, why not.
Who would do the rework? Andr?, are you willing to do it?

Sure - should I base it on Thomas DS' patchset (once merged) or his+this series?

Quoted 6 lines

The thing is that there is still a number of very useful improvements in the branch provided by Thomas DS. So in fact, I believe I might be applying Thomas DS patches first, and then if someone wants to revert back to argparse for argument parsing, a follow-up series should be sent.

Signature · 1 line

Andr?

Thomas Petazzoni

2015-03-15 13:36 UTC #22 lore raw

cover autobuild-run: python3 compat and misc improvements

In reply to André Erdmann

Dear Andr? Erdmann,

Quoted 11 lines · Andr? Erdmann

Andr? Erdmann (13):
autobuild-run, python3: print is a function autobuild-run, python3: urllib.request<>urllib2 autobuild-run, python3: bytes<>str, decode() autobuild-run, python3: configparser<>ConfigParser
autobuild-run: remove unneeded vars
autobuild-run: explicitly close web file handles
autobuild-run: get host arch once
autobuild-run: sort imports alphabetically
autobuild-run: unify "which <prog>" code
autobuild-run: use a built-in has_prog() implementation

I've applied these patches, after making some minor adjustments needed to make them apply on top of Thomas DS patch series.

Quoted 3 lines

autobuild-run: move check_requirements() to SystemInfo
autobuild-run: encapsulate subprocess calls
autobuild-run: set locale to en_US or C

Those ones were conflicting a bit too much. Could you rework them on top of the latest buildroot-test repository, and resend an updated version?

Thanks a lot!

Thomas

Signature · 3 lines

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