From 5030159e27a1d6293f0f064807d4eef5dd6de714 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 15 Jun 2016 14:33:50 +0900 Subject: tools: moveconfig: fix needless move for config with default 1 When moving an integer type option with default value 1, the tool moves configs with the same value as the default (, and then removed by the later savedefconfig). This is a needless operation. The KconfigParser.parse_one_config() should compare the config after the "=y -> =1" fixup. Signed-off-by: Masahiro Yamada Reviewed-by: Joe Hershberger --- tools/moveconfig.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/moveconfig.py b/tools/moveconfig.py index 5e5ca06d8f..03acbea94c 100755 --- a/tools/moveconfig.py +++ b/tools/moveconfig.py @@ -487,9 +487,6 @@ class KconfigParser: else: new_val = not_set - if old_val == new_val: - return (ACTION_NO_CHANGE, new_val) - # If this CONFIG is neither bool nor trisate if old_val[-2:] != '=y' and old_val[-2:] != '=m' and old_val != not_set: # tools/scripts/define2mk.sed changes '1' to 'y'. @@ -498,7 +495,8 @@ class KconfigParser: if new_val[-2:] == '=y': new_val = new_val[:-1] + '1' - return (ACTION_MOVE, new_val) + return (ACTION_NO_CHANGE if old_val == new_val else ACTION_MOVE, + new_val) def update_dotconfig(self): """Parse files for the config options and update the .config. -- cgit v1.2.1 From 5cc42a51846cd69596081a9cd9d2bd0495815525 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 15 Jun 2016 14:33:51 +0900 Subject: tools: moveconfig: change class WorkDir to class ReferenceSource The class WorkDir can be used in a very generic way, but currently it is only used for containing a reference source directory. This commit changes it for a more dedicated use. The move_config function can be more readable by enclosing the git-clone and git- checkout in the class constructor. Signed-off-by: Masahiro Yamada Reviewed-by: Joe Hershberger --- tools/moveconfig.py | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/tools/moveconfig.py b/tools/moveconfig.py index 03acbea94c..44be51fb8a 100755 --- a/tools/moveconfig.py +++ b/tools/moveconfig.py @@ -880,23 +880,39 @@ class Slots: for board in failed_boards: f.write(board + '\n') -class WorkDir: - def __init__(self): - """Create a new working directory.""" - self.work_dir = tempfile.mkdtemp() +class ReferenceSource: + + """Reference source against which original configs should be parsed.""" + + def __init__(self, commit): + """Create a reference source directory based on a specified commit. + + Arguments: + commit: commit to git-clone + """ + self.src_dir = tempfile.mkdtemp() + print "Cloning git repo to a separate work directory..." + subprocess.check_output(['git', 'clone', os.getcwd(), '.'], + cwd=self.src_dir) + print "Checkout '%s' to build the original autoconf.mk." % \ + subprocess.check_output(['git', 'rev-parse', '--short', commit]).strip() + subprocess.check_output(['git', 'checkout', commit], + stderr=subprocess.STDOUT, cwd=self.src_dir) def __del__(self): - """Delete the working directory + """Delete the reference source directory This function makes sure the temporary directory is cleaned away even if Python suddenly dies due to error. It should be done in here because it is guaranteed the destructor is always invoked when the instance of the class gets unreferenced. """ - shutil.rmtree(self.work_dir) + shutil.rmtree(self.src_dir) + + def get_dir(self): + """Return the absolute path to the reference source directory.""" - def get(self): - return self.work_dir + return self.src_dir def move_config(configs, options): """Move config options to defconfig files. @@ -914,20 +930,11 @@ def move_config(configs, options): print 'Move ' + ', '.join(configs), print '(jobs: %d)\n' % options.jobs - reference_src_dir = '' - if options.git_ref: - work_dir = WorkDir() - reference_src_dir = work_dir.get() - print "Cloning git repo to a separate work directory..." - subprocess.check_output(['git', 'clone', os.getcwd(), '.'], - cwd=reference_src_dir) - print "Checkout '%s' to build the original autoconf.mk." % \ - subprocess.check_output(['git', 'rev-parse', '--short', - options.git_ref]).strip() - subprocess.check_output(['git', 'checkout', options.git_ref], - stderr=subprocess.STDOUT, - cwd=reference_src_dir) + reference_src = ReferenceSource(options.git_ref) + reference_src_dir = reference_src.get_dir() + else: + reference_src_dir = '' if options.defconfigs: defconfigs = [line.strip() for line in open(options.defconfigs)] -- cgit v1.2.1 From f432c33f27898070718dd568feb104fc1870ca15 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 15 Jun 2016 14:33:52 +0900 Subject: tools: moveconfig: simplify source tree switching The subprocess.Popen() does not change the child process's working directory if cwd=None is given. Let's exploit this fact to refactor the source directory handling. We no longer have to pass "-C " to the sub-process because self.current_src_dir tracks the source tree against which we want to run defconfig/autoconf. The flag self.use_git_ref is not necessary either because we can know the current state by checking whether the self.current_src_dir is a valid string or None. Signed-off-by: Masahiro Yamada --- tools/moveconfig.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tools/moveconfig.py b/tools/moveconfig.py index 44be51fb8a..c0ac5f4e13 100755 --- a/tools/moveconfig.py +++ b/tools/moveconfig.py @@ -645,7 +645,7 @@ class Slot: self.defconfig = defconfig self.log = '' - self.use_git_ref = True if self.options.git_ref else False + self.current_src_dir = self.reference_src_dir self.do_defconfig() return True @@ -674,13 +674,13 @@ class Slot: if self.ps.poll() != 0: self.handle_error() elif self.state == STATE_DEFCONFIG: - if self.options.git_ref and not self.use_git_ref: + if self.reference_src_dir and not self.current_src_dir: self.do_savedefconfig() else: self.do_autoconf() elif self.state == STATE_AUTOCONF: - if self.use_git_ref: - self.use_git_ref = False + if self.current_src_dir: + self.current_src_dir = None self.do_defconfig() else: self.do_savedefconfig() @@ -706,11 +706,9 @@ class Slot: cmd = list(self.make_cmd) cmd.append(self.defconfig) - if self.use_git_ref: - cmd.append('-C') - cmd.append(self.reference_src_dir) self.ps = subprocess.Popen(cmd, stdout=self.devnull, - stderr=subprocess.PIPE) + stderr=subprocess.PIPE, + cwd=self.current_src_dir) self.state = STATE_DEFCONFIG def do_autoconf(self): @@ -728,11 +726,9 @@ class Slot: cmd.append('CROSS_COMPILE=%s' % self.cross_compile) cmd.append('KCONFIG_IGNORE_DUPLICATES=1') cmd.append('include/config/auto.conf') - if self.use_git_ref: - cmd.append('-C') - cmd.append(self.reference_src_dir) self.ps = subprocess.Popen(cmd, stdout=self.devnull, - stderr=subprocess.PIPE) + stderr=subprocess.PIPE, + cwd=self.current_src_dir) self.state = STATE_AUTOCONF def do_savedefconfig(self): @@ -934,7 +930,7 @@ def move_config(configs, options): reference_src = ReferenceSource(options.git_ref) reference_src_dir = reference_src.get_dir() else: - reference_src_dir = '' + reference_src_dir = None if options.defconfigs: defconfigs = [line.strip() for line in open(options.defconfigs)] -- cgit v1.2.1 From 96dccd9767311f4b8e585fd9e33fcb2a09e36951 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 15 Jun 2016 14:33:53 +0900 Subject: tools: moveconfig: simplify show_failed_boards() and show more info Since commit 1d085568b3de ("tools: moveconfig: display log atomically in more readable format"), the function color_text() is clever enough to exclude LF from escape sequences. Exploit it for removing the "for" loops from Slots.show_failed_boards(). Also, display "(the list has been saved in moveconfig.failed)" if there are failed boards. Signed-off-by: Masahiro Yamada Reviewed-by: Joe Hershberger --- tools/moveconfig.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tools/moveconfig.py b/tools/moveconfig.py index c0ac5f4e13..7a4136d205 100755 --- a/tools/moveconfig.py +++ b/tools/moveconfig.py @@ -860,21 +860,22 @@ class Slots: def show_failed_boards(self): """Display all of the failed boards (defconfigs).""" - failed_boards = [] + boards = [] + output_file = 'moveconfig.failed' for slot in self.slots: - failed_boards += slot.get_failed_boards() - - if len(failed_boards) > 0: - msg = [ "The following boards were not processed due to error:" ] - msg += failed_boards - for line in msg: - print >> sys.stderr, color_text(self.options.color, - COLOR_LIGHT_RED, line) - - with open('moveconfig.failed', 'w') as f: - for board in failed_boards: - f.write(board + '\n') + boards += slot.get_failed_boards() + + if boards: + boards = '\n'.join(boards) + '\n' + msg = "The following boards were not processed due to error:\n" + msg += boards + msg += "(the list has been saved in %s)\n" % output_file + print >> sys.stderr, color_text(self.options.color, COLOR_LIGHT_RED, + msg) + + with open(output_file, 'w') as f: + f.write(boards) class ReferenceSource: -- cgit v1.2.1 From fc2661eebe9e788aee61dcb0c9c8337cda1ae93b Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 15 Jun 2016 14:33:54 +0900 Subject: tools: moveconfig: show suspicious boards with possible misconversion There are some cases where config options are moved, but they are ripped off at the final savedefconfig stage: - The moved option is not user-configurable, for example, due to a missing prompt in the Kconfig entry - The config was not defined in the original config header despite the Kconfig specifies it as non-bool type - The config define in the header contains reference to another macro, for example: #define CONFIG_CONS_INDEX (CONFIG_SYS_LPC32XX_UART - 2) The current moveconfig does not support recursive macro expansion. In these cases, the conversion is very likely to be an unexpected result. That is why I decided to display the log in yellow color in commit 5da4f857beac ("tools: moveconfig: report when CONFIGs are removed by savedefconfig"). It would be nice to display the list of suspicious boards when the tool finishes processing. It is highly recommended to check the defconfigs once again when this message is displayed. Signed-off-by: Masahiro Yamada Reviewed-by: Joe Hershberger --- tools/moveconfig.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tools/moveconfig.py b/tools/moveconfig.py index 7a4136d205..d362923b22 100755 --- a/tools/moveconfig.py +++ b/tools/moveconfig.py @@ -611,6 +611,7 @@ class Slot: self.parser = KconfigParser(configs, options, self.build_dir) self.state = STATE_IDLE self.failed_boards = [] + self.suspicious_boards = [] def __del__(self): """Delete the working directory @@ -755,7 +756,10 @@ class Slot: def update_defconfig(self): """Update the input defconfig and go back to the idle state.""" - self.log += self.parser.check_defconfig() + log = self.parser.check_defconfig() + if log: + self.suspicious_boards.append(self.defconfig) + self.log += log orig_defconfig = os.path.join('configs', self.defconfig) new_defconfig = os.path.join(self.build_dir, 'defconfig') updated = not filecmp.cmp(orig_defconfig, new_defconfig) @@ -799,6 +803,11 @@ class Slot: """ return self.failed_boards + def get_suspicious_boards(self): + """Returns a list of boards (defconfigs) with possible misconversion. + """ + return self.suspicious_boards + class Slots: """Controller of the array of subprocess slots.""" @@ -877,6 +886,26 @@ class Slots: with open(output_file, 'w') as f: f.write(boards) + def show_suspicious_boards(self): + """Display all boards (defconfigs) with possible misconversion.""" + boards = [] + output_file = 'moveconfig.suspicious' + + for slot in self.slots: + boards += slot.get_suspicious_boards() + + if boards: + boards = '\n'.join(boards) + '\n' + msg = "The following boards might have been converted incorrectly.\n" + msg += "It is highly recommended to check them manually:\n" + msg += boards + msg += "(the list has been saved in %s)\n" % output_file + print >> sys.stderr, color_text(self.options.color, COLOR_YELLOW, + msg) + + with open(output_file, 'w') as f: + f.write(boards) + class ReferenceSource: """Reference source against which original configs should be parsed.""" @@ -967,6 +996,7 @@ def move_config(configs, options): print '' slots.show_failed_boards() + slots.show_suspicious_boards() def main(): try: -- cgit v1.2.1