[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

This series against the buildroot-test repo makes some improvements to the
autobuild-run script.

v5:
- add two patches to handle kill/interrupt properly
- make the kwargs dictionary passing to Process more readable
- rebase

v4:
- rename do_send_results into upload (Peter)

v3:
- introduce missing usage of do_send_results (and move patch backwards)
- update TODO list

v2:
- reorder patches
- take into account comments on boolean logic in first patch
- place docopt directly in scripts/ instead of in a subdirectory
- remove patch adding --git (Yann, Thomas)
- some bug fixes in the first version of the patches

As you can see, this iteration still has the docopt patches. Thomas Petazzoni
has expressed his reservations against the introduction of docopt, because the
benefits wouldn't weigh out against the addition of an external Python module.
Thomas told me on IRC he would have a deeper look onto this before making a
final decision.

In case the final decision is not to introduce docopt, following improvements
could be done to the current code:
  - save the arguments from the command-line into a dictionary instead of plain
    variables
  - keep the added ini_config() method to also save the options in a dictionary,
    and use the added merge() method to merge both arguments and config options
    into one final configuration, without needing to handle it manually.


Thomas De Schampheleire (11):
  scripts: add python module docopt
  autobuild-run: use docopt for argument parsing
  autobuild-run: add option --make-opts for custom Buildroot options
  autobuild-run: use **kwargs to avoid explicit parameter passthroughs
  autobuild-run: check-requirements does not need to know the login
    details
  autobuild-run: set LC_ALL=C to not use locale settings of host machine
  autobuild-run: improve the logic to generate build-end.log
  autobuild-run: save config.log files for failed package
  autobuild-run: extend TODO list
  autobuild-run: kill all children on SIGTERM
  autobuild-run: catch KeyboardInterrupt in the same way as SIGTERM

 scripts/autobuild-run      | 350 +++++++++++++++++++--------
 scripts/docopt.LICENSE-MIT |  23 ++
 scripts/docopt.py          | 581 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 851 insertions(+), 103 deletions(-)
 create mode 100644 scripts/docopt.LICENSE-MIT
 create mode 100644 scripts/docopt.py

-- 
1.8.5.1

[Buildroot] [PATCH v5 01/11] scripts: add python module docopt

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

This commit adds the 'docopt' python module, version 0.6.1.
Original source is: https://github.com/docopt/docopt
Since docopt stands on its own, only the .py file and the license is
added, other files from github are skipped.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/docopt.LICENSE-MIT |  23 ++
 scripts/docopt.py          | 581 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 604 insertions(+)
 create mode 100644 scripts/docopt.LICENSE-MIT
 create mode 100644 scripts/docopt.py

diff --git a/scripts/docopt.LICENSE-MIT b/scripts/docopt.LICENSE-MIT
new file mode 100644
index 0000000..58ff1bc
--- /dev/null
+++ b/scripts/docopt.LICENSE-MIT
@@ -0,0 +1,23 @@
+Copyright (c) 2012 Vladimir Keleshev, <vladimir@keleshev.com>
+
+Permission is hereby granted, free of charge, to any person
+obtaining a copy of this software and associated
+documentation files (the "Software"), to deal in the Software
+without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense,
+and/or sell copies of the Software, and to permit persons to
+whom the Software is furnished to do so, subject to the
+following conditions:
+
+The above copyright notice and this permission notice shall
+be included in all copies or substantial portions of the
+Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
+KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
diff --git a/scripts/docopt.py b/scripts/docopt.py
new file mode 100644
index 0000000..2e43f7c
--- /dev/null
+++ b/scripts/docopt.py
@@ -0,0 +1,581 @@
+"""Pythonic command-line interface parser that will make you smile.
+
+ * http://docopt.org
+ * Repository and issue-tracker: https://github.com/docopt/docopt
+ * Licensed under terms of MIT license (see LICENSE-MIT)
+ * Copyright (c) 2013 Vladimir Keleshev, vladimir at keleshev.com
+
+"""
+import sys
+import re
+
+
+__all__ = ['docopt']
+__version__ = '0.6.1'
+
+
+class DocoptLanguageError(Exception):
+
+    """Error in construction of usage-message by developer."""
+
+
+class DocoptExit(SystemExit):
+
+    """Exit in case user invoked program with incorrect arguments."""
+
+    usage = ''
+
+    def __init__(self, message=''):
+        SystemExit.__init__(self, (message + '\n' + self.usage).strip())
+
+
+class Pattern(object):
+
+    def __eq__(self, other):
+        return repr(self) == repr(other)
+
+    def __hash__(self):
+        return hash(repr(self))
+
+    def fix(self):
+        self.fix_identities()
+        self.fix_repeating_arguments()
+        return self
+
+    def fix_identities(self, uniq=None):
+        """Make pattern-tree tips point to same object if they are equal."""
+        if not hasattr(self, 'children'):
+            return self
+        uniq = list(set(self.flat())) if uniq is None else uniq
+        for i, child in enumerate(self.children):
+            if not hasattr(child, 'children'):
+                assert child in uniq
+                self.children[i] = uniq[uniq.index(child)]
+            else:
+                child.fix_identities(uniq)
+
+    def fix_repeating_arguments(self):
+        """Fix elements that should accumulate/increment values."""
+        either = [list(child.children) for child in transform(self).children]
+        for case in either:
+            for e in [child for child in case if case.count(child) > 1]:
+                if type(e) is Argument or type(e) is Option and e.argcount:
+                    if e.value is None:
+                        e.value = []
+                    elif type(e.value) is not list:
+                        e.value = e.value.split()
+                if type(e) is Command or type(e) is Option and e.argcount == 0:
+                    e.value = 0
+        return self
+
+
+def transform(pattern):
+    """Expand pattern into an (almost) equivalent one, but with single Either.
+
+    Example: ((-a | -b) (-c | -d)) => (-a -c | -a -d | -b -c | -b -d)
+    Quirks: [-a] => (-a), (-a...) => (-a -a)
+
+    """
+    result = []
+    groups = [[pattern]]
+    while groups:
+        children = groups.pop(0)
+        parents = [Required, Optional, OptionsShortcut, Either, OneOrMore]
+        if any(t in map(type, children) for t in parents):
+            child = [c for c in children if type(c) in parents][0]
+            children.remove(child)
+            if type(child) is Either:
+                for c in child.children:
+                    groups.append([c] + children)
+            elif type(child) is OneOrMore:
+                groups.append(child.children * 2 + children)
+            else:
+                groups.append(child.children + children)
+        else:
+            result.append(children)
+    return Either(*[Required(*e) for e in result])
+
+
+class LeafPattern(Pattern):
+
+    """Leaf/terminal node of a pattern tree."""
+
+    def __init__(self, name, value=None):
+        self.name, self.value = name, value
+
+    def __repr__(self):
+        return '%s(%r, %r)' % (self.__class__.__name__, self.name, self.value)
+
+    def flat(self, *types):
+        return [self] if not types or type(self) in types else []
+
+    def match(self, left, collected=None):
+        collected = [] if collected is None else collected
+        pos, match = self.single_match(left)
+        if match is None:
+            return False, left, collected
+        left_ = left[:pos] + left[pos + 1:]
+        same_name = [a for a in collected if a.name == self.name]
+        if type(self.value) in (int, list):
+            if type(self.value) is int:
+                increment = 1
+            else:
+                increment = ([match.value] if type(match.value) is str
+                             else match.value)
+            if not same_name:
+                match.value = increment
+                return True, left_, collected + [match]
+            same_name[0].value += increment
+            return True, left_, collected
+        return True, left_, collected + [match]
+
+
+class BranchPattern(Pattern):
+
+    """Branch/inner node of a pattern tree."""
+
+    def __init__(self, *children):
+        self.children = list(children)
+
+    def __repr__(self):
+        return '%s(%s)' % (self.__class__.__name__,
+                           ', '.join(repr(a) for a in self.children))
+
+    def flat(self, *types):
+        if type(self) in types:
+            return [self]
+        return sum([child.flat(*types) for child in self.children], [])
+
+
+class Argument(LeafPattern):
+
+    def single_match(self, left):
+        for n, pattern in enumerate(left):
+            if type(pattern) is Argument:
+                return n, Argument(self.name, pattern.value)
+        return None, None
+
+    @classmethod
+    def parse(class_, source):
+        name = re.findall('(<\S*?>)', source)[0]
+        value = re.findall('\[default: (.*)\]', source, flags=re.I)
+        return class_(name, value[0] if value else None)
+
+
+class Command(Argument):
+
+    def __init__(self, name, value=False):
+        self.name, self.value = name, value
+
+    def single_match(self, left):
+        for n, pattern in enumerate(left):
+            if type(pattern) is Argument:
+                if pattern.value == self.name:
+                    return n, Command(self.name, True)
+                else:
+                    break
+        return None, None
+
+
+class Option(LeafPattern):
+
+    def __init__(self, short=None, long=None, argcount=0, value=False):
+        assert argcount in (0, 1)
+        self.short, self.long, self.argcount = short, long, argcount
+        self.value = None if value is False and argcount else value
+
+    @classmethod
+    def parse(class_, option_description):
+        short, long, argcount, value = None, None, 0, False
+        options, _, description = option_description.strip().partition('  ')
+        options = options.replace(',', ' ').replace('=', ' ')
+        for s in options.split():
+            if s.startswith('--'):
+                long = s
+            elif s.startswith('-'):
+                short = s
+            else:
+                argcount = 1
+        if argcount:
+            matched = re.findall('\[default: (.*)\]', description, flags=re.I)
+            value = matched[0] if matched else None
+        return class_(short, long, argcount, value)
+
+    def single_match(self, left):
+        for n, pattern in enumerate(left):
+            if self.name == pattern.name:
+                return n, pattern
+        return None, None
+
+    @property
+    def name(self):
+        return self.long or self.short
+
+    def __repr__(self):
+        return 'Option(%r, %r, %r, %r)' % (self.short, self.long,
+                                           self.argcount, self.value)
+
+
+class Required(BranchPattern):
+
+    def match(self, left, collected=None):
+        collected = [] if collected is None else collected
+        l = left
+        c = collected
+        for pattern in self.children:
+            matched, l, c = pattern.match(l, c)
+            if not matched:
+                return False, left, collected
+        return True, l, c
+
+
+class Optional(BranchPattern):
+
+    def match(self, left, collected=None):
+        collected = [] if collected is None else collected
+        for pattern in self.children:
+            m, left, collected = pattern.match(left, collected)
+        return True, left, collected
+
+
+class OptionsShortcut(Optional):
+
+    """Marker/placeholder for [options] shortcut."""
+
+
+class OneOrMore(BranchPattern):
+
+    def match(self, left, collected=None):
+        assert len(self.children) == 1
+        collected = [] if collected is None else collected
+        l = left
+        c = collected
+        l_ = None
+        matched = True
+        times = 0
+        while matched:
+            # could it be that something didn't match but changed l or c?
+            matched, l, c = self.children[0].match(l, c)
+            times += 1 if matched else 0
+            if l_ == l:
+                break
+            l_ = l
+        if times >= 1:
+            return True, l, c
+        return False, left, collected
+
+
+class Either(BranchPattern):
+
+    def match(self, left, collected=None):
+        collected = [] if collected is None else collected
+        outcomes = []
+        for pattern in self.children:
+            matched, _, _ = outcome = pattern.match(left, collected)
+            if matched:
+                outcomes.append(outcome)
+        if outcomes:
+            return min(outcomes, key=lambda outcome: len(outcome[1]))
+        return False, left, collected
+
+
+class Tokens(list):
+
+    def __init__(self, source, error=DocoptExit):
+        self += source.split() if hasattr(source, 'split') else source
+        self.error = error
+
+    @staticmethod
+    def from_pattern(source):
+        source = re.sub(r'([\[\]\(\)\|]|\.\.\.)', r' \1 ', source)
+        source = [s for s in re.split('\s+|(\S*<.*?>)', source) if s]
+        return Tokens(source, error=DocoptLanguageError)
+
+    def move(self):
+        return self.pop(0) if len(self) else None
+
+    def current(self):
+        return self[0] if len(self) else None
+
+
+def parse_long(tokens, options):
+    """long ::= '--' chars [ ( ' ' | '=' ) chars ] ;"""
+    long, eq, value = tokens.move().partition('=')
+    assert long.startswith('--')
+    value = None if eq == value == '' else value
+    similar = [o for o in options if o.long == long]
+    if tokens.error is DocoptExit and similar == []:  # if no exact match
+        similar = [o for o in options if o.long and o.long.startswith(long)]
+    if len(similar) > 1:  # might be simply specified ambiguously 2+ times?
+        raise tokens.error('%s is not a unique prefix: %s?' %
+                           (long, ', '.join(o.long for o in similar)))
+    elif len(similar) < 1:
+        argcount = 1 if eq == '=' else 0
+        o = Option(None, long, argcount)
+        options.append(o)
+        if tokens.error is DocoptExit:
+            o = Option(None, long, argcount, value if argcount else True)
+    else:
+        o = Option(similar[0].short, similar[0].long,
+                   similar[0].argcount, similar[0].value)
+        if o.argcount == 0:
+            if value is not None:
+                raise tokens.error('%s must not have an argument' % o.long)
+        else:
+            if value is None:
+                if tokens.current() in [None, '--']:
+                    raise tokens.error('%s requires argument' % o.long)
+                value = tokens.move()
+        if tokens.error is DocoptExit:
+            o.value = value if value is not None else True
+    return [o]
+
+
+def parse_shorts(tokens, options):
+    """shorts ::= '-' ( chars )* [ [ ' ' ] chars ] ;"""
+    token = tokens.move()
+    assert token.startswith('-') and not token.startswith('--')
+    left = token.lstrip('-')
+    parsed = []
+    while left != '':
+        short, left = '-' + left[0], left[1:]
+        similar = [o for o in options if o.short == short]
+        if len(similar) > 1:
+            raise tokens.error('%s is specified ambiguously %d times' %
+                               (short, len(similar)))
+        elif len(similar) < 1:
+            o = Option(short, None, 0)
+            options.append(o)
+            if tokens.error is DocoptExit:
+                o = Option(short, None, 0, True)
+        else:  # why copying is necessary here?
+            o = Option(short, similar[0].long,
+                       similar[0].argcount, similar[0].value)
+            value = None
+            if o.argcount != 0:
+                if left == '':
+                    if tokens.current() in [None, '--']:
+                        raise tokens.error('%s requires argument' % short)
+                    value = tokens.move()
+                else:
+                    value = left
+                    left = ''
+            if tokens.error is DocoptExit:
+                o.value = value if value is not None else True
+        parsed.append(o)
+    return parsed
+
+
+def parse_pattern(source, options):
+    tokens = Tokens.from_pattern(source)
+    result = parse_expr(tokens, options)
+    if tokens.current() is not None:
+        raise tokens.error('unexpected ending: %r' % ' '.join(tokens))
+    return Required(*result)
+
+
+def parse_expr(tokens, options):
+    """expr ::= seq ( '|' seq )* ;"""
+    seq = parse_seq(tokens, options)
+    if tokens.current() != '|':
+        return seq
+    result = [Required(*seq)] if len(seq) > 1 else seq
+    while tokens.current() == '|':
+        tokens.move()
+        seq = parse_seq(tokens, options)
+        result += [Required(*seq)] if len(seq) > 1 else seq
+    return [Either(*result)] if len(result) > 1 else result
+
+
+def parse_seq(tokens, options):
+    """seq ::= ( atom [ '...' ] )* ;"""
+    result = []
+    while tokens.current() not in [None, ']', ')', '|']:
+        atom = parse_atom(tokens, options)
+        if tokens.current() == '...':
+            atom = [OneOrMore(*atom)]
+            tokens.move()
+        result += atom
+    return result
+
+
+def parse_atom(tokens, options):
+    """atom ::= '(' expr ')' | '[' expr ']' | 'options'
+             | long | shorts | argument | command ;
+    """
+    token = tokens.current()
+    result = []
+    if token in '([':
+        tokens.move()
+        matching, pattern = {'(': [')', Required], '[': [']', Optional]}[token]
+        result = pattern(*parse_expr(tokens, options))
+        if tokens.move() != matching:
+            raise tokens.error("unmatched '%s'" % token)
+        return [result]
+    elif token == 'options':
+        tokens.move()
+        return [OptionsShortcut()]
+    elif token.startswith('--') and token != '--':
+        return parse_long(tokens, options)
+    elif token.startswith('-') and token not in ('-', '--'):
+        return parse_shorts(tokens, options)
+    elif token.startswith('<') and token.endswith('>') or token.isupper():
+        return [Argument(tokens.move())]
+    else:
+        return [Command(tokens.move())]
+
+
+def parse_argv(tokens, options, options_first=False):
+    """Parse command-line argument vector.
+
+    If options_first:
+        argv ::= [ long | shorts ]* [ argument ]* [ '--' [ argument ]* ] ;
+    else:
+        argv ::= [ long | shorts | argument ]* [ '--' [ argument ]* ] ;
+
+    """
+    parsed = []
+    while tokens.current() is not None:
+        if tokens.current() == '--':
+            return parsed + [Argument(None, v) for v in tokens]
+        elif tokens.current().startswith('--'):
+            parsed += parse_long(tokens, options)
+        elif tokens.current().startswith('-') and tokens.current() != '-':
+            parsed += parse_shorts(tokens, options)
+        elif options_first:
+            return parsed + [Argument(None, v) for v in tokens]
+        else:
+            parsed.append(Argument(None, tokens.move()))
+    return parsed
+
+
+def parse_defaults(doc):
+    defaults = []
+    for s in parse_section('options:', doc):
+        # FIXME corner case "bla: options: --foo"
+        _, _, s = s.partition(':')  # get rid of "options:"
+        split = re.split('\n[ \t]*(-\S+?)', '\n' + s)[1:]
+        split = [s1 + s2 for s1, s2 in zip(split[::2], split[1::2])]
+        options = [Option.parse(s) for s in split if s.startswith('-')]
+        defaults += options
+    return defaults
+
+
+def parse_section(name, source):
+    pattern = re.compile('^([^\n]*' + name + '[^\n]*\n?(?:[ \t].*?(?:\n|$))*)',
+                         re.IGNORECASE | re.MULTILINE)
+    return [s.strip() for s in pattern.findall(source)]
+
+
+def formal_usage(section):
+    _, _, section = section.partition(':')  # drop "usage:"
+    pu = section.split()
+    return '( ' + ' '.join(') | (' if s == pu[0] else s for s in pu[1:]) + ' )'
+
+
+def extras(help, version, options, doc):
+    if help and any((o.name in ('-h', '--help')) and o.value for o in options):
+        print(doc.strip("\n"))
+        sys.exit()
+    if version and any(o.name == '--version' and o.value for o in options):
+        print(version)
+        sys.exit()
+
+
+class Dict(dict):
+    def __repr__(self):
+        return '{%s}' % ',\n '.join('%r: %r' % i for i in sorted(self.items()))
+
+
+def docopt(doc, argv=None, help=True, version=None, options_first=False):
+    """Parse `argv` based on command-line interface described in `doc`.
+
+    `docopt` creates your command-line interface based on its
+    description that you pass as `doc`. Such description can contain
+    --options, <positional-argument>, commands, which could be
+    [optional], (required), (mutually | exclusive) or repeated...
+
+    Parameters
+    ----------
+    doc : str
+        Description of your command-line interface.
+    argv : list of str, optional
+        Argument vector to be parsed. sys.argv[1:] is used if not
+        provided.
+    help : bool (default: True)
+        Set to False to disable automatic help on -h or --help
+        options.
+    version : any object
+        If passed, the object will be printed if --version is in
+        `argv`.
+    options_first : bool (default: False)
+        Set to True to require options precede positional arguments,
+        i.e. to forbid options and positional arguments intermix.
+
+    Returns
+    -------
+    args : dict
+        A dictionary, where keys are names of command-line elements
+        such as e.g. "--verbose" and "<path>", and values are the
+        parsed values of those elements.
+
+    Example
+    -------
+    >>> from docopt import docopt
+    >>> doc = '''
+    ... Usage:
+    ...     my_program tcp <host> <port> [--timeout=<seconds>]
+    ...     my_program serial <port> [--baud=<n>] [--timeout=<seconds>]
+    ...     my_program (-h | --help | --version)
+    ...
+    ... Options:
+    ...     -h, --help  Show this screen and exit.
+    ...     --baud=<n>  Baudrate [default: 9600]
+    ... '''
+    >>> argv = ['tcp', '127.0.0.1', '80', '--timeout', '30']
+    >>> docopt(doc, argv)
+    {'--baud': '9600',
+     '--help': False,
+     '--timeout': '30',
+     '--version': False,
+     '<host>': '127.0.0.1',
+     '<port>': '80',
+     'serial': False,
+     'tcp': True}
+
+    See also
+    --------
+    * For video introduction see http://docopt.org
+    * Full documentation is available in README.rst as well as online
+      at https://github.com/docopt/docopt#readme
+
+    """
+    argv = sys.argv[1:] if argv is None else argv
+
+    usage_sections = parse_section('usage:', doc)
+    if len(usage_sections) == 0:
+        raise DocoptLanguageError('"usage:" (case-insensitive) not found.')
+    if len(usage_sections) > 1:
+        raise DocoptLanguageError('More than one "usage:" (case-insensitive).')
+    DocoptExit.usage = usage_sections[0]
+
+    options = parse_defaults(doc)
+    pattern = parse_pattern(formal_usage(DocoptExit.usage), options)
+    # [default] syntax for argument is disabled
+    #for a in pattern.flat(Argument):
+    #    same_name = [d for d in arguments if d.name == a.name]
+    #    if same_name:
+    #        a.value = same_name[0].value
+    argv = parse_argv(Tokens(argv), list(options), options_first)
+    pattern_options = set(pattern.flat(Option))
+    for options_shortcut in pattern.flat(OptionsShortcut):
+        doc_options = parse_defaults(doc)
+        options_shortcut.children = list(set(doc_options) - pattern_options)
+        #if any_options:
+        #    options_shortcut.children += [Option(o.short, o.long, o.argcount)
+        #                    for o in argv if type(o) is Option]
+    extras(help, version, argv, doc)
+    matched, left, collected = pattern.fix().match(argv)
+    if matched and left == []:  # better error message if left?
+        return Dict((a.name, a.value) for a in (pattern.flat() + collected))
+    raise DocoptExit()
-- 
1.8.5.1

[Buildroot] [PATCH v5 02/11] autobuild-run: use docopt for argument parsing

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

Using docopt, argument parsing becomes trivial. Adding a new argument is
a matter of updating the usage text.
This commit removes the original cumbersome argument handling and uses
docopt instead. A method is added to import the settings from the
configuration file in a similar dictionary as the one created by docopt,
so that both can be merged (giving priority to the configuration file,
as before).

With these changes, each option can be passed as argument and in the
configuration file. This means that http-user and http-password can now
also be added as argument (even though passing the password on the
command-line is not recommended).

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 134 +++++++++++++++++++++++++++-----------------------
 1 file changed, 72 insertions(+), 62 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 4d9ef90..b686a78 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -57,6 +57,37 @@
 #   BR2_PACKAGE_CLASSPATH=y, improve the script to detect whether the
 #   necessary host machine requirements are there to build classpath.
 
+"""autobuild-run - run Buildroot autobuilder
+
+Usage: autobuild-run [options]
+
+Options:
+  -h, --help                     show this help message and exit
+  -V, --version                  show version
+  -n, --ninstances NINSTANCES    number of parallel instances [default: 1]
+  -j, --njobs NJOBS              number of parallel jobs [default: 1]
+  -s, --submitter SUBMITTER      name/machine of submitter [default: N/A]
+  --http-login LOGIN             username to send results with
+  --http-password PASSWORD       password to send results with (for security
+                                 reasons it is recommended to define this in the
+                                 config file instead, with user-read permissions
+                                 only)
+  -c, --config CONFIG            path to configuration file
+
+Format of the configuration file:
+
+  All arguments can also be specified in the configuration file specified with
+  --config, using 'key = value' format (not including the leading --
+  characters). For example:
+
+   [main]
+   ninstances = <value>
+   njobs = <value>
+   http-login = <value>
+   http-password = <value>
+   submitter = <value>
+"""
+
 import urllib2
 import csv
 from random import randint
@@ -70,6 +101,7 @@ import sys
 import hashlib
 import argparse
 import ConfigParser
+import docopt
 
 MAX_DURATION = 60 * 60 * 4
 VERSION = 1
@@ -522,81 +554,59 @@ def run_instance(instance, njobs, http_login, http_password, submitter, sysinfo)
         ret = do_build(instance, njobs, instance_log)
         send_results(instance, http_login, http_password, submitter, instance_log, ret)
 
-def config_get():
-    """Get configuration parameters, either from the command line or the config file"""
+# args / config file merging inspired by:
+# https://github.com/docopt/docopt/blob/master/examples/config_file_example.py
 
-    epilog_text = """
-Format of the configuration file:
+def load_ini_config(configfile):
+    """Load configuration from file, returning a docopt-like dictionary"""
 
-   [main]
-   ninstances = <value>
-   njobs = <value>
-   http-login = <value>
-   http-password = <value>
-   submitter = <value>
-"""
+    if not os.path.exists(configfile):
+        print "ERROR: configuration file %s does not exist" % configfile
+        sys.exit(1)
+
+    config = ConfigParser.RawConfigParser()
+    if not config.read(configfile):
+        print "ERROR: cannot parse configuration file %s" % configfile
+        sys.exit(1)
+
+    # Prepend '--' to options specified in the config file, so they can be
+    # merged with those given on the command-line
+    return dict(('--%s' % key, value) for key, value in config.items('main'))
 
-    parser = argparse.ArgumentParser(description='Run Buildroot autobuilder',
-                                     formatter_class=argparse.RawDescriptionHelpFormatter,
-                                     epilog=epilog_text)
-    parser.add_argument("--ninstances", '-n', metavar="NINSTANCES",
-                        help="Number of parallel instances", default=None)
-    parser.add_argument("--njobs", '-j', metavar="NJOBS",
-                        help="Number of parallel jobs", default=None)
-    parser.add_argument("--submitter", '-s', metavar="SUBMITTER",
-                        help="Name/machine of submitter")
-    parser.add_argument("--config", '-c', metavar="CONFIG",
-                        help="Path to configuration file")
-    args = parser.parse_args()
-
-    ninstances = 1
-    njobs = 1
-    http_login = None
-    http_password = None
-    submitter = "N/A"
-
-    if args.config:
-        if not os.path.exists(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
-            sys.exit(1)
-        if parser.has_option('main', 'ninstances'):
-            ninstances = parser.getint('main', 'ninstances')
-        if parser.has_option('main', 'njobs'):
-            njobs = parser.getint('main', 'njobs')
-        if parser.has_option('main', 'http-login'):
-            http_login = parser.get('main', 'http-login')
-        if parser.has_option('main', 'http-password'):
-            http_password = parser.get('main', 'http-password')
-        if parser.has_option('main', 'submitter'):
-            submitter = parser.get('main', 'submitter')
-
-    if args.njobs:
-        njobs = int(args.njobs)
-    if args.ninstances:
-        ninstances = int(args.ninstances)
-    if args.submitter:
-        submitter = args.submitter
-
-    return (ninstances, njobs, http_login, http_password, submitter)
+
+def merge(dict_1, dict_2):
+    """Merge two dictionaries.
+
+    Values that evaluate to true take priority over falsy values.
+    `dict_1` takes priority over `dict_2`.
+
+    """
+    return dict((str(key), dict_1.get(key) or dict_2.get(key))
+                for key in set(dict_2) | set(dict_1))
 
 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:
+
+    args = docopt.docopt(__doc__, version=VERSION)
+
+    if args['--config']:
+        ini_config = load_ini_config(args['--config'])
+        # merge config/args, priority given to config
+        args = merge(ini_config, args)
+
+    check_requirements(args['--http-login'], args['--http-password'])
+    if args['--http-login'] is None or args['--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"
     def sigterm_handler(signum, frame):
         os.killpg(os.getpgid(os.getpid()), signal.SIGTERM)
         sys.exit(1)
     processes = []
-    for i in range(0, ninstances):
-        p = Process(target=run_instance, args=(i, njobs, http_login, http_password, submitter, sysinfo))
+    for i in range(0, int(args['--ninstances'])):
+        p = Process(target=run_instance, args=(i, int(args['--njobs']),
+                args['--http-login'], args['--http-password'],
+                args['--submitter'], sysinfo))
         p.start()
         processes.append(p)
     signal.signal(signal.SIGTERM, sigterm_handler)
-- 
1.8.5.1

[Buildroot] [PATCH v5 03/11] autobuild-run: add option --make-opts for custom Buildroot options

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

In some environments, the Buildroot configuration may need minor
tweaking. For example, in corporate environments behind a firewall,
version control commands like git, svn, ... may actually need wrappers.
Such wrappers could be configured using the corresponding BR2_<CMD>
configuration options.

Since these tweaks are very environment-specific, they should not be
part of the configuration file itself. Otherwise, other people trying to
reproduce the build in another environment will see builds failing due
to other reasons, for example the wrapper not being found.

This commit provides a new configuration option 'make-opts', passable as
argument or in the config file, that accepts a string of make options to
pass to the Buildroot make command. For example:
    --make-opts="BR2_GIT=git-wrapper BR2_svn=svn-wrapper"

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index b686a78..8b0091d 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -72,6 +72,9 @@ Options:
                                  reasons it is recommended to define this in the
                                  config file instead, with user-read permissions
                                  only)
+  --make-opts OPTSTRING          string of extra options to pass to Buildroot
+                                 make, such as specific command wrappers
+                                 [default: ]
   -c, --config CONFIG            path to configuration file
 
 Format of the configuration file:
@@ -420,7 +423,7 @@ def gen_config(instance, log, sysinfo):
 
     return 0
 
-def do_build(instance, njobs, log):
+def do_build(instance, njobs, log, make_opts):
     """Run the build itself"""
 
     idir = "instance-%d" % instance
@@ -433,8 +436,10 @@ 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)
+    cmd = ["timeout", str(MAX_DURATION), "make", "O=%s" % outputdir,
+            "-C", srcdir, "BR2_DL_DIR=%s" % dldir, "BR2_JLEVEL=%s" % njobs] \
+          + make_opts.split()
+    ret = subprocess.call(cmd, stdout=f, stderr=f)
     # 124 is a special error code that indicates we have reached the
     # timeout
     if ret == 124:
@@ -525,7 +530,8 @@ def send_results(instance, http_login, http_password, submitter, log, result):
         os.rename(os.path.join(outputdir, "results.tar.bz2"), resultfilename)
         log_write(log, "INFO: results saved as %s" % resultfilename)
 
-def run_instance(instance, njobs, http_login, http_password, submitter, sysinfo):
+def run_instance(instance, njobs, http_login, http_password, submitter,
+                 make_opts, sysinfo):
     """Main per-instance loop
 
     Prepare the build, generate a configuration, run the build, and submit the
@@ -551,7 +557,7 @@ def run_instance(instance, njobs, http_login, http_password, submitter, sysinfo)
         if ret != 0:
             continue
 
-        ret = do_build(instance, njobs, instance_log)
+        ret = do_build(instance, njobs, instance_log, make_opts)
         send_results(instance, http_login, http_password, submitter, instance_log, ret)
 
 # args / config file merging inspired by:
@@ -606,7 +612,7 @@ def main():
     for i in range(0, int(args['--ninstances'])):
         p = Process(target=run_instance, args=(i, int(args['--njobs']),
                 args['--http-login'], args['--http-password'],
-                args['--submitter'], sysinfo))
+                args['--submitter'], args['--make-opts'], sysinfo))
         p.start()
         processes.append(p)
     signal.signal(signal.SIGTERM, sigterm_handler)
-- 
1.8.5.1

[Buildroot] [PATCH v5 04/11] autobuild-run: use **kwargs to avoid explicit parameter passthroughs

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

The current version of autobuild-run has some extensive explicit
parameter passing, for example:

- fixup_config needs sysinfo, passed via gen_config, in turn via
  run_instance, in turn via main.
- send_results needs username/password settings, passed via
  run_instance, in turn via main.

Everytime a leaf function needs an extra parameter (for example coming
from the arguments or config file), the entire call chain needs to be
adapted to pass along that parameter.

This patch introduces the **kwargs dictionary principle, that allows
implicit parameter passing. A function can accept this dictionary and
extract parameters from it by name. The dictionary can be passed as a
whole to a child function, without explicitly enumerating which entries
in the dictionary are needed in the child.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---

v5: use dict(foo = A, bar = B) instead of { 'foo' = A, 'bar' = B } as this
    is more readable.

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

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 8b0091d..4fc883a 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -189,7 +189,7 @@ def get_toolchain_configs():
         configs.append(config)
     return configs
 
-def prepare_build(instance, log):
+def prepare_build(**kwargs):
     """Prepare for the next build of the specified instance
 
     This function prepares the build by making sure all the needed
@@ -197,7 +197,8 @@ def prepare_build(instance, log):
     code, and cleaning up remaining stuff from previous builds.
     """
 
-    idir = "instance-%d" % instance
+    idir = "instance-%d" % kwargs['instance']
+    log = kwargs['log']
 
     log_write(log, "INFO: preparing a new build")
 
@@ -246,7 +247,7 @@ def prepare_build(instance, log):
 
     return 0
 
-def fixup_config(instance, sysinfo):
+def fixup_config(**kwargs):
     """Finalize the configuration and reject any problematic combinations
 
     This function returns 'True' when the configuration has been
@@ -255,9 +256,10 @@ def fixup_config(instance, sysinfo):
     generated).
     """
 
-    idir = "instance-%d" % instance
-    outputdir = os.path.join(idir, "output")
+    idir = "instance-%d" % kwargs['instance']
+    sysinfo = kwargs['sysinfo']
 
+    outputdir = os.path.join(idir, "output")
     with open(os.path.join(outputdir, ".config")) as configf:
         configlines = configf.readlines()
 
@@ -347,7 +349,7 @@ def fixup_config(instance, sysinfo):
 
     return True
 
-def gen_config(instance, log, sysinfo):
+def gen_config(**kwargs):
     """Generate a new random configuration
 
     This function generates the configuration, by choosing a random
