* [Buildroot] [PATCH buildroot-test 2/8] scripts: add python module docopt
2014-10-16 19:15 [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Thomas De Schampheleire
@ 2014-10-16 19:15 ` Thomas De Schampheleire
2014-10-16 19:15 ` [Buildroot] [PATCH buildroot-test 3/8] autobuild-run: use docopt for argument parsing Thomas De Schampheleire
` (6 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-16 19:15 UTC (permalink / raw)
To: buildroot
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.
The __init__.py file is added to allow treating docopt as a package (and
thus keep the files in a separate directory).
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
scripts/docopt/LICENSE-MIT | 23 ++
scripts/docopt/__init__.py | 0
scripts/docopt/docopt.py | 581 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 604 insertions(+)
create mode 100644 scripts/docopt/LICENSE-MIT
create mode 100644 scripts/docopt/__init__.py
create mode 100644 scripts/docopt/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/__init__.py b/scripts/docopt/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/scripts/docopt/docopt.py b/scripts/docopt/docopt.py
new file mode 100644
index 0000000..2e43f7c
--- /dev/null
+++ b/scripts/docopt/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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 3/8] autobuild-run: use docopt for argument parsing
2014-10-16 19:15 [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Thomas De Schampheleire
2014-10-16 19:15 ` [Buildroot] [PATCH buildroot-test 2/8] scripts: add python module docopt Thomas De Schampheleire
@ 2014-10-16 19:15 ` Thomas De Schampheleire
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 4/8] autobuild-run: convert regular function comments into docstrings Thomas De Schampheleire
` (5 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-16 19:15 UTC (permalink / raw)
To: buildroot
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 | 131 ++++++++++++++++++++++++++------------------------
1 file changed, 69 insertions(+), 62 deletions(-)
diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 2ead1f2..8c1918c 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
+from docopt import docopt
MAX_DURATION = 60 * 60 * 4
VERSION = 1
@@ -488,72 +520,45 @@ 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)
-# Function to get the configuration parameters, either from the
-# command line, or through a configuration file.
-def config_get():
- epilog_text = """
-Format of the configuration file:
+# args / config file merging inspired by:
+# https://github.com/docopt/docopt/blob/master/examples/config_file_example.py
+def load_ini_config(configfile):
+ if not os.path.exists(configfile):
+ print "ERROR: configuration file %s does not exist" % configfile
+ sys.exit(1)
- [main]
- ninstances = <value>
- njobs = <value>
- http-login = <value>
- http-password = <value>
- submitter = <value>
-"""
+ config = ConfigParser.RawConfigParser()
+ if not config.read(configfile):
+ print "ERROR: cannot parse configuration file %s" % configfile
+ sys.exit(1)
- 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)
+ # 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'))
+
+
+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))
if __name__ == '__main__':
check_version()
sysinfo = SystemInfo()
- (ninstances, njobs, http_login, http_password, submitter) = config_get()
- do_send_results = http_login and http_password
+
+ 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)
+
+ do_send_results = args['--http-login'] and args['--http-password']
check_requirements(do_send_results)
if not do_send_results:
print "WARN: due to the lack of http login/password details, results will not be submitted"
@@ -562,8 +567,10 @@ if __name__ == '__main__':
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 4/8] autobuild-run: convert regular function comments into docstrings
2014-10-16 19:15 [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Thomas De Schampheleire
2014-10-16 19:15 ` [Buildroot] [PATCH buildroot-test 2/8] scripts: add python module docopt Thomas De Schampheleire
2014-10-16 19:15 ` [Buildroot] [PATCH buildroot-test 3/8] autobuild-run: use docopt for argument parsing Thomas De Schampheleire
@ 2014-10-16 19:16 ` Thomas De Schampheleire
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 5/8] autobuild-run: add option --make-opts for custom Buildroot options Thomas De Schampheleire
` (4 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-16 19:16 UTC (permalink / raw)
To: buildroot
From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Documenting a function in Python is done with docstrings, rather than
with comments above the function. The conventions are described in
PEP257: http://legacy.python.org/dev/peps/pep-0257/
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
scripts/autobuild-run | 75 +++++++++++++++++++++++++++++++++------------------
1 file changed, 49 insertions(+), 26 deletions(-)
diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 8c1918c..170cfe3 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -156,13 +156,16 @@ class SystemInfo:
self.has_jar = \
subprocess.call(["which", "jar"], stdout=devnull, stderr=devnull) == 0
-# This function fetches the possible toolchain configurations, and
-# returns an array of dictionaries, with for each toolchain:
-# - url: the URL of the toolchain defconfig
-# - libc: the C library used by the toolchain
-# - hostarch: the host architecture for which the toolchain is built
-# - contents: an array of lines of the defconfig
def get_toolchain_configs():
+ """Fetch and return the possible toolchain configurations
+
+ This function returns an array of dictionaries, with for each toolchain:
+ - url: the URL of the toolchain defconfig
+ - libc: the C library used by the toolchain
+ - hostarch: the host architecture for which the toolchain is built
+ - contents: an array of lines of the defconfig
+ """
+
r = urllib2.urlopen('http://autobuild.buildroot.org/toolchains/configs/toolchain-configs.csv')
l = r.readlines()
configs = []
@@ -183,10 +186,14 @@ def get_toolchain_configs():
configs.append(config)
return configs
-# This function prepares the build by making sure all the needed
-# directories are created, cloning or updating the Buildroot source
-# code, and cleaning up remaining stuff from previous builds.
def prepare_build(instance, log):
+ """Prepare for the next build of the specified instance
+
+ This function prepares the build by making sure all the needed
+ directories are created, cloning or updating the Buildroot source
+ code, and cleaning up remaining stuff from previous builds.
+ """
+
idir = "instance-%d" % instance
log_write(log, "INFO: preparing a new build")
@@ -236,14 +243,15 @@ def prepare_build(instance, log):
return 0
-# This function makes adjustments to the configuration, as well as
-# additional validation to avoid cases that are known not to work.
-#
-# This function returns 'True' when the configuration has been
-# accepted, and 'False' when the configuration has not been accepted
-# (in which case another random configuration will be generated).
-
def fixup_config(instance, sysinfo):
+ """Finalize the configuration and reject any problematic combinations
+
+ This function returns 'True' when the configuration has been
+ accepted, and 'False' when the configuration has not been accepted because
+ it is known to fail (in which case another random configuration will be
+ generated).
+ """
+
idir = "instance-%d" % instance
outputdir = os.path.join(idir, "output")
@@ -322,10 +330,14 @@ def fixup_config(instance, sysinfo):
return True
-# This function generates the configuration, by choosing a random
-# toolchain configuration and then generating a random selection of
-# packages.
def gen_config(instance, log, sysinfo):
+ """Generate a new random configuration
+
+ This function generates the configuration, by choosing a random
+ toolchain configuration and then generating a random selection of
+ packages.
+ """
+
idir = "instance-%d" % instance
dldir = os.path.join(idir, "dl")
# We need the absolute path to use with O=, because the relative
@@ -394,8 +406,9 @@ def gen_config(instance, log, sysinfo):
return 0
-# Run the build itself
def do_build(instance, njobs, log):
+ """Run the build itself"""
+
idir = "instance-%d" % instance
# We need the absolute path to use with O=, because the relative
# path to the output directory here is not relative to the
@@ -423,10 +436,14 @@ def do_build(instance, njobs, log):
log_write(log, "INFO: build successful")
return 0
-# This function prepares the tarball with the results, and either
-# submits them to the official server (if the appropriate credentials
-# are available) or stores them locally as tarballs.
def send_results(instance, http_login, http_password, submitter, log, result):
+ """Prepare and store/send tarball with results
+
+ This function prepares the tarball with the results, and either
+ submits them to the official server (if the appropriate credentials
+ are available) or stores them locally as tarballs.
+ """
+
idir = "instance-%d" % instance
outputdir = os.path.abspath(os.path.join(idir, "output"))
srcdir = os.path.join(idir, "buildroot")
@@ -494,10 +511,13 @@ 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)
-# This function implements the main per-instance loop, which prepares
-# the build, generate a configuration, runs the build, and submits the
-# results.
def run_instance(instance, njobs, http_login, http_password, submitter, sysinfo):
+ """Main per-instance loop
+
+ Prepare the build, generate a configuration, run the build, and submit the
+ results.
+ """
+
idir = "instance-%d" % instance
# If it doesn't exist, create the instance directory
@@ -522,7 +542,10 @@ def run_instance(instance, njobs, http_login, http_password, submitter, sysinfo)
# args / config file merging inspired by:
# https://github.com/docopt/docopt/blob/master/examples/config_file_example.py
+
def load_ini_config(configfile):
+ """Load configuration from file, returning a docopt-like dictionary"""
+
if not os.path.exists(configfile):
print "ERROR: configuration file %s does not exist" % configfile
sys.exit(1)
--
1.8.5.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 5/8] autobuild-run: add option --make-opts for custom Buildroot options
2014-10-16 19:15 [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Thomas De Schampheleire
` (2 preceding siblings ...)
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 4/8] autobuild-run: convert regular function comments into docstrings Thomas De Schampheleire
@ 2014-10-16 19:16 ` Thomas De Schampheleire
2014-10-17 10:46 ` Thomas De Schampheleire
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 6/8] autobuild-run: create main method to locally-scope all variables Thomas De Schampheleire
` (3 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-16 19:16 UTC (permalink / raw)
To: buildroot
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,
direct git access (as required by some packages) may not be possible. A
git wrapper may be needed, which is normally configured through the
BR2_GIT configuration option.
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 git 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 | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 170cfe3..57654de 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -72,6 +72,8 @@ 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
-c, --config CONFIG path to configuration file
Format of the configuration file:
@@ -406,7 +408,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
@@ -419,8 +421,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:
@@ -511,7 +515,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
@@ -537,7 +542,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:
@@ -593,7 +598,7 @@ if __name__ == '__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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 5/8] autobuild-run: add option --make-opts for custom Buildroot options
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 5/8] autobuild-run: add option --make-opts for custom Buildroot options Thomas De Schampheleire
@ 2014-10-17 10:46 ` Thomas De Schampheleire
0 siblings, 0 replies; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-17 10:46 UTC (permalink / raw)
To: buildroot
On Thu, Oct 16, 2014 at 9:16 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> 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,
> direct git access (as required by some packages) may not be possible. A
> git wrapper may be needed, which is normally configured through the
> BR2_GIT configuration option.
>
> 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 git 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 | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 170cfe3..57654de 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -72,6 +72,8 @@ 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
> -c, --config CONFIG path to configuration file
>
> Format of the configuration file:
> @@ -406,7 +408,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
> @@ -419,8 +421,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()
This line currently fails if no make opts are provided, will fix.
Best regards,
Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 6/8] autobuild-run: create main method to locally-scope all variables
2014-10-16 19:15 [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Thomas De Schampheleire
` (3 preceding siblings ...)
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 5/8] autobuild-run: add option --make-opts for custom Buildroot options Thomas De Schampheleire
@ 2014-10-16 19:16 ` Thomas De Schampheleire
2014-10-17 22:24 ` Yann E. MORIN
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 7/8] autobuild-run: use **kwargs to avoid explicit parameter passthroughs Thomas De Schampheleire
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-16 19:16 UTC (permalink / raw)
To: buildroot
From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
In the current autobuild-run script, all variables created in the 'main'
part are in fact global, even though the script does not intend them to
be. For example, sysinfo is global, even though it is passed as function
argument to run_instance.
To avoid this accidental globalization, create a main method instead.
All variables created therein are now local to that method.
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
scripts/autobuild-run | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 57654de..95f2390 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -575,7 +575,7 @@ def merge(dict_1, dict_2):
return dict((str(key), dict_1.get(key) or dict_2.get(key))
for key in set(dict_2) | set(dict_1))
-if __name__ == '__main__':
+def main():
check_version()
sysinfo = SystemInfo()
@@ -604,3 +604,6 @@ if __name__ == '__main__':
signal.signal(signal.SIGTERM, sigterm_handler)
for p in processes:
p.join()
+
+if __name__ == '__main__':
+ main()
--
1.8.5.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 6/8] autobuild-run: create main method to locally-scope all variables
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 6/8] autobuild-run: create main method to locally-scope all variables Thomas De Schampheleire
@ 2014-10-17 22:24 ` Yann E. MORIN
2014-10-18 11:02 ` Maxime Hadjinlian
0 siblings, 1 reply; 23+ messages in thread
From: Yann E. MORIN @ 2014-10-17 22:24 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2014-10-16 21:16 +0200, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> In the current autobuild-run script, all variables created in the 'main'
> part are in fact global, even though the script does not intend them to
> be. For example, sysinfo is global, even though it is passed as function
> argument to run_instance.
>
> To avoid this accidental globalization, create a main method instead.
> All variables created therein are now local to that method.
>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
For what it's worth:
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
;-)
Regards,
Yann E. MORIN.
> ---
> scripts/autobuild-run | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 57654de..95f2390 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -575,7 +575,7 @@ def merge(dict_1, dict_2):
> return dict((str(key), dict_1.get(key) or dict_2.get(key))
> for key in set(dict_2) | set(dict_1))
>
> -if __name__ == '__main__':
> +def main():
> check_version()
> sysinfo = SystemInfo()
>
> @@ -604,3 +604,6 @@ if __name__ == '__main__':
> signal.signal(signal.SIGTERM, sigterm_handler)
> for p in processes:
> p.join()
> +
> +if __name__ == '__main__':
> + main()
> --
> 1.8.5.1
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 6/8] autobuild-run: create main method to locally-scope all variables
2014-10-17 22:24 ` Yann E. MORIN
@ 2014-10-18 11:02 ` Maxime Hadjinlian
0 siblings, 0 replies; 23+ messages in thread
From: Maxime Hadjinlian @ 2014-10-18 11:02 UTC (permalink / raw)
To: buildroot
Hi Yann, Thomas, all
Acked-by: "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>
On Sat, Oct 18, 2014 at 12:24 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Thomas, All,
>
> On 2014-10-16 21:16 +0200, Thomas De Schampheleire spake thusly:
>> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>>
>> In the current autobuild-run script, all variables created in the 'main'
>> part are in fact global, even though the script does not intend them to
>> be. For example, sysinfo is global, even though it is passed as function
>> argument to run_instance.
>>
>> To avoid this accidental globalization, create a main method instead.
>> All variables created therein are now local to that method.
>>
>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> For what it's worth:
>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> ;-)
>
> Regards,
> Yann E. MORIN.
>
>> ---
>> scripts/autobuild-run | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>> index 57654de..95f2390 100755
>> --- a/scripts/autobuild-run
>> +++ b/scripts/autobuild-run
>> @@ -575,7 +575,7 @@ def merge(dict_1, dict_2):
>> return dict((str(key), dict_1.get(key) or dict_2.get(key))
>> for key in set(dict_2) | set(dict_1))
>>
>> -if __name__ == '__main__':
>> +def main():
>> check_version()
>> sysinfo = SystemInfo()
>>
>> @@ -604,3 +604,6 @@ if __name__ == '__main__':
>> signal.signal(signal.SIGTERM, sigterm_handler)
>> for p in processes:
>> p.join()
>> +
>> +if __name__ == '__main__':
>> + main()
>> --
>> 1.8.5.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 7/8] autobuild-run: use **kwargs to avoid explicit parameter passthroughs
2014-10-16 19:15 [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Thomas De Schampheleire
` (4 preceding siblings ...)
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 6/8] autobuild-run: create main method to locally-scope all variables Thomas De Schampheleire
@ 2014-10-16 19:16 ` Thomas De Schampheleire
2014-10-17 11:27 ` Thomas De Schampheleire
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 8/8] autobuild-run: add support for custom git wrapper Thomas De Schampheleire
2014-10-17 22:20 ` [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Yann E. MORIN
7 siblings, 1 reply; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-16 19:16 UTC (permalink / raw)
To: buildroot
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>
---
scripts/autobuild-run | 71 ++++++++++++++++++++++++++++++---------------------
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 95f2390..ceed0ae 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -188,7 +188,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
@@ -196,7 +196,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")
@@ -245,7 +246,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
@@ -254,9 +255,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()
@@ -332,7 +334,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
@@ -340,7 +342,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
@@ -391,7 +395,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" % \
@@ -408,10 +412,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
@@ -422,8 +428,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
@@ -440,7 +447,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
@@ -448,7 +455,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")
@@ -483,7 +492,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.
@@ -493,11 +502,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",
@@ -515,35 +525,36 @@ 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,
- 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
@@ -596,9 +607,11 @@ 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={
+ '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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 7/8] autobuild-run: use **kwargs to avoid explicit parameter passthroughs
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 7/8] autobuild-run: use **kwargs to avoid explicit parameter passthroughs Thomas De Schampheleire
@ 2014-10-17 11:27 ` Thomas De Schampheleire
0 siblings, 0 replies; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-17 11:27 UTC (permalink / raw)
To: buildroot
On Thu, Oct 16, 2014 at 9:16 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> 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.
>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> ---
> scripts/autobuild-run | 71 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 95f2390..ceed0ae 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -188,7 +188,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
> @@ -196,7 +196,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")
>
> @@ -245,7 +246,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
> @@ -254,9 +255,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()
>
> @@ -332,7 +334,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
> @@ -340,7 +342,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
> @@ -391,7 +395,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" % \
> @@ -408,10 +412,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
> @@ -422,8 +428,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
> @@ -440,7 +447,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
> @@ -448,7 +455,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")
> @@ -483,7 +492,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.
> @@ -493,11 +502,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",
> @@ -515,35 +525,36 @@ 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,
> - 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
> @@ -596,9 +607,11 @@ 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={
> + '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
In this patch, one occurrence of 'instance' is not correctly
substituted with kwargs['instance']. I will fix this.
Clearly, some longer testing is needed with this patch set, but
getting your feedback on the principles already is good.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 8/8] autobuild-run: add support for custom git wrapper
2014-10-16 19:15 [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Thomas De Schampheleire
` (5 preceding siblings ...)
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 7/8] autobuild-run: use **kwargs to avoid explicit parameter passthroughs Thomas De Schampheleire
@ 2014-10-16 19:16 ` Thomas De Schampheleire
2014-10-17 22:30 ` Yann E. MORIN
2014-10-17 22:20 ` [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Yann E. MORIN
7 siblings, 1 reply; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-16 19:16 UTC (permalink / raw)
To: buildroot
From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
In corporate environments, there may be no direct git access, and a
wrapper program that sets a proxy may be needed. See [1] for an example
solution.
This patch to autobuild-run provides a --git argument that can be used
to set the path to such a git wrapper. By default, the argument is plain
'git'.
[1] http://gitolite.com/git-over-proxy.html
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
scripts/autobuild-run | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index ceed0ae..42441d0 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -74,6 +74,9 @@ Options:
only)
--make-opts OPTSTRING string of extra options to pass to Buildroot
make, such as specific command wrappers
+ --git GITCMD path to git program (possibly a wrapper) used
+ to clone the Buildroot repository
+ [default: git]
-c, --config CONFIG path to configuration file
Format of the configuration file:
@@ -224,7 +227,7 @@ def prepare_build(**kwargs):
# didn't exist already.
srcdir = os.path.join(idir, "buildroot")
if not os.path.exists(srcdir):
- ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
+ ret = subprocess.call([kwargs['git'], "clone", "git://git.busybox.net/buildroot", srcdir],
stdout=log, stderr=log)
if ret != 0:
log_write(log, "ERROR: could not clone Buildroot sources")
@@ -232,7 +235,7 @@ def prepare_build(**kwargs):
# Update the Buildroot sources.
abssrcdir = os.path.abspath(srcdir)
- ret = subprocess.call(["git", "pull"], cwd=srcdir, stdout=log, stderr=log)
+ ret = subprocess.call([kwargs['git'], "pull"], cwd=srcdir, stdout=log, stderr=log)
if ret != 0:
log_write(log, "ERROR: could not pull Buildroot sources")
return -1
@@ -475,8 +478,8 @@ def send_results(result, **kwargs):
shutil.copyfile(os.path.join(outputdir, "legal-info", "manifest.csv"),
os.path.join(resultdir, "licenses-manifest.csv"))
- subprocess.call(["git log master -n 1 --pretty=format:%%H > %s" % \
- os.path.join(resultdir, "gitid")],
+ subprocess.call(["%s log master -n 1 --pretty=format:%%H > %s" % \
+ (kwargs['git'], 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"))],
@@ -610,7 +613,7 @@ def main():
p = Process(target=run_instance, kwargs={
'instance': i, 'njobs': args['--njobs'], 'sysinfo': sysinfo,
'http_login': args['--http-login'],
- 'http_password': args['--http-password'],
+ 'http_password': args['--http-password'], 'git': args['--git'],
'submitter': args['--submitter'], 'make_opts': args['--make-opts']})
p.start()
processes.append(p)
--
1.8.5.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 8/8] autobuild-run: add support for custom git wrapper
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 8/8] autobuild-run: add support for custom git wrapper Thomas De Schampheleire
@ 2014-10-17 22:30 ` Yann E. MORIN
2014-10-18 13:00 ` Thomas Petazzoni
0 siblings, 1 reply; 23+ messages in thread
From: Yann E. MORIN @ 2014-10-17 22:30 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2014-10-16 21:16 +0200, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> In corporate environments, there may be no direct git access, and a
> wrapper program that sets a proxy may be needed. See [1] for an example
> solution.
>
> This patch to autobuild-run provides a --git argument that can be used
> to set the path to such a git wrapper. By default, the argument is plain
> 'git'.
>
> [1] http://gitolite.com/git-over-proxy.html
You know you can make this stick, right?
git config --global --add core.gitproxy /path/to/git-proxy-command
And then you're done once and for all. ;-)
Regards,
Yann E. MORIN.
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> ---
> scripts/autobuild-run | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index ceed0ae..42441d0 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -74,6 +74,9 @@ Options:
> only)
> --make-opts OPTSTRING string of extra options to pass to Buildroot
> make, such as specific command wrappers
> + --git GITCMD path to git program (possibly a wrapper) used
> + to clone the Buildroot repository
> + [default: git]
> -c, --config CONFIG path to configuration file
>
> Format of the configuration file:
> @@ -224,7 +227,7 @@ def prepare_build(**kwargs):
> # didn't exist already.
> srcdir = os.path.join(idir, "buildroot")
> if not os.path.exists(srcdir):
> - ret = subprocess.call(["git", "clone", "git://git.busybox.net/buildroot", srcdir],
> + ret = subprocess.call([kwargs['git'], "clone", "git://git.busybox.net/buildroot", srcdir],
> stdout=log, stderr=log)
> if ret != 0:
> log_write(log, "ERROR: could not clone Buildroot sources")
> @@ -232,7 +235,7 @@ def prepare_build(**kwargs):
>
> # Update the Buildroot sources.
> abssrcdir = os.path.abspath(srcdir)
> - ret = subprocess.call(["git", "pull"], cwd=srcdir, stdout=log, stderr=log)
> + ret = subprocess.call([kwargs['git'], "pull"], cwd=srcdir, stdout=log, stderr=log)
> if ret != 0:
> log_write(log, "ERROR: could not pull Buildroot sources")
> return -1
> @@ -475,8 +478,8 @@ def send_results(result, **kwargs):
> shutil.copyfile(os.path.join(outputdir, "legal-info", "manifest.csv"),
> os.path.join(resultdir, "licenses-manifest.csv"))
>
> - subprocess.call(["git log master -n 1 --pretty=format:%%H > %s" % \
> - os.path.join(resultdir, "gitid")],
> + subprocess.call(["%s log master -n 1 --pretty=format:%%H > %s" % \
> + (kwargs['git'], 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"))],
> @@ -610,7 +613,7 @@ def main():
> p = Process(target=run_instance, kwargs={
> 'instance': i, 'njobs': args['--njobs'], 'sysinfo': sysinfo,
> 'http_login': args['--http-login'],
> - 'http_password': args['--http-password'],
> + 'http_password': args['--http-password'], 'git': args['--git'],
> 'submitter': args['--submitter'], 'make_opts': args['--make-opts']})
> p.start()
> processes.append(p)
> --
> 1.8.5.1
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 8/8] autobuild-run: add support for custom git wrapper
2014-10-17 22:30 ` Yann E. MORIN
@ 2014-10-18 13:00 ` Thomas Petazzoni
2014-10-18 19:08 ` Thomas De Schampheleire
0 siblings, 1 reply; 23+ messages in thread
From: Thomas Petazzoni @ 2014-10-18 13:00 UTC (permalink / raw)
To: buildroot
Dear Yann E. MORIN,
On Sat, 18 Oct 2014 00:30:23 +0200, Yann E. MORIN wrote:
> You know you can make this stick, right?
>
> git config --global --add core.gitproxy /path/to/git-proxy-command
>
> And then you're done once and for all. ;-)
Yes, I was also wondering why a change to each and every git invocation
was needed: surely like wget there should be a way to configure git to
use a proxy.
With what Yann has given, Thomas, do you still need this patch?
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 8/8] autobuild-run: add support for custom git wrapper
2014-10-18 13:00 ` Thomas Petazzoni
@ 2014-10-18 19:08 ` Thomas De Schampheleire
0 siblings, 0 replies; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-18 19:08 UTC (permalink / raw)
To: buildroot
Hi,
On Sat, Oct 18, 2014 at 3:00 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Yann E. MORIN,
>
> On Sat, 18 Oct 2014 00:30:23 +0200, Yann E. MORIN wrote:
>
>> You know you can make this stick, right?
>>
>> git config --global --add core.gitproxy /path/to/git-proxy-command
>>
>> And then you're done once and for all. ;-)
>
> Yes, I was also wondering why a change to each and every git invocation
> was needed: surely like wget there should be a way to configure git to
> use a proxy.
>
> With what Yann has given, Thomas, do you still need this patch?
>
Hmm, yes indeed.
On my laptop, I don't want to do that as I need/don't need the proxy
depending on if I'm connected to the company network or not. But in
case of an autobuilder it's not the case.
So yes, I will drop this patch.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details
2014-10-16 19:15 [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Thomas De Schampheleire
` (6 preceding siblings ...)
2014-10-16 19:16 ` [Buildroot] [PATCH buildroot-test 8/8] autobuild-run: add support for custom git wrapper Thomas De Schampheleire
@ 2014-10-17 22:20 ` Yann E. MORIN
2014-10-18 10:15 ` Maxime Hadjinlian
2014-10-18 13:38 ` Arnout Vandecappelle
7 siblings, 2 replies; 23+ messages in thread
From: Yann E. MORIN @ 2014-10-17 22:20 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2014-10-16 21:15 +0200, Thomas De Schampheleire spake thusly:
> 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 do_send_results to hide these
> details.
>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> ---
> scripts/autobuild-run | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 7497001..2ead1f2 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -85,12 +85,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(do_send_results=False):
> devnull = open(os.devnull, "w")
> needed_progs = ["make", "git", "gcc", "timeout"]
> missing_requirements = False
>
> - if http_login and http_password:
> + if do_send_results:
> needed_progs.append("curl")
>
> for prog in needed_progs:
> @@ -553,8 +553,9 @@ if __name__ == '__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:
> + do_send_results = http_login and http_password
I was told that we should no treat 'None' as 'False', or a non-empty
string as 'True'. This should be something like:
do_send_results = (not http_login is None) and (not http_password is None)
Regards,
Yann E. MORIN.
> + check_requirements(do_send_results)
> + if not do_send_results:
> 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):
> --
> 1.8.5.1
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details
2014-10-17 22:20 ` [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Yann E. MORIN
@ 2014-10-18 10:15 ` Maxime Hadjinlian
2014-10-18 10:25 ` Yann E. MORIN
2014-10-18 13:38 ` Arnout Vandecappelle
1 sibling, 1 reply; 23+ messages in thread
From: Maxime Hadjinlian @ 2014-10-18 10:15 UTC (permalink / raw)
To: buildroot
Hi Yann, Thomas, All
On Sat, Oct 18, 2014 at 12:20 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Thomas, All,
>
> On 2014-10-16 21:15 +0200, Thomas De Schampheleire spake thusly:
>> 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 do_send_results to hide these
>> details.
>>
>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>> ---
>> scripts/autobuild-run | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>> index 7497001..2ead1f2 100755
>> --- a/scripts/autobuild-run
>> +++ b/scripts/autobuild-run
>> @@ -85,12 +85,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(do_send_results=False):
>> devnull = open(os.devnull, "w")
>> needed_progs = ["make", "git", "gcc", "timeout"]
>> missing_requirements = False
>>
>> - if http_login and http_password:
>> + if do_send_results:
>> needed_progs.append("curl")
>>
>> for prog in needed_progs:
>> @@ -553,8 +553,9 @@ if __name__ == '__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:
>> + do_send_results = http_login and http_password
>
> I was told that we should no treat 'None' as 'False', or a non-empty
> string as 'True'. This should be something like:'
You can treat 'None' as 'False' and also a non empty string will value
a logical 'True'.
In a python interpreter, do bool(None) and bool('blabla'), you'll
respectively get 'False' and 'True'.
I hope I am not the one you told you that in the first place...
>
> do_send_results = (not http_login is None) and (not http_password is None)
>
> Regards,
> Yann E. MORIN.
>
>> + check_requirements(do_send_results)
>> + if not do_send_results:
>> 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):
>> --
>> 1.8.5.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details
2014-10-18 10:15 ` Maxime Hadjinlian
@ 2014-10-18 10:25 ` Yann E. MORIN
2014-10-18 10:38 ` Maxime Hadjinlian
0 siblings, 1 reply; 23+ messages in thread
From: Yann E. MORIN @ 2014-10-18 10:25 UTC (permalink / raw)
To: buildroot
Maxime, All,
On 2014-10-18 12:15 +0200, Maxime Hadjinlian spake thusly:
> On Sat, Oct 18, 2014 at 12:20 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > Thomas, All,
> >
> > On 2014-10-16 21:15 +0200, Thomas De Schampheleire spake thusly:
> >> 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 do_send_results to hide these
> >> details.
> >>
> >> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> >> ---
> >> scripts/autobuild-run | 9 +++++----
> >> 1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> >> index 7497001..2ead1f2 100755
> >> --- a/scripts/autobuild-run
> >> +++ b/scripts/autobuild-run
> >> @@ -85,12 +85,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(do_send_results=False):
> >> devnull = open(os.devnull, "w")
> >> needed_progs = ["make", "git", "gcc", "timeout"]
> >> missing_requirements = False
> >>
> >> - if http_login and http_password:
> >> + if do_send_results:
> >> needed_progs.append("curl")
> >>
> >> for prog in needed_progs:
> >> @@ -553,8 +553,9 @@ if __name__ == '__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:
> >> + do_send_results = http_login and http_password
> >
> > I was told that we should no treat 'None' as 'False', or a non-empty
> > string as 'True'. This should be something like:'
> You can treat 'None' as 'False' and also a non empty string will value
> a logical 'True'.
> In a python interpreter, do bool(None) and bool('blabla'), you'll
> respectively get 'False' and 'True'.
>
> I hope I am not the one you told you that in the first place...
Not sure it was Samuel or you, but I just looked at PEP8:
http://legacy.python.org/dev/peps/pep-0008/#programming-recommendations
Comparisons to singletons like None should always be done with is or
is not, never the equality operators.
Also, beware of writing if x when you really mean if x is not None --
e.g. when testing whether a variable or argument that defaults to None
was set to some other value. The other value might have a type (such as
a container) that could be false in a boolean context!
Regards,
Yann E. MORIN.
> > do_send_results = (not http_login is None) and (not http_password is None)
> >
> > Regards,
> > Yann E. MORIN.
> >
> >> + check_requirements(do_send_results)
> >> + if not do_send_results:
> >> 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):
> >> --
> >> 1.8.5.1
> >>
> >> _______________________________________________
> >> buildroot mailing list
> >> buildroot at busybox.net
> >> http://lists.busybox.net/mailman/listinfo/buildroot
> >
> > --
> > .-----------------.--------------------.------------------.--------------------.
> > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> > | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
> > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> > '------------------------------^-------^------------------^--------------------'
> > _______________________________________________
> > buildroot mailing list
> > buildroot at busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 23+ messages in thread* [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details
2014-10-18 10:25 ` Yann E. MORIN
@ 2014-10-18 10:38 ` Maxime Hadjinlian
0 siblings, 0 replies; 23+ messages in thread
From: Maxime Hadjinlian @ 2014-10-18 10:38 UTC (permalink / raw)
To: buildroot
On Sat, Oct 18, 2014 at 12:25 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Maxime, All,
>
> On 2014-10-18 12:15 +0200, Maxime Hadjinlian spake thusly:
>> On Sat, Oct 18, 2014 at 12:20 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> > Thomas, All,
>> >
>> > On 2014-10-16 21:15 +0200, Thomas De Schampheleire spake thusly:
>> >> 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 do_send_results to hide these
>> >> details.
>> >>
>> >> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>> >> ---
>> >> scripts/autobuild-run | 9 +++++----
>> >> 1 file changed, 5 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
>> >> index 7497001..2ead1f2 100755
>> >> --- a/scripts/autobuild-run
>> >> +++ b/scripts/autobuild-run
>> >> @@ -85,12 +85,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(do_send_results=False):
>> >> devnull = open(os.devnull, "w")
>> >> needed_progs = ["make", "git", "gcc", "timeout"]
>> >> missing_requirements = False
>> >>
>> >> - if http_login and http_password:
>> >> + if do_send_results:
>> >> needed_progs.append("curl")
>> >>
>> >> for prog in needed_progs:
>> >> @@ -553,8 +553,9 @@ if __name__ == '__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:
>> >> + do_send_results = http_login and http_password
>> >
>> > I was told that we should no treat 'None' as 'False', or a non-empty
>> > string as 'True'. This should be something like:'
>> You can treat 'None' as 'False' and also a non empty string will value
>> a logical 'True'.
>> In a python interpreter, do bool(None) and bool('blabla'), you'll
>> respectively get 'False' and 'True'.
>>
>> I hope I am not the one you told you that in the first place...
>
> Not sure it was Samuel or you, but I just looked at PEP8:
>
> http://legacy.python.org/dev/peps/pep-0008/#programming-recommendations
>
> Comparisons to singletons like None should always be done with is or
> is not, never the equality operators.
>
> Also, beware of writing if x when you really mean if x is not None --
> e.g. when testing whether a variable or argument that defaults to None
> was set to some other value. The other value might have a type (such as
> a container) that could be false in a boolean context!
>
Here it's fine since we only deal with basic type.
> Regards,
> Yann E. MORIN.
>
>> > do_send_results = (not http_login is None) and (not http_password is None)
>> >
>> > Regards,
>> > Yann E. MORIN.
>> >
>> >> + check_requirements(do_send_results)
>> >> + if not do_send_results:
>> >> 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):
>> >> --
>> >> 1.8.5.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details
2014-10-17 22:20 ` [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details Yann E. MORIN
2014-10-18 10:15 ` Maxime Hadjinlian
@ 2014-10-18 13:38 ` Arnout Vandecappelle
2014-10-18 13:52 ` Maxime Hadjinlian
1 sibling, 1 reply; 23+ messages in thread
From: Arnout Vandecappelle @ 2014-10-18 13:38 UTC (permalink / raw)
To: buildroot
On 18/10/14 00:20, Yann E. MORIN wrote:
> Thomas, All,
>
> On 2014-10-16 21:15 +0200, Thomas De Schampheleire spake thusly:
[snip]
>> @@ -553,8 +553,9 @@ if __name__ == '__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:
>> + do_send_results = http_login and http_password
>
> I was told that we should no treat 'None' as 'False', or a non-empty
> string as 'True'. This should be something like:
>
> do_send_results = (not http_login is None) and (not http_password is None)
I'm with Yann (not Maxime) on this one: if you would have an autobuild server
that doesn't need a username or password, then there would be no distinction
between sending or not sending results. with 'http_login is not None and
http_password is not None' you can set them to the empty string to force sending.
In practice, of course, it doesn't matter because we have only one autobuild
server and it does require a non-empty username and password.
Regards,
Arnout
>
> Regards,
> Yann E. MORIN.
>
>> + check_requirements(do_send_results)
>> + if not do_send_results:
>> 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):
>> --
>> 1.8.5.1
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details
2014-10-18 13:38 ` Arnout Vandecappelle
@ 2014-10-18 13:52 ` Maxime Hadjinlian
2014-10-18 17:34 ` Yann E. MORIN
0 siblings, 1 reply; 23+ messages in thread
From: Maxime Hadjinlian @ 2014-10-18 13:52 UTC (permalink / raw)
To: buildroot
On Sat, Oct 18, 2014 at 3:38 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 18/10/14 00:20, Yann E. MORIN wrote:
>> Thomas, All,
>>
>> On 2014-10-16 21:15 +0200, Thomas De Schampheleire spake thusly:
> [snip]
>>> @@ -553,8 +553,9 @@ if __name__ == '__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:
>>> + do_send_results = http_login and http_password
>>
>> I was told that we should no treat 'None' as 'False', or a non-empty
>> string as 'True'. This should be something like:
>>
>> do_send_results = (not http_login is None) and (not http_password is None)
Just for the sake of it:
do_send_results = (http_login is not None) and (http_password is not None)
>
> I'm with Yann (not Maxime) on this one: if you would have an autobuild server
> that doesn't need a username or password, then there would be no distinction
> between sending or not sending results. with 'http_login is not None and
> http_password is not None' you can set them to the empty string to force sending.
Very true, I did not think about that case.
And as bonus point, like Yann said previously, it's more explicit.
>
> In practice, of course, it doesn't matter because we have only one autobuild
> server and it does require a non-empty username and password.
>
>
> Regards,
> Arnout
>
>>
>> Regards,
>> Yann E. MORIN.
>>
>>> + check_requirements(do_send_results)
>>> + if not do_send_results:
>>> 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):
>>> --
>>> 1.8.5.1
>>>
>>> _______________________________________________
>>> buildroot mailing list
>>> buildroot at busybox.net
>>> http://lists.busybox.net/mailman/listinfo/buildroot
>>
>
>
> --
> Arnout Vandecappelle arnout at mind be
> Senior Embedded Software Architect +32-16-286500
> Essensium/Mind http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details
2014-10-18 13:52 ` Maxime Hadjinlian
@ 2014-10-18 17:34 ` Yann E. MORIN
2014-10-18 19:06 ` Thomas De Schampheleire
0 siblings, 1 reply; 23+ messages in thread
From: Yann E. MORIN @ 2014-10-18 17:34 UTC (permalink / raw)
To: buildroot
All,
On 2014-10-18 15:52 +0200, Maxime Hadjinlian spake thusly:
> On Sat, Oct 18, 2014 at 3:38 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> > On 18/10/14 00:20, Yann E. MORIN wrote:
> >> Thomas, All,
> >>
> >> On 2014-10-16 21:15 +0200, Thomas De Schampheleire spake thusly:
> > [snip]
> >>> @@ -553,8 +553,9 @@ if __name__ == '__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:
> >>> + do_send_results = http_login and http_password
> >>
> >> I was told that we should no treat 'None' as 'False', or a non-empty
> >> string as 'True'. This should be something like:
> >>
> >> do_send_results = (not http_login is None) and (not http_password is None)
> Just for the sake of it:
> do_send_results = (http_login is not None) and (http_password is not None)
Yup, I forgot that '... is not ...' is preferred over 'not ... is ...'
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Buildroot] [PATCH buildroot-test 1/8] autobuild-run: check-requirements does not need to know the login details
2014-10-18 17:34 ` Yann E. MORIN
@ 2014-10-18 19:06 ` Thomas De Schampheleire
0 siblings, 0 replies; 23+ messages in thread
From: Thomas De Schampheleire @ 2014-10-18 19:06 UTC (permalink / raw)
To: buildroot
[sorry, sent from non-subscribed account first, will bounce on the list]
All,
On Sat, Oct 18, 2014 at 7:34 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> All,
>
> On 2014-10-18 15:52 +0200, Maxime Hadjinlian spake thusly:
>> On Sat, Oct 18, 2014 at 3:38 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> > On 18/10/14 00:20, Yann E. MORIN wrote:
>> >> Thomas, All,
>> >>
>> >> On 2014-10-16 21:15 +0200, Thomas De Schampheleire spake thusly:
>> > [snip]
>> >>> @@ -553,8 +553,9 @@ if __name__ == '__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:
>> >>> + do_send_results = http_login and http_password
>> >>
>> >> I was told that we should no treat 'None' as 'False', or a non-empty
>> >> string as 'True'. This should be something like:
>> >>
>> >> do_send_results = (not http_login is None) and (not http_password is None)
>> Just for the sake of it:
>> do_send_results = (http_login is not None) and (http_password is not None)
>
> Yup, I forgot that '... is not ...' is preferred over 'not ... is ...'
>
Extensive debate on just one line :)
First of all, the construct 'http_login and http_password' was already
in the original code, I just placed the result in a variable to allow
for reuse.
Secondly, regarding the quote Yann made of PEP8:
Comparisons to singletons like None should always be done with is or
is not, never the equality operators.
--> This is not relevant in this case, according to me, since we are
not using equality operators (==, !=).
Also, beware of writing if x when you really mean if x is not None --
e.g. when testing whether a variable or argument that defaults to None
was set to some other value. The other value might have a type (such as
a container) that could be false in a boolean context!
--> This part can be discussed. Arnout suggested that there may be a
hypothetical server that accepts empty http logins or password. Yes,
if you want to allow that, you cannot simply convert http_login in a
boolean as the empty string would render False, but it's not a case I
was coding for.
If I see code like:
do_send_results = (http_login is not None) and (http_password is not None)
my first reaction would be to change it into
do_send_results = http_login and http_password
although indeed it's not exactly the same. So a comment should be
written to make it explicit that the empty string is accepted as a
valid login/password.
All in all, I wonder if the empty string as login/password is a valid
use case to consider, and whether the uglier code is worth that.
But I'm not a difficult person, so if the consensus is to change that
line, no problem!
Best regards,
Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread