[U-Boot] [PATCH v3 0/7] tools/genboardscfg.py: various fixes and performance improvement

Now this series can apply on u-boot/master (commit 055626ac) without any prerequisites.
Masahiro Yamada (7): tools/genboardscfg.py: ignore defconfigs starting with a dot tools/genboardscfg.py: be tolerant of missing MAINTAINERS tools/genboardscfg.py: be tolerant of insane Kconfig tools/genboardscfg.py: wait for unfinished subprocesses before error-out tools/genboardscfg.py: fix minor problems on termination tools/genboardscfg.py: check if the boards.cfg is up to date tools/genboardscfg.py: improve performance
tools/genboardscfg.py | 275 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 201 insertions(+), 74 deletions(-)

Kconfig in U-Boot creates a temporary file configs/.tmp_defconfig during processing "make <board>_defconfig". The temporary file might be left over for some reasons.
Just in case, tools/genboardscfg.py should make sure to not read such garbage files.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: None
tools/genboardscfg.py | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py index e92e4f8..e13c8e4 100755 --- a/tools/genboardscfg.py +++ b/tools/genboardscfg.py @@ -411,6 +411,8 @@ def __gen_boards_cfg(jobs): for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR): dirpath = dirpath[len(CONFIG_DIR) + 1:] for filename in fnmatch.filter(filenames, '*_defconfig'): + if fnmatch.fnmatch(filename, '.*'): + continue defconfigs.append(os.path.join(dirpath, filename))
# Parse all the MAINTAINERS files

On Mon, Aug 25, 2014 at 12:39:42PM +0900, Masahiro Yamada wrote:
Kconfig in U-Boot creates a temporary file configs/.tmp_defconfig during processing "make <board>_defconfig". The temporary file might be left over for some reasons.
Just in case, tools/genboardscfg.py should make sure to not read such garbage files.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