@@ -355,7 +357,9 @@ def gen_config(instance, log, sysinfo):
     packages.
     """
 
-    idir = "instance-%d" % instance
+    idir = "instance-%d" % kwargs['instance']
+    log = kwargs['log']
+
     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
@@ -406,7 +410,7 @@ def gen_config(instance, log, sysinfo):
         if ret != 0:
             log_write(log, "ERROR: cannot generate random configuration")
             return -1
-        if fixup_config(instance, sysinfo):
+        if fixup_config(**kwargs):
             break
 
     ret = subprocess.call(["yes '' 2>/dev/null| make O=%s -C %s oldconfig" % \
@@ -423,10 +427,12 @@ def gen_config(instance, log, sysinfo):
 
     return 0
 
-def do_build(instance, njobs, log, make_opts):
+def do_build(**kwargs):
     """Run the build itself"""
 
-    idir = "instance-%d" % instance
+    idir = "instance-%d" % kwargs['instance']
+    log = kwargs['log']
+
     # 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
@@ -437,8 +443,9 @@ def do_build(instance, njobs, log, make_opts):
     f = open(os.path.join(outputdir, "logfile"), "w+")
     log_write(log, "INFO: build started")
     cmd = ["timeout", str(MAX_DURATION), "make", "O=%s" % outputdir,
-            "-C", srcdir, "BR2_DL_DIR=%s" % dldir, "BR2_JLEVEL=%s" % njobs] \
-          + make_opts.split()
+            "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
+            "BR2_JLEVEL=%s" % kwargs['njobs']] \
+          + kwargs['make_opts'].split()
     ret = subprocess.call(cmd, stdout=f, stderr=f)
     # 124 is a special error code that indicates we have reached the
     # timeout
@@ -455,7 +462,7 @@ def do_build(instance, njobs, log, make_opts):
     log_write(log, "INFO: build successful")
     return 0
 
-def send_results(instance, http_login, http_password, submitter, log, result):
+def send_results(result, **kwargs):
     """Prepare and store/send tarball with results
 
     This function prepares the tarball with the results, and either
