Hi Alper,
On Sat, 29 Jan 2022 at 08:22, Alper Nebi Yasak <alpernebiya...@gmail.com> wrote:
>
> Commit 37f815cad07d ("moveconfig: Use a function to read files") adds a
> helper function that can read a file as lines, but strips the newline
> characters. This change broke parts of moveconfig code that relied on
> their existence, resulting in a few issues:
>
> Configs that are defined as empty aren't removed from header files (e.g.
> "#define CONFIG_REMAKE_ELF"). Make regex patterns use '\b' to match word
> boundaries instead of '\W' (which matched the newlines) so these lines
> still match and get removed.
>
> All changes in defconfig are considered removed by savedefconfig even
> if they weren't, and line continuations in the headers aren't recognized
> and removed properly, because their checks explicitly look for a newline
> character. Remove the character from both comparisons.
>
> The printed diff of header files is wrongly formatted and raises an
> IndexError if a blank line was removed. Let print() print the new lines,
> and use size-independent ways to check strings to fix the diff output.
>
> Signed-off-by: Alper Nebi Yasak <alpernebiya...@gmail.com>
> ---
>
> tools/moveconfig.py | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
I was a little worried about the subtleties here. Thanks for fixing
it! I almost got annoyed and wrote some tests, but I guess we can limp
on without them.
>
> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> index 35fe6710d70a..1bcf58caf14c 100755
> --- a/tools/moveconfig.py
> +++ b/tools/moveconfig.py
> @@ -205,12 +205,12 @@ def show_diff(alines, blines, file_path, color_enabled):
> tofile=os.path.join('b', file_path))
>
> for line in diff:
> - if line[0] == '-' and line[1] != '-':
> - print(color_text(color_enabled, COLOR_RED, line), end=' ')
> - elif line[0] == '+' and line[1] != '+':
> - print(color_text(color_enabled, COLOR_GREEN, line), end=' ')
> + if line.startswith('-') and not line.startswith('--'):
> + print(color_text(color_enabled, COLOR_RED, line))
> + elif line.startswith('+') and not line.startswith('++'):
> + print(color_text(color_enabled, COLOR_GREEN, line))
> else:
> - print(line, end=' ')
> + print(line)
>
> def extend_matched_lines(lines, matched, pre_patterns, post_patterns,
> extend_pre, extend_post):
> @@ -368,7 +368,7 @@ def cleanup_one_header(header_path, patterns, args):
>
> matched = []
> for i, line in enumerate(lines):
> - if i - 1 in matched and lines[i - 1][-2:] == '\\\n':
> + if i - 1 in matched and lines[i - 1].endswith('\\'):
> matched.append(i)
> continue
> for pattern in patterns:
> @@ -380,9 +380,9 @@ def cleanup_one_header(header_path, patterns, args):
> return
>
> # remove empty #ifdef ... #endif, successive blank lines
> - pattern_if = re.compile(r'#\s*if(def|ndef)?\W') # #if, #ifdef, #ifndef
> - pattern_elif = re.compile(r'#\s*el(if|se)\W') # #elif, #else
> - pattern_endif = re.compile(r'#\s*endif\W') # #endif
> + pattern_if = re.compile(r'#\s*if(def|ndef)?\b') # #if, #ifdef, #ifndef
> + pattern_elif = re.compile(r'#\s*el(if|se)\b') # #elif, #else
> + pattern_endif = re.compile(r'#\s*endif\b') # #endif
> pattern_blank = re.compile(r'^\s*$') # empty line
>
> while True:
> @@ -424,8 +424,8 @@ def cleanup_headers(configs, args):
>
> patterns = []
> for config in configs:
> - patterns.append(re.compile(r'#\s*define\s+%s\W' % config))
> - patterns.append(re.compile(r'#\s*undef\s+%s\W' % config))
> + patterns.append(re.compile(r'#\s*define\s+%s\b' % config))
> + patterns.append(re.compile(r'#\s*undef\s+%s\b' % config))
>
> for dir in 'include', 'arch', 'board':
> for (dirpath, dirnames, filenames) in os.walk(dir):
> @@ -451,7 +451,7 @@ def cleanup_one_extra_option(defconfig_path, configs,
> args):
> """
>
> start = 'CONFIG_SYS_EXTRA_OPTIONS="'
> - end = '"\n'
> + end = '"'
''
>
> lines = read_file(defconfig_path)
>
> @@ -812,7 +812,7 @@ def check_defconfig(self):
> for (action, value) in self.results:
> if action != ACTION_MOVE:
> continue
> - if not value + '\n' in defconfig_lines:
> + if not value in defconfig_lines:
> log += color_text(self.args.color, COLOR_YELLOW,
> "'%s' was removed by savedefconfig.\n" %
> value)
> --
> 2.34.1
>
Regards,
Simon