tools/genboardscfg.py expects all the boards have MAINTAINERS. If someone adds a new board but misses to add its MAINTAINERS file, tools/genboardscfg.py fails to generate the boards.cfg file. It is annoying for the other developers.
This commit allows tools/genboardscfg.py to display warning messages and continue processing even if some MAINTAINERS files are missing or have broken formats.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: None
tools/genboardscfg.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py index e13c8e4..2ff2dbd 100755 --- a/tools/genboardscfg.py +++ b/tools/genboardscfg.py @@ -100,13 +100,19 @@ class MaintainersDatabase: Returns: Either 'Active' or 'Orphan' """ + if not target in self.database: + print >> sys.stderr, "WARNING: no status info for '%s'" % target + return '-' + tmp = self.database[target][0] if tmp.startswith('Maintained'): return 'Active' elif tmp.startswith('Orphan'): return 'Orphan' else: - print >> sys.stderr, 'Error: %s: unknown status' % tmp + print >> sys.stderr, ("WARNING: %s: unknown status for '%s'" % + (tmp, target)) + return '-'
def get_maintainers(self, target): """Return the maintainers of the given board. @@ -114,6 +120,10 @@ class MaintainersDatabase: If the board has two or more maintainers, they are separated with colons. """ + if not target in self.database: + print >> sys.stderr, "WARNING: no maintainers for '%s'" % target + return '' + return ':'.join(self.database[target][1])
def parse_file(self, file):

On Mon, Aug 25, 2014 at 12:39:43PM +0900, Masahiro Yamada wrote:
tools/genboardscfg.py expects all the boards have MAINTAINERS. If someone adds a new board but misses to add its MAINTAINERS file, tools/genboardscfg.py fails to generate the boards.cfg file. It is annoying for the other developers.
This commit allows tools/genboardscfg.py to display warning messages and continue processing even if some MAINTAINERS files are missing or have broken formats.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

The tools/genboardscfg.py expects all the Kconfig and defconfig are written correctly. Imagine someone accidentally has broken a board. Error-out just for one broken board is annoying for the other developers. Let the tool skip insane boards and continue processing.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: None
tools/genboardscfg.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py index 2ff2dbd..722b316 100755 --- a/tools/genboardscfg.py +++ b/tools/genboardscfg.py @@ -215,7 +215,10 @@ class DotConfigParser: # sanity check of '.config' file for field in self.must_fields: if not field in fields: - sys.exit('Error: %s is not defined in %s' % (field, defconfig)) + print >> sys.stderr, ( + "WARNING: '%s' is not defined in '%s'. Skip." % + (field, defconfig)) + return
# fix-up for aarch64 if fields['arch'] == 'arm' and 'cpu' in fields: @@ -307,7 +310,11 @@ class Slot: return True if self.ps.poll() == None: return False - self.parser.parse(self.defconfig) + if self.ps.poll() == 0: + self.parser.parse(self.defconfig) + else: + print >> sys.stderr, ("WARNING: failed to process '%s'. skip." % + self.defconfig) self.occupied = False return True

On Mon, Aug 25, 2014 at 12:39:44PM +0900, Masahiro Yamada wrote:
The tools/genboardscfg.py expects all the Kconfig and defconfig are written correctly. Imagine someone accidentally has broken a board. Error-out just for one broken board is annoying for the other developers. Let the tool skip insane boards and continue processing.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

When an error occurs or the program is terminated by the user on the way, the destructer __del__ of class Slot is invoked and the work directories are removed.
We have to make sure there are no subprocesses (in this case, "make O=<work_dir> ...") using the work directories before removing them. Otherwise the subprocess spits a bunch of error messages possibly causing more problems. Perhaps some users may get upset to see too many error messages.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: None
tools/genboardscfg.py | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py index 722b316..67d450f 100755 --- a/tools/genboardscfg.py +++ b/tools/genboardscfg.py @@ -276,6 +276,9 @@ class Slot:
def __del__(self): """Delete the working directory""" + if not self.occupied: + while self.ps.poll() == None: + pass shutil.rmtree(self.build_dir)
def add(self, defconfig):

On Mon, Aug 25, 2014 at 12:39:45PM +0900, Masahiro Yamada wrote:
When an error occurs or the program is terminated by the user on the way, the destructer __del__ of class Slot is invoked and the work directories are removed.
We have to make sure there are no subprocesses (in this case, "make O=<work_dir> ...") using the work directories before removing them. Otherwise the subprocess spits a bunch of error messages possibly causing more problems. Perhaps some users may get upset to see too many error messages.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

This tool deletes the incomplete boards.cfg if it encounters an error or is is terminated by the user.
I notice some problems even though they rarely happen.
[1] The boards.cfg is removed if the program is terminated during __gen_boards_cfg() function but before boards.cfg is actually touched. In this case, the previous boards.cfg should be kept as it is.
[2] If an error occurs while deleting the incomplete boards.cfg, the program throws another exception. This hides the privious exception and we will not be able to know the real cause.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Rebase on commit 055626ac
Changes in v2: None
tools/genboardscfg.py | 156 +++++++++++++++++++++++++++++--------------------- 1 file changed, 90 insertions(+), 66 deletions(-)
diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py index 67d450f..b4620d9 100755 --- a/tools/genboardscfg.py +++ b/tools/genboardscfg.py @@ -413,63 +413,95 @@ class Indicator: sys.stdout.write('\r' + msg) sys.stdout.flush()
-def __gen_boards_cfg(jobs): - """Generate boards.cfg file. +class BoardsFileGenerator:
- Arguments: - jobs: The number of jobs to run simultaneously + """Generator of boards.cfg."""
- Note: - The incomplete boards.cfg is left over when an error (including - the termination by the keyboard interrupt) occurs on the halfway. - """ - check_top_directory() - print 'Generating %s ... (jobs: %d)' % (BOARD_FILE, jobs) - - # All the defconfig files to be processed - defconfigs = [] - for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR): - dirpath = dirpath[len(CONFIG_DIR) + 1:] - for filename in fnmatch.filter(filenames, '*_defconfig'): - if fnmatch.fnmatch(filename, '.*'): - continue - defconfigs.append(os.path.join(dirpath, filename)) - - # Parse all the MAINTAINERS files - maintainers_database = MaintainersDatabase() - for (dirpath, dirnames, filenames) in os.walk('.'): - if 'MAINTAINERS' in filenames: - maintainers_database.parse_file(os.path.join(dirpath, - 'MAINTAINERS')) - - # Output lines should be piped into the reformat tool - reformat_process = subprocess.Popen(REFORMAT_CMD, stdin=subprocess.PIPE, - stdout=open(BOARD_FILE, 'w')) - pipe = reformat_process.stdin - pipe.write(COMMENT_BLOCK) - - indicator = Indicator(len(defconfigs)) - slots = Slots(jobs, pipe, maintainers_database) - - # Main loop to process defconfig files: - # Add a new subprocess into a vacant slot. - # Sleep if there is no available slot. - for defconfig in defconfigs: - while not slots.add(defconfig): - while not slots.available(): - # No available slot: sleep for a while - time.sleep(SLEEP_TIME) - indicator.inc() - - # wait until all the subprocesses finish - while not slots.empty(): - time.sleep(SLEEP_TIME) - print '' - - # wait until the reformat tool finishes - reformat_process.communicate() - if reformat_process.returncode != 0: - sys.exit('"%s" failed' % REFORMAT_CMD[0]) + def __init__(self): + """Prepare basic things for generating boards.cfg.""" + # All the defconfig files to be processed + defconfigs = [] + for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR): + dirpath = dirpath[len(CONFIG_DIR) + 1:] + for filename in fnmatch.filter(filenames, '*_defconfig'): + if fnmatch.fnmatch(filename, '.*'): + continue + defconfigs.append(os.path.join(dirpath, filename)) + self.defconfigs = defconfigs + self.indicator = Indicator(len(defconfigs)) + + # Parse all the MAINTAINERS files + maintainers_database = MaintainersDatabase() + for (dirpath, dirnames, filenames) in os.walk('.'): + if 'MAINTAINERS' in filenames: + maintainers_database.parse_file(os.path.join(dirpath, + 'MAINTAINERS')) + self.maintainers_database = maintainers_database + + def __del__(self): + """Delete the incomplete boards.cfg + + This destructor deletes boards.cfg if the private member 'in_progress' + is defined as True. The 'in_progress' member is set to True at the + beginning of the generate() method and set to False at its end. + So, in_progress==True means generating boards.cfg was terminated + on the way. + """ + + if hasattr(self, 'in_progress') and self.in_progress: + try: + os.remove(BOARD_FILE) + except OSError as exception: + # Ignore 'No such file or directory' error + if exception.errno != errno.ENOENT: + raise + print 'Removed incomplete %s' % BOARD_FILE + + def generate(self, jobs): + """Generate boards.cfg + + This method sets the 'in_progress' member to True at the beginning + and sets it to False on success. The boards.cfg should not be + touched before/after this method because 'in_progress' is used + to detect the incomplete boards.cfg. + + Arguments: + jobs: The number of jobs to run simultaneously + """ + + self.in_progress = True + print 'Generating %s ... (jobs: %d)' % (BOARD_FILE, jobs) + + # Output lines should be piped into the reformat tool + reformat_process = subprocess.Popen(REFORMAT_CMD, + stdin=subprocess.PIPE, + stdout=open(BOARD_FILE, 'w')) + pipe = reformat_process.stdin + pipe.write(COMMENT_BLOCK) + + slots = Slots(jobs, pipe, self.maintainers_database) + + # Main loop to process defconfig files: + # Add a new subprocess into a vacant slot. + # Sleep if there is no available slot. + for defconfig in self.defconfigs: + while not slots.add(defconfig): + while not slots.available(): + # No available slot: sleep for a while + time.sleep(SLEEP_TIME) + self.indicator.inc() + + # wait until all the subprocesses finish + while not slots.empty(): + time.sleep(SLEEP_TIME) + print '' + + # wait until the reformat tool finishes + reformat_process.communicate() + if reformat_process.returncode != 0: + sys.exit('"%s" failed' % REFORMAT_CMD[0]) + + self.in_progress = False
def gen_boards_cfg(jobs): """Generate boards.cfg file. @@ -480,17 +512,9 @@ def gen_boards_cfg(jobs): Arguments: jobs: The number of jobs to run simultaneously """ - try: - __gen_boards_cfg(jobs) - except: - # We should remove incomplete boards.cfg - try: - os.remove(BOARD_FILE) - except OSError as exception: - # Ignore 'No such file or directory' error - if exception.errno != errno.ENOENT: - raise - raise + check_top_directory() + generator = BoardsFileGenerator() + generator.generate(jobs)
def main(): parser = optparse.OptionParser()

On Mon, Aug 25, 2014 at 12:39:46PM +0900, Masahiro Yamada wrote:
This tool deletes the incomplete boards.cfg if it encounters an error or is is terminated by the user.
I notice some problems even though they rarely happen.
[1] The boards.cfg is removed if the program is terminated during __gen_boards_cfg() function but before boards.cfg is actually touched. In this case, the previous boards.cfg should be kept as it is.
[2] If an error occurs while deleting the incomplete boards.cfg, the program throws another exception. This hides the privious exception and we will not be able to know the real cause.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

It looks silly to regenerate the boards.cfg even when it is already up to date.
The tool should exit with doing nothing if the boards.cfg is newer than any of defconfig, Kconfig and MAINTAINERS files.
Specify -f (--force) option to get the boards.cfg regenerated regardless its time stamp.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Use "with ... as ..." and "except ... as ..."
Changes in v2: None
tools/genboardscfg.py | 58 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 2 deletions(-)
diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py index b4620d9..7142567 100755 --- a/tools/genboardscfg.py +++ b/tools/genboardscfg.py @@ -85,6 +85,52 @@ def get_make_cmd(): sys.exit('GNU Make not found') return ret[0].rstrip()
+def output_is_new(): + """Check if the boards.cfg file is up to date. + + Returns: + True if the boards.cfg file exists and is newer than any of + *_defconfig, MAINTAINERS and Kconfig*. False otherwise. + """ + try: + ctime = os.path.getctime(BOARD_FILE) + except OSError as exception: + if exception.errno == errno.ENOENT: + # return False on 'No such file or directory' error + return False + else: + raise + + for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR): + for filename in fnmatch.filter(filenames, '*_defconfig'): + if fnmatch.fnmatch(filename, '.*'): + continue + filepath = os.path.join(dirpath, filename) + if ctime < os.path.getctime(filepath): + return False + + for (dirpath, dirnames, filenames) in os.walk('.'): + for filename in filenames: + if (fnmatch.fnmatch(filename, '*~') or + not fnmatch.fnmatch(filename, 'Kconfig*') and + not filename == 'MAINTAINERS'): + continue + filepath = os.path.join(dirpath, filename) + if ctime < os.path.getctime(filepath): + return False + + # Detect a board that has been removed since the current boards.cfg + # was generated + with open(BOARD_FILE) as f: + for line in f: + if line[0] == '#' or line == '\n': + continue + defconfig = line.split()[6] + '_defconfig' + if not os.path.exists(os.path.join(CONFIG_DIR, defconfig)): + return False + + return True + ### classes ### class MaintainersDatabase:
@@ -503,7 +549,7 @@ class BoardsFileGenerator:
self.in_progress = False
-def gen_boards_cfg(jobs): +def gen_boards_cfg(jobs=1, force=False): """Generate boards.cfg file.
The incomplete boards.cfg is deleted if an error (including @@ -513,6 +559,10 @@ def gen_boards_cfg(jobs): jobs: The number of jobs to run simultaneously """ check_top_directory() + if not force and output_is_new(): + print "%s is up to date. Nothing to do." % BOARD_FILE + sys.exit(0) + generator = BoardsFileGenerator() generator.generate(jobs)
@@ -521,7 +571,10 @@ def main(): # Add options here parser.add_option('-j', '--jobs', help='the number of jobs to run simultaneously') + parser.add_option('-f', '--force', action="store_true", default=False, + help='regenerate the output even if it is new') (options, args) = parser.parse_args() + if options.jobs: try: jobs = int(options.jobs) @@ -534,7 +587,8 @@ def main(): except (OSError, ValueError): print 'info: failed to get the number of CPUs. Set jobs to 1' jobs = 1 - gen_boards_cfg(jobs) + + gen_boards_cfg(jobs, force=options.force)
if __name__ == '__main__': main()

On Mon, Aug 25, 2014 at 12:39:47PM +0900, Masahiro Yamada wrote:
It looks silly to regenerate the boards.cfg even when it is already up to date.
The tool should exit with doing nothing if the boards.cfg is newer than any of defconfig, Kconfig and MAINTAINERS files.
Specify -f (--force) option to get the boards.cfg regenerated regardless its time stamp.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

I guess some developers are already getting sick of this tool because it generally takes a few minites to generate the boards.cfg on a reasonable computer.
The idea popped up on my mind was to skip Makefiles and to run script/kconfig/conf directly. This tool should become about 4 times faster. You might still not be satisfied, but better than doing nothing.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org ---
On my computer (Core i7 2700K + 16GB memory),
60 sec --> 14 sec
This commit does not solve the root cause at all. We still need to find a better way to replace this patch.
Changes in v3: - Use "with ... as ..." instead of explicitely doing f.close()
Changes in v2: - A little optimization move configs/.tmp_defconfig to ./.tmp_defconfig
tools/genboardscfg.py | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py index 7142567..855c99c 100755 --- a/tools/genboardscfg.py +++ b/tools/genboardscfg.py @@ -30,7 +30,7 @@ CONFIG_DIR = 'configs' REFORMAT_CMD = [os.path.join('tools', 'reformat.py'), '-i', '-d', '-', '-s', '8'] SHOW_GNU_MAKE = 'scripts/show-gnu-make' -SLEEP_TIME=0.03 +SLEEP_TIME=0.003
COMMENT_BLOCK = '''# # List of boards @@ -312,13 +312,20 @@ class Slot: Arguments: output: File object which the result is written to maintainers_database: An instance of class MaintainersDatabase + devnull: file object of 'dev/null' + make_cmd: the command name of Make """ - self.occupied = False self.build_dir = tempfile.mkdtemp() self.devnull = devnull - self.make_cmd = make_cmd + self.ps = subprocess.Popen([make_cmd, 'O=' + self.build_dir, + 'allnoconfig'], stdout=devnull) + self.occupied = True self.parser = DotConfigParser(self.build_dir, output, maintainers_database) + self.env = os.environ.copy() + self.env['srctree'] = os.getcwd() + self.env['UBOOTVERSION'] = 'dummy' + self.env['KCONFIG_OBJDIR'] = ''
def __del__(self): """Delete the working directory""" @@ -341,13 +348,31 @@ class Slot: """ if self.occupied: return False - o = 'O=' + self.build_dir - self.ps = subprocess.Popen([self.make_cmd, o, defconfig], - stdout=self.devnull) + + with open(os.path.join(self.build_dir, '.tmp_defconfig'), 'w') as f: + for line in open(os.path.join(CONFIG_DIR, defconfig)): + colon = line.find(':CONFIG_') + if colon == -1: + f.write(line) + else: + f.write(line[colon + 1:]) + + self.ps = subprocess.Popen([os.path.join('scripts', 'kconfig', 'conf'), + '--defconfig=.tmp_defconfig', 'Kconfig'], + stdout=self.devnull, + cwd=self.build_dir, + env=self.env) + self.defconfig = defconfig self.occupied = True return True
+ def wait(self): + """Wait until the current subprocess finishes.""" + while self.occupied and self.ps.poll() == None: + time.sleep(SLEEP_TIME) + self.occupied = False + def poll(self): """Check if the subprocess is running and invoke the .config parser if the subprocess is terminated. @@ -385,6 +410,8 @@ class Slots: for i in range(jobs): self.slots.append(Slot(output, maintainers_database, devnull, make_cmd)) + for slot in self.slots: + slot.wait()
def add(self, defconfig): """Add a new subprocess if a vacant slot is available.

On Mon, Aug 25, 2014 at 12:39:48PM +0900, Masahiro Yamada wrote:
I guess some developers are already getting sick of this tool because it generally takes a few minites to generate the boards.cfg on a reasonable computer.
The idea popped up on my mind was to skip Makefiles and to run script/kconfig/conf directly. This tool should become about 4 times faster. You might still not be satisfied, but better than doing nothing.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (2)
-
Masahiro Yamada
-
Tom Rini