@@ -463,7 +470,9 @@ def send_results(instance, http_login, http_password, submitter, log, result):
     are available) or stores them locally as tarballs.
     """
 
-    idir = "instance-%d" % instance
+    idir = "instance-%d" % kwargs['instance']
+    log = kwargs['log']
+
     outputdir = os.path.abspath(os.path.join(idir, "output"))
     srcdir = os.path.join(idir, "buildroot")
     resultdir = os.path.join(outputdir, "results")
@@ -498,7 +507,7 @@ def send_results(instance, http_login, http_password, submitter, log, result):
     resultf.close()
 
     with open(os.path.join(resultdir, "submitter"), "w+") as submitterf:
-        submitterf.write(submitter)
+        submitterf.write(kwargs['submitter'])
 
     # Yes, shutil.make_archive() would be nice, but it doesn't exist
     # in Python 2.6.
@@ -508,11 +517,12 @@ def send_results(instance, http_login, http_password, submitter, log, result):
         log_write(log, "ERROR: could not make results tarball")
         sys.exit(1)
 
-    if http_login and http_password:
+    if kwargs['http_login'] and kwargs['http_password']:
         # 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),
+        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",
@@ -526,39 +536,40 @@ def send_results(instance, http_login, http_password, submitter, log, result):
         # No http login/password, keep tarballs locally
         with open(os.path.join(outputdir, "results.tar.bz2"), 'rb') as f:
             sha1 = hashlib.sha1(f.read()).hexdigest()
-        resultfilename = "instance-%d-%s.tar.bz2" % (instance, sha1)
+        resultfilename = "instance-%d-%s.tar.bz2" % (kwargs['instance'], sha1)
         os.rename(os.path.join(outputdir, "results.tar.bz2"), resultfilename)
         log_write(log, "INFO: results saved as %s" % resultfilename)
 
-def run_instance(instance, njobs, http_login, http_password, submitter,
-                 make_opts, sysinfo):
+def run_instance(**kwargs):
     """Main per-instance loop
 
     Prepare the build, generate a configuration, run the build, and submit the
     results.
     """
 
-    idir = "instance-%d" % instance
+    idir = "instance-%d" % kwargs['instance']
 
     # If it doesn't exist, create the instance directory
     if not os.path.exists(idir):
         os.mkdir(idir)
 
-    instance_log = open(os.path.join(idir, "instance.log"), "a+")
-    log_write(instance_log, "INFO: instance started")
+    kwargs['log'] = open(os.path.join(idir, "instance.log"), "a+")
+    log_write(kwargs['log'], "INFO: instance started")
+
     while True:
         check_version()
 
-        ret = prepare_build(instance, instance_log)
+        ret = prepare_build(**kwargs)
         if ret != 0:
             continue
 
-        ret = gen_config(instance, instance_log, sysinfo)
+        ret = gen_config(**kwargs)
         if ret != 0:
             continue
 
-        ret = do_build(instance, njobs, instance_log, make_opts)
-        send_results(instance, http_login, http_password, submitter, instance_log, ret)
+        ret = do_build(**kwargs)
+
+        send_results(ret, **kwargs)
 
 # args / config file merging inspired by:
 # https://github.com/docopt/docopt/blob/master/examples/config_file_example.py
@@ -610,9 +621,15 @@ def main():
         sys.exit(1)
     processes = []
     for i in range(0, int(args['--ninstances'])):
-        p = Process(target=run_instance, args=(i, int(args['--njobs']),
-                args['--http-login'], args['--http-password'],
-                args['--submitter'], args['--make-opts'], sysinfo))
+        p = Process(target=run_instance, kwargs=dict(
+                instance = i,
+                njobs = args['--njobs'],
+                sysinfo = sysinfo,
+                http_login = args['--http-login'],
+                http_password = args['--http-password'],
+                submitter = args['--submitter'],
+                make_opts = args['--make-opts']
+            ))
         p.start()
         processes.append(p)
     signal.signal(signal.SIGTERM, sigterm_handler)
-- 
1.8.5.1

[Buildroot] [PATCH v5 05/11] autobuild-run: check-requirements does not need to know the login details

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

check-requirements simply has to know if the results have to be sent, so
it can check on some extra requirements. The username and password are
irrelevant here.
This commit introduces a boolean variable upload to hide these details.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 4fc883a..ab53d3f 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -120,12 +120,12 @@ def check_version():
         print "ERROR: script version too old, please upgrade."
         sys.exit(1)
 
-def check_requirements(http_login, http_password):
+def check_requirements(upload=False):
     devnull = open(os.devnull, "w")
     needed_progs = ["make", "git", "gcc", "timeout"]
     missing_requirements = False
 
-    if http_login and http_password:
+    if upload:
         needed_progs.append("curl")
 
     for prog in needed_progs:
@@ -517,7 +517,7 @@ def send_results(result, **kwargs):
         log_write(log, "ERROR: could not make results tarball")
         sys.exit(1)
 
-    if kwargs['http_login'] and kwargs['http_password']:
+    if kwargs['upload']:
         # 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.
@@ -612,10 +612,15 @@ def main():
         # merge config/args, priority given to config
         args = merge(ini_config, args)
 
-    check_requirements(args['--http-login'], args['--http-password'])
-    if args['--http-login'] is None or args['--http-password'] is None:
+    # http_login/password could theoretically be allowed as empty, so check
+    # explicitly on None.
+    upload = (args['--http-login'] is not None) \
+                and (args['--http-password'] is not None)
+    check_requirements(upload)
+    if not upload:
         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)
@@ -628,7 +633,8 @@ def main():
                 http_login = args['--http-login'],
                 http_password = args['--http-password'],
                 submitter = args['--submitter'],
-                make_opts = args['--make-opts']
+                make_opts = args['--make-opts'],
+                upload = upload
             ))
         p.start()
         processes.append(p)
-- 
1.8.5.1

[Buildroot] [PATCH v5 06/11] autobuild-run: set LC_ALL=C to not use locale settings of host machine

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

If the host machine running autobuild-run happens to have set a
non-English locale, the error messages will be displayed in that
language too. For public results like those generated in the Buildroot
autobuilders, this is a problem.

Therefore, set LC_ALL to a fixed locale (C) at the beginning of the script.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index ab53d3f..d085029 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -48,8 +48,6 @@
 #   of just using the last 500 lines of the build log, search the
 #   start of the build of the failing package.
 #
-# - Add LC_ALL=C where appropriate.
-#
 # - Include the config.log file (when it exists) in the tarball for
 #   failed builds when the failure occurs on an autotools package.
 #
@@ -602,6 +600,11 @@ def merge(dict_1, dict_2):
                 for key in set(dict_2) | set(dict_1))
 
 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()
 
-- 
1.8.5.1

[Buildroot] [PATCH v5 07/11] autobuild-run: improve the logic to generate build-end.log

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

This patch makes the generation of build-end.log more intelligent, by
not simply adding the 500 last lines, but instead capture the entire
output of the failed package.

If no failure reason can be found (possibly a good run) or the start for
that package cannot be found, use the 500 last lines as fallback.

The logic to find the failed package is taken from the result import
script web/import.inc.php. If needed, the search range of the last 3
lines could be extended.

Note that the package-version string extracted from the build log is
split on the last hyphen into a tuple (package, version). This is
needed to find back the beginning of that package's build (where the
package name is separated from the version with a space, not a hyphen).
However, this logic will fail to work when the version also contains a
hyphen, for example lmbench-3.0-a9. In this case, the resulting tuple
will be (lmbench-3.0,a9) and the text searched in the build log
'>>> lmbench-3.0 a9' (which will not be found). In this case, the
fallback of the last 500 lines will be used.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 53 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 7 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index d085029..32466e8 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -44,10 +44,6 @@
 #
 # TODO:
 #
-# - Improve the logic that generates the 'build-end.log' file. Instead
-#   of just using the last 500 lines of the build log, search the
-#   start of the build of the failing package.
-#
 # - Include the config.log file (when it exists) in the tarball for
 #   failed builds when the failure occurs on an autotools package.
 #
@@ -491,9 +487,52 @@ def send_results(result, **kwargs):
     subprocess.call(["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)
+
+    def get_failure_reason():
+        # Output is a tuple (package, version), or None.
+
+        lastlines = subprocess.check_output(["tail", "-n", "3",
+                        os.path.join(outputdir, "logfile")]).splitlines()
+
+        import re
+        regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
+        for line in lastlines:
+            m = regexp.search(line)
+            if m:
+                return m.group(1).rsplit('-', 1)
+
+        # not found
+        return None
+
+    def extract_end_log(resultfile):
+        """Save the last part of the build log, starting from the failed package"""
+
+        def extract_last_500_lines():
+            subprocess.call(["tail -500 %s > %s" % \
+                             (os.path.join(outputdir, "logfile"), resultfile)],
+                            shell=True)
+
+        reason = get_failure_reason()
+        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)
+            # Search for first action on the failed package
+            offset = mf.find('>>> %s' % ' '.join(reason))
+            if offset != -1:
+                with open(resultfile, "w") as endlog:
+                    endlog.write(mf[offset:])
+            else:
+                # not found, use last 500 lines as fallback
+                extract_last_500_lines()
+
+            mf.close()
+            f.close()
+
+    extract_end_log(os.path.join(resultdir, "build-end.log"))
 
     resultf = open(os.path.join(resultdir, "status"), "w+")
     if result == 0:
-- 
1.8.5.1

[Buildroot] [PATCH v5 08/11] autobuild-run: save config.log files for failed package

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

This patch will save all config.log files of the failed package in the
result tarball, to help problem analysis. This is typically useful for
autotools packages, although the added logic does not check that
explicitly (i.e. any failing package that has a config.log file in the
output directory will see this file saved).

The saving of config.log files happens recursively, the directory
structure is preserved in the result directory.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 32466e8..198c7bc 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -44,9 +44,6 @@
 #
 # TODO:
 #
-# - Include the config.log file (when it exists) in the tarball for
-#   failed builds when the failure occurs on an autotools package.
-#
 # - 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.
@@ -534,6 +531,26 @@ def send_results(result, **kwargs):
 
     extract_end_log(os.path.join(resultdir, "build-end.log"))
 
+    def copy_config_log_files():
+        """Recursively copy any config.log files from the failing package"""
+
+        reason = get_failure_reason()
+        if not reason:
+            return
+
+        srcroot = os.path.join(outputdir, "build", '-'.join(reason))
+        destroot = os.path.join(resultdir, '-'.join(reason))
+
+        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':
+                    os.makedirs(dest)
+                    shutil.copy(os.path.join(root, file), os.path.join(dest, file))
+
+    copy_config_log_files()
+
     resultf = open(os.path.join(resultdir, "status"), "w+")
     if result == 0:
         resultf.write("OK")
-- 
1.8.5.1

[Buildroot] [PATCH v5 09/11] autobuild-run: extend TODO list

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

Some misc. additions to the autobuild-run TODO list.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 198c7bc..237d443 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -47,6 +47,17 @@
 # - 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
+#   a failure
+# - Properly handle kill of main script: stop all running children and do not
+#   print out stacktraces
+# - Fix problem in removal of output directory: sometimes this fails with
+#   message 'directory not empty' which suggests that someone is writing to the
+#   directory at the time of removal.
 
 """autobuild-run - run Buildroot autobuilder
 
