[PATCH] moveconfig: Fix code relying on now-stripped newline characters

[Simon Glass]

Hi Alper,

On Sun, 30 Jan 2022 at 13:14, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>
> On 30/01/2022 00:09, Simon Glass wrote:
> > On Sat, 29 Jan 2022 at 08:22, Alper Nebi Yasak <alpernebiyasak at 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
> >
> > 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.
>
> Can't guarantee I found everything, just lucky I was trying a migration
> on REMAKE_ELF :)

Reviewed-by: Simon Glass <sjg at chromium.org>
Tested-by: Simon Glass <sjg at chromium.org>

>
> >>
> >> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> >> index 35fe6710d70a..1bcf58caf14c 100755
> >> --- a/tools/moveconfig.py
> >> +++ b/tools/moveconfig.py
> >> @@ -451,7 +451,7 @@ def cleanup_one_extra_option(defconfig_path, configs, args):
> >>      """
> >>
> >>      start = 'CONFIG_SYS_EXTRA_OPTIONS="'
> >> -    end = '"\n'
> >> +    end = '"'
> >
> > ''
>
> Looks to me like the double-quote character is necessary here, if I add
> CONFIG_SYS_EXTRA_OPTIONS="REMAKE_ELF" to chromebook_bob_defconfig and
> try migrating CONFIG_REMAKE_ELF the line doesn't get removed otherwise.

Yes you are right, ignore this. It is barely legible in the font I am
using here.

>
> Trying to add more lines of diff context below:
>
> >>
> >>      lines = read_file(defconfig_path)
> >>
>         for i, line in enumerate(lines):
>             if line.startswith(start) and line.endswith(end):
>                 break
>         else:
>             # CONFIG_SYS_EXTRA_OPTIONS was not found in this defconfig
>             return
>
>         old_tokens = line[len(start):-len(end)].split(',')
>         new_tokens = []
>
>         for token in old_tokens:
>             pos = token.find('=')
>             if not (token[:pos] if pos >= 0 else token) in configs:
>                 new_tokens.append(token)

Regards,
Simon