* [U-Boot] [PATCH 1/5] tools: moveconfig: fix needless move for config with default 1
2016-06-15 5:33 [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement Masahiro Yamada
@ 2016-06-15 5:33 ` Masahiro Yamada
2016-06-20 21:25 ` Joe Hershberger
2016-06-15 5:33 ` [U-Boot] [PATCH 2/5] tools: moveconfig: change class WorkDir to class ReferenceSource Masahiro Yamada
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Masahiro Yamada @ 2016-06-15 5:33 UTC (permalink / raw)
To: u-boot
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 <yamada.masahiro@socionext.com>
---
tools/moveconfig.py | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 238e961..06ff4d9 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.
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 2/5] tools: moveconfig: change class WorkDir to class ReferenceSource
2016-06-15 5:33 [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement Masahiro Yamada
2016-06-15 5:33 ` [U-Boot] [PATCH 1/5] tools: moveconfig: fix needless move for config with default 1 Masahiro Yamada
@ 2016-06-15 5:33 ` Masahiro Yamada
2016-06-20 21:28 ` Joe Hershberger
2016-06-15 5:33 ` [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching Masahiro Yamada
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Masahiro Yamada @ 2016-06-15 5:33 UTC (permalink / raw)
To: u-boot
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 <yamada.masahiro@socionext.com>
---
tools/moveconfig.py | 49 ++++++++++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 06ff4d9..f4e2580 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)]
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching
2016-06-15 5:33 [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement Masahiro Yamada
2016-06-15 5:33 ` [U-Boot] [PATCH 1/5] tools: moveconfig: fix needless move for config with default 1 Masahiro Yamada
2016-06-15 5:33 ` [U-Boot] [PATCH 2/5] tools: moveconfig: change class WorkDir to class ReferenceSource Masahiro Yamada
@ 2016-06-15 5:33 ` Masahiro Yamada
2016-06-20 21:33 ` Joe Hershberger
2016-06-15 5:33 ` [U-Boot] [PATCH 4/5] tools: moveconfig: simplify show_failed_boards() and show more info Masahiro Yamada
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Masahiro Yamada @ 2016-06-15 5:33 UTC (permalink / raw)
To: u-boot
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 <reference_src_dir>" 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 <yamada.masahiro@socionext.com>
---
tools/moveconfig.py | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index f4e2580..0e03751 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)]
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching
2016-06-15 5:33 ` [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching Masahiro Yamada
@ 2016-06-20 21:33 ` Joe Hershberger
2016-06-21 1:53 ` Masahiro Yamada
0 siblings, 1 reply; 16+ messages in thread
From: Joe Hershberger @ 2016-06-20 21:33 UTC (permalink / raw)
To: u-boot
On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 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 <reference_src_dir>" 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 <yamada.masahiro@socionext.com>
> ---
>
> tools/moveconfig.py | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> index f4e2580..0e03751 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
This seems less clear to read. There is no current source dir? I think
you need a different name.
> 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)]
> --
> 1.9.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching
2016-06-20 21:33 ` Joe Hershberger
@ 2016-06-21 1:53 ` Masahiro Yamada
2016-06-21 16:25 ` Joe Hershberger
0 siblings, 1 reply; 16+ messages in thread
From: Masahiro Yamada @ 2016-06-21 1:53 UTC (permalink / raw)
To: u-boot
2016-06-21 6:33 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:
> On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> 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 <reference_src_dir>" 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 <yamada.masahiro@socionext.com>
>> ---
>>
>> tools/moveconfig.py | 22 +++++++++-------------
>> 1 file changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
>> index f4e2580..0e03751 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
>
> This seems less clear to read. There is no current source dir? I think
> you need a different name.
Maybe, self.subprocess_dir or something?
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching
2016-06-21 1:53 ` Masahiro Yamada
@ 2016-06-21 16:25 ` Joe Hershberger
2016-06-21 21:13 ` Masahiro Yamada
0 siblings, 1 reply; 16+ messages in thread
From: Joe Hershberger @ 2016-06-21 16:25 UTC (permalink / raw)
To: u-boot
On Mon, Jun 20, 2016 at 8:53 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2016-06-21 6:33 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:
>> On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>> 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 <reference_src_dir>" 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 <yamada.masahiro@socionext.com>
>>> ---
>>>
>>> tools/moveconfig.py | 22 +++++++++-------------
>>> 1 file changed, 9 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
>>> index f4e2580..0e03751 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
>>
>> This seems less clear to read. There is no current source dir? I think
>> you need a different name.
>
>
> Maybe, self.subprocess_dir or something?
How about something like self.alternate_src_dir?
-Joe
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching
2016-06-21 16:25 ` Joe Hershberger
@ 2016-06-21 21:13 ` Masahiro Yamada
2016-06-21 21:41 ` Joe Hershberger
0 siblings, 1 reply; 16+ messages in thread
From: Masahiro Yamada @ 2016-06-21 21:13 UTC (permalink / raw)
To: u-boot
2016-06-22 1:25 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:
> On Mon, Jun 20, 2016 at 8:53 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> 2016-06-21 6:33 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:
>>> On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada
>>> <yamada.masahiro@socionext.com> wrote:
>>>> 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 <reference_src_dir>" 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 <yamada.masahiro@socionext.com>
>>>> ---
>>>>
>>>> tools/moveconfig.py | 22 +++++++++-------------
>>>> 1 file changed, 9 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
>>>> index f4e2580..0e03751 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
>>>
>>> This seems less clear to read. There is no current source dir? I think
>>> you need a different name.
>>
>>
>> Maybe, self.subprocess_dir or something?
>
> How about something like self.alternate_src_dir?
>
So,
reference_src_dir is still
alternate_src_dir moves
This is not clear to me from the variable names.
My first choice "current" means
it is a moving directory.
I can live with subprocess_dir, though.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching
2016-06-21 21:13 ` Masahiro Yamada
@ 2016-06-21 21:41 ` Joe Hershberger
0 siblings, 0 replies; 16+ messages in thread
From: Joe Hershberger @ 2016-06-21 21:41 UTC (permalink / raw)
To: u-boot
On Tue, Jun 21, 2016 at 4:13 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2016-06-22 1:25 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:
>> On Mon, Jun 20, 2016 at 8:53 PM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>> 2016-06-21 6:33 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:
>>>> On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada
>>>> <yamada.masahiro@socionext.com> wrote:
>>>>> 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 <reference_src_dir>" 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 <yamada.masahiro@socionext.com>
>>>>> ---
>>>>>
>>>>> tools/moveconfig.py | 22 +++++++++-------------
>>>>> 1 file changed, 9 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
>>>>> index f4e2580..0e03751 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
>>>>
>>>> This seems less clear to read. There is no current source dir? I think
>>>> you need a different name.
>>>
>>>
>>> Maybe, self.subprocess_dir or something?
>>
>> How about something like self.alternate_src_dir?
>>
>
> So,
> reference_src_dir is still
> alternate_src_dir moves
>
>
> This is not clear to me from the variable names.
>
>
> My first choice "current" means
> it is a moving directory.
>
> I can live with subprocess_dir, though.
OK, do that then.
-Joe
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 4/5] tools: moveconfig: simplify show_failed_boards() and show more info
2016-06-15 5:33 [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement Masahiro Yamada
` (2 preceding siblings ...)
2016-06-15 5:33 ` [U-Boot] [PATCH 3/5] tools: moveconfig: simplify source tree switching Masahiro Yamada
@ 2016-06-15 5:33 ` Masahiro Yamada
2016-06-20 21:35 ` Joe Hershberger
2016-06-15 5:33 ` [U-Boot] [PATCH 5/5] tools: moveconfig: show suspicious boards with possible misconversion Masahiro Yamada
2016-06-22 0:25 ` [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement Masahiro Yamada
5 siblings, 1 reply; 16+ messages in thread
From: Masahiro Yamada @ 2016-06-15 5:33 UTC (permalink / raw)
To: u-boot
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 <yamada.masahiro@socionext.com>
---
tools/moveconfig.py | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 0e03751..f795a7f 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:
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 4/5] tools: moveconfig: simplify show_failed_boards() and show more info
2016-06-15 5:33 ` [U-Boot] [PATCH 4/5] tools: moveconfig: simplify show_failed_boards() and show more info Masahiro Yamada
@ 2016-06-20 21:35 ` Joe Hershberger
0 siblings, 0 replies; 16+ messages in thread
From: Joe Hershberger @ 2016-06-20 21:35 UTC (permalink / raw)
To: u-boot
On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 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 <yamada.masahiro@socionext.com>
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 5/5] tools: moveconfig: show suspicious boards with possible misconversion
2016-06-15 5:33 [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement Masahiro Yamada
` (3 preceding siblings ...)
2016-06-15 5:33 ` [U-Boot] [PATCH 4/5] tools: moveconfig: simplify show_failed_boards() and show more info Masahiro Yamada
@ 2016-06-15 5:33 ` Masahiro Yamada
2016-06-20 21:37 ` Joe Hershberger
2016-06-22 0:25 ` [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement Masahiro Yamada
5 siblings, 1 reply; 16+ messages in thread
From: Masahiro Yamada @ 2016-06-15 5:33 UTC (permalink / raw)
To: u-boot
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 <yamada.masahiro@socionext.com>
---
tools/moveconfig.py | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index f795a7f..bf60dbc 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:
--
1.9.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [U-Boot] [PATCH 5/5] tools: moveconfig: show suspicious boards with possible misconversion
2016-06-15 5:33 ` [U-Boot] [PATCH 5/5] tools: moveconfig: show suspicious boards with possible misconversion Masahiro Yamada
@ 2016-06-20 21:37 ` Joe Hershberger
0 siblings, 0 replies; 16+ messages in thread
From: Joe Hershberger @ 2016-06-20 21:37 UTC (permalink / raw)
To: u-boot
On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 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 <yamada.masahiro@socionext.com>
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement
2016-06-15 5:33 [U-Boot] [PATCH 0/5] tools: moveconfig: some more fix, cleanups, improvement Masahiro Yamada
` (4 preceding siblings ...)
2016-06-15 5:33 ` [U-Boot] [PATCH 5/5] tools: moveconfig: show suspicious boards with possible misconversion Masahiro Yamada
@ 2016-06-22 0:25 ` Masahiro Yamada
5 siblings, 0 replies; 16+ messages in thread
From: Masahiro Yamada @ 2016-06-22 0:25 UTC (permalink / raw)
To: u-boot
2016-06-15 14:33 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>
> I sent a pull request for moveconfig recently,
> but this tool can be improved more.
>
> (This series has no impact on the on-going BOOTDELAY fix-up.)
>
>
>
> Masahiro Yamada (5):
> tools: moveconfig: fix needless move for config with default 1
> tools: moveconfig: change class WorkDir to class ReferenceSource
> tools: moveconfig: simplify source tree switching
> tools: moveconfig: simplify show_failed_boards() and show more info
> tools: moveconfig: show suspicious boards with possible misconversion
>
> tools/moveconfig.py | 130 ++++++++++++++++++++++++++++++++-
Applied to u-boot-uniphier/master.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 16+ messages in thread