-- 
1.8.5.1

[Buildroot] [PATCH v5 10/11] autobuild-run: kill all children on SIGTERM

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

The autobuild-run spawns the main build process through the timeout
command. To handle its job correctly, this command creates all children
in its own process group, different from the process group of
autobuild-run itself.
Thus, when autobuild-run is killed and the signal handler kills the
entire process group, the build processes run through timeout remain
alive.

To handle this, record the PIDs of the timeout processes in an array
shared between the main autobuild-run process and its instances. The
signal handler will iterate over all active processes in this array, and
kill them explicitly.

If a new timeout process would be started after the signal handler was
invoked but before the entire process tree is killed, this process could
remain alive too. To prevent this from occurring, the signal handler now
starts with terminating all instances.

Lastly, the signal handler would be called for all instances, which is
not intended, so prevent that by uninstalling the signal handler as a
first step of the handler itself.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 237d443..3c448bd 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -97,9 +97,10 @@ import urllib2
 import csv
 from random import randint
 import subprocess
-from multiprocessing import Process
+import multiprocessing
 import signal
 import os
+import errno
 import shutil
 from time import localtime, strftime
 import sys
@@ -444,11 +445,16 @@ def do_build(**kwargs):
     srcdir = os.path.join(idir, "buildroot")
     f = open(os.path.join(outputdir, "logfile"), "w+")
     log_write(log, "INFO: build started")
+
     cmd = ["timeout", str(MAX_DURATION), "make", "O=%s" % outputdir,
             "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
             "BR2_JLEVEL=%s" % kwargs['njobs']] \
           + kwargs['make_opts'].split()
-    ret = subprocess.call(cmd, stdout=f, stderr=f)
+    sub = subprocess.Popen(cmd, stdout=f, stderr=f)
+    kwargs['buildpid'][kwargs['instance']] = sub.pid
+    ret = sub.wait()
+    kwargs['buildpid'][kwargs['instance']] = 0
+
     # 124 is a special error code that indicates we have reached the
     # timeout
     if ret == 124:
@@ -692,8 +698,32 @@ def main():
         print "WARN: tarballs of results will be kept locally only"
 
     def sigterm_handler(signum, frame):
+        """Kill all children"""
+
+        # uninstall signal handler to prevent being called for all subprocesses
+        signal.signal(signal.SIGTERM, signal.SIG_DFL)
+
+        # stop all instances to prevent new children to be spawned
+        for p in processes:
+            p.terminate()
+
+        # kill build processes started with timeout (that puts its children
+        # explicitly in a separate process group)
+        for pid in buildpid:
+            if pid == 0:
+                continue
+            try:
+                os.kill(pid, signal.SIGTERM)
+            except OSError as e:
+                if e.errno != errno.ESRCH: # No such process, ignore
+                    raise
+
+        # kill any remaining children in our process group
         os.killpg(os.getpgid(os.getpid()), signal.SIGTERM)
+
         sys.exit(1)
+
+    buildpid = multiprocessing.Array('i', int(args['--ninstances']))
     processes = []
     for i in range(0, int(args['--ninstances'])):
         p = Process(target=run_instance, kwargs=dict(
@@ -704,11 +734,14 @@ def main():
                 http_password = args['--http-password'],
                 submitter = args['--submitter'],
                 make_opts = args['--make-opts'],
-                upload = upload
+                upload = upload,
+                buildpid = buildpid
             ))
         p.start()
         processes.append(p)
+
     signal.signal(signal.SIGTERM, sigterm_handler)
+
     for p in processes:
         p.join()
 
-- 
1.8.5.1

[Buildroot] [PATCH v5 11/11] autobuild-run: catch KeyboardInterrupt in the same way as SIGTERM

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

Do not handle a keyboard interrupt (SIGINT) in any different way than a
SIGTERM signal (kill). In both cases, the entire process tree of the
autobuild-run process should be killed.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 scripts/autobuild-run | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 3c448bd..c8ce918 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -701,6 +701,7 @@ def main():
         """Kill all children"""
 
         # uninstall signal handler to prevent being called for all subprocesses
+        signal.signal(signal.SIGINT, signal.SIG_IGN)
         signal.signal(signal.SIGTERM, signal.SIG_DFL)
 
         # stop all instances to prevent new children to be spawned
@@ -740,6 +741,7 @@ def main():
         p.start()
         processes.append(p)
 
+    signal.signal(signal.SIGINT, sigterm_handler)
     signal.signal(signal.SIGTERM, sigterm_handler)
 
     for p in processes:
-- 
1.8.5.1

[Buildroot] [PATCH v5 10/11] autobuild-run: kill all children on SIGTERM

[Thomas De Schampheleire]

On Fri, Dec 12, 2014 at 9:04 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> The autobuild-run spawns the main build process through the timeout
> command. To handle its job correctly, this command creates all children
> in its own process group, different from the process group of
> autobuild-run itself.
> Thus, when autobuild-run is killed and the signal handler kills the
> entire process group, the build processes run through timeout remain
> alive.
>
> To handle this, record the PIDs of the timeout processes in an array
> shared between the main autobuild-run process and its instances. The
> signal handler will iterate over all active processes in this array, and
> kill them explicitly.
>
> If a new timeout process would be started after the signal handler was
> invoked but before the entire process tree is killed, this process could
> remain alive too. To prevent this from occurring, the signal handler now
> starts with terminating all instances.
>
> Lastly, the signal handler would be called for all instances, which is
> not intended, so prevent that by uninstalling the signal handler as a
> first step of the handler itself.
>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> ---
>  scripts/autobuild-run | 39 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 36 insertions(+), 3 deletions(-)

Some notes here:

- I forgot to remove the TODO entry in this patch. This could be done
when patches are merged or in a next iteration.

- Thomas Petazzoni previously proposed to get rid of 'timeout' and
handle the timeout in Python logic. I did investigate this path but
the end result is more complex. The solution would look like this:

    1. instead of calling subprocess.call('timeout make ...') from the
instance directly, one would spawn yet another process just for the
subprocess call. Inside this process, one would do
subprocess.call('make').

    2. the new Process would be joined with a timeout (join(timeout))

    3. however, while this join will return when the timeout expires,
the process itself will continue to live.  This is not what we want.
To solve that problem, we have to save the pid of the subprocess call,
share it with the instance that spawned the process using
multiprocessing.Value, and upon timeout, let the instance kill the
spawned subprocess and all its children. Killing the subprocess is not
that hard, but I think one would need to do special things to ensure
all the children are killed too. In fact, the timeout command already
handles all this: it creates a separate process group so that it can
kill the entire process group.

  Looking at these steps, step 4 is more or less what is done in the
current patch that still uses the 'timeout' command, while steps 1 and
2 are new and make the solution unnecessarily complex. Hence I opted
to keep 'timeout'.

Best regards,
Thomas

[Buildroot] [PATCH v5 10/11] autobuild-run: kill all children on SIGTERM

[Thomas De Schampheleire]

[now with the buildroot-subscribed address]

On Fri, Dec 12, 2014 at 9:04 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> The autobuild-run spawns the main build process through the timeout
> command. To handle its job correctly, this command creates all children
> in its own process group, different from the process group of
> autobuild-run itself.
> Thus, when autobuild-run is killed and the signal handler kills the
> entire process group, the build processes run through timeout remain
> alive.
>
> To handle this, record the PIDs of the timeout processes in an array
> shared between the main autobuild-run process and its instances. The
> signal handler will iterate over all active processes in this array, and
> kill them explicitly.
>
> If a new timeout process would be started after the signal handler was
> invoked but before the entire process tree is killed, this process could
> remain alive too. To prevent this from occurring, the signal handler now
> starts with terminating all instances.
>
> Lastly, the signal handler would be called for all instances, which is
> not intended, so prevent that by uninstalling the signal handler as a
> first step of the handler itself.


Some notes here:

- I forgot to remove the TODO entry in this patch. This could be done
when patches are merged or in a next iteration.

- Thomas Petazzoni previously proposed to get rid of 'timeout' and
handle the timeout in Python logic. I did investigate this path but
the end result is more complex. The solution would look like this:

    1. instead of calling subprocess.call('timeout make ...') from the
instance directly, one would spawn yet another process just for the
subprocess call. Inside this process, one would do
subprocess.call('make').

    2. the new Process would be joined with a timeout (join(timeout))

    3. however, while this join will return when the timeout expires,
the process itself will continue to live.  This is not what we want.
To solve that problem, we have to save the pid of the subprocess call,
share it with the instance that spawned the process using
multiprocessing.Value, and upon timeout, let the instance kill the
spawned subprocess and all its children. Killing the subprocess is not
that hard, but I think one would need to do special things to ensure
all the children are killed too. In fact, the timeout command already
handles all this: it creates a separate process group so that it can
kill the entire process group.

  Looking at these steps, step 4 is more or less what is done in the
current patch that still uses the 'timeout' command, while steps 1 and
2 are new and make the solution unnecessarily complex. Hence I opted
to keep 'timeout'.

Best regards,
Thomas

[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[Thomas De Schampheleire]

Hi Thomas,

On Fri, Dec 12, 2014 at 9:04 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> This series against the buildroot-test repo makes some improvements to the
> autobuild-run script.
>
> v5:
> - add two patches to handle kill/interrupt properly
> - make the kwargs dictionary passing to Process more readable
> - rebase
>
> v4:
> - rename do_send_results into upload (Peter)
>
> v3:
> - introduce missing usage of do_send_results (and move patch backwards)
> - update TODO list
>
> v2:
> - reorder patches
> - take into account comments on boolean logic in first patch
> - place docopt directly in scripts/ instead of in a subdirectory
> - remove patch adding --git (Yann, Thomas)
> - some bug fixes in the first version of the patches
>
> As you can see, this iteration still has the docopt patches. Thomas Petazzoni
> has expressed his reservations against the introduction of docopt, because the
> benefits wouldn't weigh out against the addition of an external Python module.
> Thomas told me on IRC he would have a deeper look onto this before making a
> final decision.
>
> In case the final decision is not to introduce docopt, following improvements
> could be done to the current code:
>   - save the arguments from the command-line into a dictionary instead of plain
>     variables
>   - keep the added ini_config() method to also save the options in a dictionary,
>     and use the added merge() method to merge both arguments and config options
>     into one final configuration, without needing to handle it manually.
>
>
> Thomas De Schampheleire (11):
>   scripts: add python module docopt
>   autobuild-run: use docopt for argument parsing
>   autobuild-run: add option --make-opts for custom Buildroot options
>   autobuild-run: use **kwargs to avoid explicit parameter passthroughs
>   autobuild-run: check-requirements does not need to know the login
>     details
>   autobuild-run: set LC_ALL=C to not use locale settings of host machine
>   autobuild-run: improve the logic to generate build-end.log
>   autobuild-run: save config.log files for failed package
>   autobuild-run: extend TODO list
>   autobuild-run: kill all children on SIGTERM
>   autobuild-run: catch KeyboardInterrupt in the same way as SIGTERM
>
>  scripts/autobuild-run      | 350 +++++++++++++++++++--------
>  scripts/docopt.LICENSE-MIT |  23 ++
>  scripts/docopt.py          | 581 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 851 insertions(+), 103 deletions(-)
>  create mode 100644 scripts/docopt.LICENSE-MIT
>  create mode 100644 scripts/docopt.py
>

How about this patchset?

Thanks,
Thomas

[Buildroot] [PATCH v5 02/11] autobuild-run: use docopt for argument parsing

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Fri, 12 Dec 2014 21:04:47 +0100, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> 
> Using docopt, argument parsing becomes trivial. Adding a new argument is
> a matter of updating the usage text.
> This commit removes the original cumbersome argument handling and uses
> docopt instead. A method is added to import the settings from the
> configuration file in a similar dictionary as the one created by docopt,
> so that both can be merged (giving priority to the configuration file,
> as before).

Actually, in the current implementation, priority is given to the
command line option, and not the configuration file. Like most Unix
tools, I believe.

So to make your implementation behave like the existing one, I believe
I just need to change:

> +    if args['--config']:
> +        ini_config = load_ini_config(args['--config'])
> +        # merge config/args, priority given to config
> +        args = merge(ini_config, args)

to:

	   # merge config/args, priority given to args
	   args = merge(args, ini_config)

No need to resend just for this, I can fixup when applying.

Thanks!

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

[Buildroot] [PATCH v5 04/11] autobuild-run: use **kwargs to avoid explicit parameter passthroughs

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Fri, 12 Dec 2014 21:04:49 +0100, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> 
> The current version of autobuild-run has some extensive explicit
> parameter passing, for example:
> 
> - fixup_config needs sysinfo, passed via gen_config, in turn via
>   run_instance, in turn via main.
> - send_results needs username/password settings, passed via
>   run_instance, in turn via main.
> 
> Everytime a leaf function needs an extra parameter (for example coming
> from the arguments or config file), the entire call chain needs to be
> adapted to pass along that parameter.
> 
> This patch introduces the **kwargs dictionary principle, that allows
> implicit parameter passing. A function can accept this dictionary and
> extract parameters from it by name. The dictionary can be passed as a
> whole to a child function, without explicitly enumerating which entries
> in the dictionary are needed in the child.

I must say I find this **kwargs thing a bit weird. Yes indeed, it
avoids passing several parameters to functions. But on the other hand,
it means that you're passing the *entire* context to *all* functions.
Which more or less boils down to using global variables. It means that
it isn't clear looking at the prototype of a function, which parameters
it is taking.

Maybe it's just that I'm not doing enough Python, and I'm thinking too
much C, but I find this **kwargs to not be such a great idea. But oh
well, since this is Python, I guess I should convince myself to use the
Python spirit :)

Best regards,

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

[Buildroot] [PATCH v5 02/11] autobuild-run: use docopt for argument parsing

[Thomas De Schampheleire]

On Thu, Feb 5, 2015 at 11:52 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Fri, 12 Dec 2014 21:04:47 +0100, Thomas De Schampheleire wrote:
>> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>>
>> Using docopt, argument parsing becomes trivial. Adding a new argument is
>> a matter of updating the usage text.
>> This commit removes the original cumbersome argument handling and uses
>> docopt instead. A method is added to import the settings from the
>> configuration file in a similar dictionary as the one created by docopt,
>> so that both can be merged (giving priority to the configuration file,
>> as before).
>
> Actually, in the current implementation, priority is given to the
> command line option, and not the configuration file. Like most Unix
> tools, I believe.
>
> So to make your implementation behave like the existing one, I believe
> I just need to change:
>
>> +    if args['--config']:
>> +        ini_config = load_ini_config(args['--config'])
>> +        # merge config/args, priority given to config
>> +        args = merge(ini_config, args)
>
> to:
>
>            # merge config/args, priority given to args
>            args = merge(args, ini_config)
>

Yes, that should be it.
I had indeed misinterpreted the original code priority.

Thanks,
Thomas

[Buildroot] [PATCH v5 04/11] autobuild-run: use **kwargs to avoid explicit parameter passthroughs

[Thomas De Schampheleire]

Hi Thomas,

On Fri, Feb 6, 2015 at 8:58 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Fri, 12 Dec 2014 21:04:49 +0100, Thomas De Schampheleire wrote:
>> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>>
>> The current version of autobuild-run has some extensive explicit
>> parameter passing, for example:
>>
>> - fixup_config needs sysinfo, passed via gen_config, in turn via
>>   run_instance, in turn via main.
>> - send_results needs username/password settings, passed via
>>   run_instance, in turn via main.
>>
>> Everytime a leaf function needs an extra parameter (for example coming
>> from the arguments or config file), the entire call chain needs to be
>> adapted to pass along that parameter.
>>
>> This patch introduces the **kwargs dictionary principle, that allows
>> implicit parameter passing. A function can accept this dictionary and
>> extract parameters from it by name. The dictionary can be passed as a
>> whole to a child function, without explicitly enumerating which entries
>> in the dictionary are needed in the child.
>
> I must say I find this **kwargs thing a bit weird. Yes indeed, it
> avoids passing several parameters to functions. But on the other hand,
> it means that you're passing the *entire* context to *all* functions.
> Which more or less boils down to using global variables. It means that
> it isn't clear looking at the prototype of a function, which parameters
> it is taking.
>
> Maybe it's just that I'm not doing enough Python, and I'm thinking too
> much C, but I find this **kwargs to not be such a great idea. But oh
> well, since this is Python, I guess I should convince myself to use the
> Python spirit :)
>

Maybe someone else with good Python knowledge can chime in and tell us
if this is a good way or not of handling things.
Even in C, a function that takes 4+ arguments is not a good idea.
Passing such things in a struct (or a dict in python) is more
appropriate, IMO.

Best regards,
Thomas

[Buildroot] [PATCH v5 10/11] autobuild-run: kill all children on SIGTERM

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Fri, 12 Dec 2014 21:04:55 +0100, Thomas De Schampheleire wrote:

> -from multiprocessing import Process
> +import multiprocessing

This change...

>      for i in range(0, int(args['--ninstances'])):
>          p = Process(target=run_instance, kwargs=dict(

Breaks this, it should be changed to multiprocessing.Process.

I've fixed that up locally for now, but it essentially means that your
patch series was never executed, as it basically cannot work at all :-/

Best regards,

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

[Buildroot] [PATCH v5 03/11] autobuild-run: add option --make-opts for custom Buildroot options

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Fri, 12 Dec 2014 21:04:48 +0100, Thomas De Schampheleire wrote:

> +    cmd = ["timeout", str(MAX_DURATION), "make", "O=%s" % outputdir,
> +            "-C", srcdir, "BR2_DL_DIR=%s" % dldir, "BR2_JLEVEL=%s" % njobs] \
> +          + make_opts.split()

This breaks badly when no make_opts are passed:

Traceback (most recent call last):
  File "/usr/lib/python2.6/multiprocessing/process.py", line 232, in _bootstrap
    self.run()
  File "/usr/lib/python2.6/multiprocessing/process.py", line 88, in run
    self._target(*self._args, **self._kwargs)
  File "./autobuild-run", line 660, in run_instance
    ret = do_build(**kwargs)
  File "./autobuild-run", line 471, in do_build
    + kwargs['make_opts'].split()
AttributeError: 'NoneType' object has no attribute 'split'

Best regards,

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

[Buildroot] [PATCH v5 02/11] autobuild-run: use docopt for argument parsing

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Fri, 12 Dec 2014 21:04:47 +0100, Thomas De Schampheleire wrote:

> +    if args['--config']:
> +        ini_config = load_ini_config(args['--config'])
> +        # merge config/args, priority given to config
> +        args = merge(ini_config, args)

As we discussed, I changed this to:

	args = merge(args, ini_config)

since the command line arguments should have priority over the values
defined in the configuration file, like any other Unix tool does.

However, this doesn't seem to work: since the docopt documentation says
that ninstances default value is 1 when no value is defined, I believe
'args' contains a value of 1 for ninstances, and it doesn't fall back
to using the value defined in the configuration file.

Do you see a way of fixing this?

Thanks,

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

[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Fri, 12 Dec 2014 21:04:45 +0100, Thomas De Schampheleire wrote:

> This series against the buildroot-test repo makes some improvements to the
> autobuild-run script.

I've finally taken some time to test this series, and I've reported two
separate problems as replies to e-mails: the make-opts split() issue,
and the merging of argument vs. configuration file values.

To avoid duplicate work, I've pushed the current state of your work
(with the fix to the merging order, and the multiprocessing.Process
fix) to:

   git://git.free-electrons.com/users/thomas-petazzoni/buildroot-test.git

in a branch called 'thomas-ds'.

Since both issues are more or less tied to the docopt migration, can
you have a look at them, and maybe simply propose followup patches that
I can merge back into the problematic commits, to fix those issues?

Thanks a lot,

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

[Buildroot] [PATCH v5 10/11] autobuild-run: kill all children on SIGTERM

[Thomas De Schampheleire]

Hi Thomas,

On Sat, Feb 28, 2015 at 8:28 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Fri, 12 Dec 2014 21:04:55 +0100, Thomas De Schampheleire wrote:
>
>> -from multiprocessing import Process
>> +import multiprocessing
>
> This change...
>
>>      for i in range(0, int(args['--ninstances'])):
>>          p = Process(target=run_instance, kwargs=dict(
>
> Breaks this, it should be changed to multiprocessing.Process.
>
> I've fixed that up locally for now, but it essentially means that your
> patch series was never executed, as it basically cannot work at all :-/

Naturally I did test this series, and this patch, but I must have made
a final 'fixup' before submitting without retesting. Shame on me,
sorry :-s

[Buildroot] [PATCH v5 03/11] autobuild-run: add option --make-opts for custom Buildroot options

[Thomas De Schampheleire]

On Sat, Feb 28, 2015 at 8:29 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Fri, 12 Dec 2014 21:04:48 +0100, Thomas De Schampheleire wrote:
>
>> +    cmd = ["timeout", str(MAX_DURATION), "make", "O=%s" % outputdir,
>> +            "-C", srcdir, "BR2_DL_DIR=%s" % dldir, "BR2_JLEVEL=%s" % njobs] \
>> +          + make_opts.split()
>
> This breaks badly when no make_opts are passed:
>
> Traceback (most recent call last):
>   File "/usr/lib/python2.6/multiprocessing/process.py", line 232, in _bootstrap
>     self.run()
>   File "/usr/lib/python2.6/multiprocessing/process.py", line 88, in run
>     self._target(*self._args, **self._kwargs)
>   File "./autobuild-run", line 660, in run_instance
>     ret = do_build(**kwargs)
>   File "./autobuild-run", line 471, in do_build
>     + kwargs['make_opts'].split()
> AttributeError: 'NoneType' object has no attribute 'split'
>

make-opts is given the empty string '' as default in the original code
I sent, and thus can be splitted without the above error. I guess you
are seeing this after making some other change?

[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[Thomas De Schampheleire]

Hi Thomas,

On Sat, Feb 28, 2015 at 8:34 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Fri, 12 Dec 2014 21:04:45 +0100, Thomas De Schampheleire wrote:
>
>> This series against the buildroot-test repo makes some improvements to the
>> autobuild-run script.
>
> I've finally taken some time to test this series, and I've reported two
> separate problems as replies to e-mails: the make-opts split() issue,
> and the merging of argument vs. configuration file values.
>
> To avoid duplicate work, I've pushed the current state of your work
> (with the fix to the merging order, and the multiprocessing.Process
> fix) to:
>
>    git://git.free-electrons.com/users/thomas-petazzoni/buildroot-test.git
>
> in a branch called 'thomas-ds'.
>
> Since both issues are more or less tied to the docopt migration, can
> you have a look at them, and maybe simply propose followup patches that
> I can merge back into the problematic commits, to fix those issues?

Thanks for looking into this.
As replied on both mails, the make-opts split() issue is AFAICS not
present in the original code, but the multiprocessing error was a big
mistake by me.

I have sent two follow-up patches based on your branch. The first one
solves the argument handling priority issue you mentioned, while the
second one is a forgotten change in one of the earlier patches, as I
had mentioned before.

Let me know if I can address other issues.

Best regards,
Thomas

[Buildroot] [PATCH v5 03/11] autobuild-run: add option --make-opts for custom Buildroot options

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Sun, 1 Mar 2015 21:42:34 +0100, Thomas De Schampheleire wrote:

> make-opts is given the empty string '' as default in the original code
> I sent, and thus can be splitted without the above error. I guess you
> are seeing this after making some other change?

Nope, I don't think I've done any change regarding this, unless I did
a mistake, of course. You can see the code as I tested it in the Git
repository I pointed at.

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

[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Fri, 12 Dec 2014 21:04:45 +0100, Thomas De Schampheleire wrote:

> Thomas De Schampheleire (11):
>   scripts: add python module docopt
>   autobuild-run: use docopt for argument parsing
>   autobuild-run: add option --make-opts for custom Buildroot options
>   autobuild-run: use **kwargs to avoid explicit parameter passthroughs
>   autobuild-run: check-requirements does not need to know the login
>     details
>   autobuild-run: set LC_ALL=C to not use locale settings of host machine
>   autobuild-run: improve the logic to generate build-end.log
>   autobuild-run: save config.log files for failed package
>   autobuild-run: extend TODO list
>   autobuild-run: kill all children on SIGTERM
>   autobuild-run: catch KeyboardInterrupt in the same way as SIGTERM

I've applied your patch series. A few comments, though:

 1/ I also applied your two follow-up patches
    http://patchwork.ozlabs.org/patch/444865/ and
    http://patchwork.ozlabs.org/patch/444866/. As requested in the
    commit log, the second patch was squashed into the patch adding the
    support for killing subprocesses.

 2/ I had to rework patch http://patchwork.ozlabs.org/patch/444866/
    because subprocess.check_output() doesn't exist in Python 2.6. So
    I've used a different construct.

 3/ Killing all subprocesses apparently still doesn't work completely
    correctly, I got the backtrace of the attached file once when
    interrupting the autobuild-run script with Ctrl+C, on a system that
    uses Python 2.6. And the child processes (timeout processes) were
    still running.

Thanks a lot, and sorry for the huge delay!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: backtrace.txt
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20150315/4339d329/attachment.txt>

[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[Thomas De Schampheleire]

Hi Thomas,

On Sun, Mar 15, 2015 at 2:34 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Fri, 12 Dec 2014 21:04:45 +0100, Thomas De Schampheleire wrote:
>
>> Thomas De Schampheleire (11):
>>   scripts: add python module docopt
>>   autobuild-run: use docopt for argument parsing
>>   autobuild-run: add option --make-opts for custom Buildroot options
>>   autobuild-run: use **kwargs to avoid explicit parameter passthroughs
>>   autobuild-run: check-requirements does not need to know the login
>>     details
>>   autobuild-run: set LC_ALL=C to not use locale settings of host machine
>>   autobuild-run: improve the logic to generate build-end.log
>>   autobuild-run: save config.log files for failed package
>>   autobuild-run: extend TODO list
>>   autobuild-run: kill all children on SIGTERM
>>   autobuild-run: catch KeyboardInterrupt in the same way as SIGTERM
>
> I've applied your patch series. A few comments, though:
>
>  1/ I also applied your two follow-up patches
>     http://patchwork.ozlabs.org/patch/444865/ and
>     http://patchwork.ozlabs.org/patch/444866/. As requested in the
>     commit log, the second patch was squashed into the patch adding the
>     support for killing subprocesses.
>
>  2/ I had to rework patch http://patchwork.ozlabs.org/patch/444866/
>     because subprocess.check_output() doesn't exist in Python 2.6. So
>     I've used a different construct.
>
>  3/ Killing all subprocesses apparently still doesn't work completely
>     correctly, I got the backtrace of the attached file once when
>     interrupting the autobuild-run script with Ctrl+C, on a system that
>     uses Python 2.6. And the child processes (timeout processes) were
>     still running.

To understand better, some questions:
- you saw this situation once, with Python 2.6, but have been able to
successfully stop autobuild run with Ctrl-C with Python 2.6 on other
occasions?

- did you happen to use Python 2.7 with these patches on a certain
machine? Did you see issues?

In my testing I have been using Python 2.7.
It could be that there is something different in Python 2.6, not sure.
I would need to look into this deeper, unless Andr? who seems to be
much more knowledgeable about Python has an idea?

Thanks,
Thomas

[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Sun, 15 Mar 2015 14:51:38 +0100, Thomas De Schampheleire wrote:

> To understand better, some questions:
> - you saw this situation once, with Python 2.6, but have been able to
> successfully stop autobuild run with Ctrl-C with Python 2.6 on other
> occasions?

I just tried again, and same behavior. Here are the processes that
remain:

22897 pts/2    S      0:00 timeout 14400 make O=/ssd1/thomas/autobuild/instance-0/output -C instance-0/buildroot BR2_DL_DIR=/ssd1/thomas/autobuild/instance-0/dl BR2_JLEVEL=4
22898 pts/2    S      0:15  \_ make O=/ssd1/thomas/autobuild/instance-0/output -C instance-0/buildroot BR2_DL_DIR=/ssd1/thomas/autobuild/instance-0/dl BR2_JLEVEL=4
15337 pts/2    S      0:00      \_ /usr/bin/make -j4 -C /ssd1/thomas/autobuild/instance-0/output/build/host-nodejs-0.10.36 PATH=/ssd1/thomas/autobuild/instance-0/output/host/bin:/ssd1/th
[...]
25240 pts/2    S      0:00 timeout 14400 make O=/ssd1/thomas/autobuild/instance-3/output -C instance-3/buildroot BR2_DL_DIR=/ssd1/thomas/autobuild/instance-3/dl BR2_JLEVEL=4
25242 pts/2    S      0:14  \_ make O=/ssd1/thomas/autobuild/instance-3/output -C instance-3/buildroot BR2_DL_DIR=/ssd1/thomas/autobuild/instance-3/dl BR2_JLEVEL=4
25512 pts/2    S      0:00      \_ /usr/bin/make -j4 -C /ssd1/thomas/autobuild/instance-3/output/build/samba4-4.2.0 DESTDIR=/ssd1/thomas/autobuild/instance-3/output/host/usr/i686-buildro
[...]
 2853 pts/2    S      0:00 timeout 14400 make O=/ssd1/thomas/autobuild/instance-2/output -C instance-2/buildroot BR2_DL_DIR=/ssd1/thomas/autobuild/instance-2/dl BR2_JLEVEL=4
 2854 pts/2    S      0:08  \_ make O=/ssd1/thomas/autobuild/instance-2/output -C instance-2/buildroot BR2_DL_DIR=/ssd1/thomas/autobuild/instance-2/dl BR2_JLEVEL=4
25772 pts/2    S      0:00      \_ /bin/bash -c cd /ssd1/thomas/autobuild/instance-2/output/build/host-heimdal-1.5.3/ &&  PATH="/ssd1/thomas/autobuild/instance-2/output/host/bin:/ssd1/th
[...]
 7028 pts/2    S      0:00 timeout 14400 make O=/ssd1/thomas/autobuild/instance-1/output -C instance-1/buildroot BR2_DL_DIR=/ssd1/thomas/autobuild/instance-1/dl BR2_JLEVEL=4
 7029 pts/2    S      0:06  \_ make O=/ssd1/thomas/autobuild/instance-1/output -C instance-1/buildroot BR2_DL_DIR=/ssd1/thomas/autobuild/instance-1/dl BR2_JLEVEL=4
19878 pts/2    S      0:00      \_ /bin/bash -c (cd /ssd1/thomas/autobuild/instance-1/output/build/host-cmake-3.1.3; LDFLAGS="-L/ssd1/thomas/autobuild/instance-1/output/host/lib -L/ssd1/

> - did you happen to use Python 2.7 with these patches on a certain
> machine? Did you see issues?

No, I have been testing them only on gcc75 for now, which uses Python 2.6.

I'm planning on deploying them on the Free Electrons build server as
well, but it also has Python 2.6.

> In my testing I have been using Python 2.7.
> It could be that there is something different in Python 2.6, not sure.
> I would need to look into this deeper, unless Andr? who seems to be
> much more knowledgeable about Python has an idea?

That would be nice :-)

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

[Buildroot] [PATCH v5 00/11] autobuild-run improvements

[André Erdmann]

Hi,

2015-03-15 15:10 GMT+01:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Dear Thomas De Schampheleire,
>
> On Sun, 15 Mar 2015 14:51:38 +0100, Thomas De Schampheleire wrote:
>
>> To understand better, some questions:
>> - you saw this situation once, with Python 2.6, but have been able to
>> successfully stop autobuild run with Ctrl-C with Python 2.6 on other
>> occasions?
>
> I just tried again, and same behavior. Here are the processes that
> remain:
>
> [...]
>
>> In my testing I have been using Python 2.7.
>> It could be that there is something different in Python 2.6, not sure.
>> I would need to look into this deeper, unless Andr? who seems to be
>> much more knowledgeable about Python has an idea?
>
> That would be nice :-)
>

I'll look into that - one thing to consider is that SIGINT of any form
gets also delivered to multiprocessing.Process instances, so putting a
"signal.signal(signal.SIGINT, signal.SIG_IGN)" before creating the
processes might help (line ~803; ugly workaround, actually). But I
doubt it's as easy as that..

-- 
Andr?