* [U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks
@ 2019-01-30 7:23 Chris Packham
2019-01-30 8:55 ` Masahiro Yamada
2019-02-10 13:08 ` [U-Boot] [U-Boot, " Tom Rini
0 siblings, 2 replies; 5+ messages in thread
From: Chris Packham @ 2019-01-30 7:23 UTC (permalink / raw)
To: u-boot
Moveconfig already attempts to remove empty #if/#endif blocks when there
is a matching CONFIG_ being moved. Add a second pass which covers files
without a match.
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
This was previously submitted as
http://patchwork.ozlabs.org/patch/924901/ there still seems to be cases
of #if/#endif left over from Kconfig migrations so perhaps this is still
needed/wanted.
I've plumbed this in as a second pass because ultimately we may want to
make this a separate option. Also I couldn't figure out how to implement
this without using re.M so I couldn't make it work in with the line by
line parsing of cleanup_one_header().
Changes in v3:
- drop rfc
Changes in v2:
- use re.compile
tools/moveconfig.py | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index caa81ac2ed77..1a214c560546 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -545,6 +545,28 @@ def confirm(options, prompt):
return True
+def cleanup_empty_blocks(header_path, options):
+ """Clean up empty conditional blocks
+
+ Arguments:
+ header_path: path to the cleaned file.
+ options: option flags.
+ """
+ pattern = re.compile(r'^\s*#\s*if.*$\n^\s*#\s*endif.*$\n*', flags=re.M)
+ with open(header_path) as f:
+ data = f.read()
+
+ new_data = pattern.sub('\n', data)
+
+ show_diff(data.splitlines(True), new_data.splitlines(True), header_path,
+ options.color)
+
+ if options.dry_run:
+ return
+
+ with open(header_path, 'w') as f:
+ f.write(new_data)
+
def cleanup_one_header(header_path, patterns, options):
"""Clean regex-matched lines away from a file.
@@ -626,8 +648,9 @@ def cleanup_headers(configs, options):
continue
for filename in filenames:
if not fnmatch.fnmatch(filename, '*~'):
- cleanup_one_header(os.path.join(dirpath, filename),
- patterns, options)
+ header_path = os.path.join(dirpath, filename)
+ cleanup_one_header(header_path, patterns, options)
+ cleanup_empty_blocks(header_path, options)
def cleanup_one_extra_option(defconfig_path, configs, options):
"""Delete config defines in CONFIG_SYS_EXTRA_OPTIONS in one defconfig file.
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks
2019-01-30 7:23 [U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks Chris Packham
@ 2019-01-30 8:55 ` Masahiro Yamada
2019-01-31 6:25 ` Masahiro Yamada
2019-02-10 13:08 ` [U-Boot] [U-Boot, " Tom Rini
1 sibling, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2019-01-30 8:55 UTC (permalink / raw)
To: u-boot
On Wed, Jan 30, 2019 at 4:23 PM Chris Packham <judge.packham@gmail.com> wrote:
>
> Moveconfig already attempts to remove empty #if/#endif blocks when there
> is a matching CONFIG_ being moved. Add a second pass which covers files
> without a match.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> This was previously submitted as
> http://patchwork.ozlabs.org/patch/924901/ there still seems to be cases
> of #if/#endif left over from Kconfig migrations so perhaps this is still
> needed/wanted.
>
> I've plumbed this in as a second pass because ultimately we may want to
> make this a separate option. Also I couldn't figure out how to implement
> this without using re.M so I couldn't make it work in with the line by
> line parsing of cleanup_one_header().
This seems useful to find leftover code
regardless of the CONFIG options you are moving.
One drawback is, it may fold unrelated cleanups.
Maybe, it would be useful to add a separate option to turn on this feature,
or make it into a separate tool.
> Changes in v3:
> - drop rfc
>
> Changes in v2:
> - use re.compile
>
> tools/moveconfig.py | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> index caa81ac2ed77..1a214c560546 100755
> --- a/tools/moveconfig.py
> +++ b/tools/moveconfig.py
> @@ -545,6 +545,28 @@ def confirm(options, prompt):
>
> return True
>
> +def cleanup_empty_blocks(header_path, options):
> + """Clean up empty conditional blocks
> +
> + Arguments:
> + header_path: path to the cleaned file.
> + options: option flags.
> + """
> + pattern = re.compile(r'^\s*#\s*if.*$\n^\s*#\s*endif.*$\n*', flags=re.M)
> + with open(header_path) as f:
> + data = f.read()
> +
> + new_data = pattern.sub('\n', data)
> +
> + show_diff(data.splitlines(True), new_data.splitlines(True), header_path,
> + options.color)
> +
> + if options.dry_run:
> + return
> +
> + with open(header_path, 'w') as f:
> + f.write(new_data)
> +
> def cleanup_one_header(header_path, patterns, options):
> """Clean regex-matched lines away from a file.
>
> @@ -626,8 +648,9 @@ def cleanup_headers(configs, options):
> continue
> for filename in filenames:
> if not fnmatch.fnmatch(filename, '*~'):
> - cleanup_one_header(os.path.join(dirpath, filename),
> - patterns, options)
> + header_path = os.path.join(dirpath, filename)
> + cleanup_one_header(header_path, patterns, options)
> + cleanup_empty_blocks(header_path, options)
>
> def cleanup_one_extra_option(defconfig_path, configs, options):
> """Delete config defines in CONFIG_SYS_EXTRA_OPTIONS in one defconfig file.
> --
> 2.20.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks
2019-01-30 8:55 ` Masahiro Yamada
@ 2019-01-31 6:25 ` Masahiro Yamada
2019-01-31 9:05 ` Chris Packham
0 siblings, 1 reply; 5+ messages in thread
From: Masahiro Yamada @ 2019-01-31 6:25 UTC (permalink / raw)
To: u-boot
On Wed, Jan 30, 2019 at 5:55 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Wed, Jan 30, 2019 at 4:23 PM Chris Packham <judge.packham@gmail.com> wrote:
> >
> > Moveconfig already attempts to remove empty #if/#endif blocks when there
> > is a matching CONFIG_ being moved. Add a second pass which covers files
> > without a match.
> >
> > Signed-off-by: Chris Packham <judge.packham@gmail.com>
> > ---
> > This was previously submitted as
> > http://patchwork.ozlabs.org/patch/924901/ there still seems to be cases
> > of #if/#endif left over from Kconfig migrations so perhaps this is still
> > needed/wanted.
> >
> > I've plumbed this in as a second pass because ultimately we may want to
> > make this a separate option. Also I couldn't figure out how to implement
> > this without using re.M so I couldn't make it work in with the line by
> > line parsing of cleanup_one_header().
>
>
> This seems useful to find leftover code
> regardless of the CONFIG options you are moving.
>
> One drawback is, it may fold unrelated cleanups.
>
> Maybe, it would be useful to add a separate option to turn on this feature,
> or make it into a separate tool.
>
>
>
I tested this patch.
It detected one empty block.
diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
index 04bce2f..ac1a6cb 100644
--- a/include/configs/omap3_cairo.h
+++ b/include/configs/omap3_cairo.h
@@ -197,8 +197,6 @@
* are needed and peripheral clocks for UART2 must be enabled in
* function per_clocks_enable().
*/
-#ifdef CONFIG_SPL_BUILD
-#endif
/* Provide the MACH_TYPE value the vendor kernel requires */
#define CONFIG_MACH_TYPE 3063
This seems a leftover of 9baa2bce.
$ git show 9baa2bce -- include/configs/omap3_cairo.h
commit 9baa2bce28901321d6f62399b5ebeb3fcb8e8a57
Author: Adam Ford <aford173@gmail.com>
Date: Tue Aug 7 07:08:32 2018 -0500
Removed unused references to CONFIG_SERIALx
After creating CONS_INDEX and migrating a bunch of boards to it,
there are a bunch of defined references to CONFIG_SERIALx which
are not referenced in any C code or #ifdef, so they can now be
removed
Signed-off-by: Adam Ford <aford173@gmail.com>
diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
index 0133416..04bce2f 100644
--- a/include/configs/omap3_cairo.h
+++ b/include/configs/omap3_cairo.h
@@ -198,8 +198,6 @@
* function per_clocks_enable().
*/
#ifdef CONFIG_SPL_BUILD
-#undef CONFIG_SERIAL3
-#define CONFIG_SERIAL2
#endif
/* Provide the MACH_TYPE value the vendor kernel requires */
However, I doubt it used the moveconfig tool.
I re-ran the tool against the previous commit.
$ git checkout 9baa2bce2890^
$ tools/moveconfig.py -H SERIAL0 SERIAL1 SERIAL2 SERIAL3
Clean up headers? [y/n]: y
y
...
$ git diff -- include/configs/omap3_cairo.h
diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
index 0133416..ac1a6cb 100644
--- a/include/configs/omap3_cairo.h
+++ b/include/configs/omap3_cairo.h
@@ -197,10 +197,6 @@
* are needed and peripheral clocks for UART2 must be enabled in
* function per_clocks_enable().
*/
-#ifdef CONFIG_SPL_BUILD
-#undef CONFIG_SERIAL3
-#define CONFIG_SERIAL2
-#endif
/* Provide the MACH_TYPE value the vendor kernel requires */
#define CONFIG_MACH_TYPE 3063
cleanup_one_header() did a good job.
Is there a particular case that the second path is needed?
--
Best Regards
Masahiro Yamada
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks
2019-01-31 6:25 ` Masahiro Yamada
@ 2019-01-31 9:05 ` Chris Packham
0 siblings, 0 replies; 5+ messages in thread
From: Chris Packham @ 2019-01-31 9:05 UTC (permalink / raw)
To: u-boot
On Thu, 31 Jan 2019 19:26 Masahiro Yamada <yamada.masahiro@socionext.com
wrote:
> On Wed, Jan 30, 2019 at 5:55 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > On Wed, Jan 30, 2019 at 4:23 PM Chris Packham <judge.packham@gmail.com>
> wrote:
> > >
> > > Moveconfig already attempts to remove empty #if/#endif blocks when
> there
> > > is a matching CONFIG_ being moved. Add a second pass which covers files
> > > without a match.
> > >
> > > Signed-off-by: Chris Packham <judge.packham@gmail.com>
> > > ---
> > > This was previously submitted as
> > > http://patchwork.ozlabs.org/patch/924901/ there still seems to be
> cases
> > > of #if/#endif left over from Kconfig migrations so perhaps this is
> still
> > > needed/wanted.
> > >
> > > I've plumbed this in as a second pass because ultimately we may want to
> > > make this a separate option. Also I couldn't figure out how to
> implement
> > > this without using re.M so I couldn't make it work in with the line by
> > > line parsing of cleanup_one_header().
> >
> >
> > This seems useful to find leftover code
> > regardless of the CONFIG options you are moving.
> >
> > One drawback is, it may fold unrelated cleanups.
> >
> > Maybe, it would be useful to add a separate option to turn on this
> feature,
> > or make it into a separate tool.
> >
> >
> >
>
>
> I tested this patch.
> It detected one empty block.
>
>
> diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
> index 04bce2f..ac1a6cb 100644
> --- a/include/configs/omap3_cairo.h
> +++ b/include/configs/omap3_cairo.h
> @@ -197,8 +197,6 @@
> * are needed and peripheral clocks for UART2 must be enabled in
> * function per_clocks_enable().
> */
> -#ifdef CONFIG_SPL_BUILD
> -#endif
>
> /* Provide the MACH_TYPE value the vendor kernel requires */
> #define CONFIG_MACH_TYPE 3063
>
>
>
>
>
>
>
> This seems a leftover of 9baa2bce.
>
Yes. I sent a patch for this as I was testing.
>
> $ git show 9baa2bce -- include/configs/omap3_cairo.h
> commit 9baa2bce28901321d6f62399b5ebeb3fcb8e8a57
> Author: Adam Ford <aford173@gmail.com>
> Date: Tue Aug 7 07:08:32 2018 -0500
>
> Removed unused references to CONFIG_SERIALx
>
> After creating CONS_INDEX and migrating a bunch of boards to it,
> there are a bunch of defined references to CONFIG_SERIALx which
> are not referenced in any C code or #ifdef, so they can now be
> removed
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
> index 0133416..04bce2f 100644
> --- a/include/configs/omap3_cairo.h
> +++ b/include/configs/omap3_cairo.h
> @@ -198,8 +198,6 @@
> * function per_clocks_enable().
> */
> #ifdef CONFIG_SPL_BUILD
> -#undef CONFIG_SERIAL3
> -#define CONFIG_SERIAL2
> #endif
>
> /* Provide the MACH_TYPE value the vendor kernel requires */
>
>
>
>
>
> However, I doubt it used the moveconfig tool.
>
>
> I re-ran the tool against the previous commit.
>
>
> $ git checkout 9baa2bce2890^
> $ tools/moveconfig.py -H SERIAL0 SERIAL1 SERIAL2 SERIAL3
> Clean up headers? [y/n]: y
> y
> ...
> $ git diff -- include/configs/omap3_cairo.h
> diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
> index 0133416..ac1a6cb 100644
> --- a/include/configs/omap3_cairo.h
> +++ b/include/configs/omap3_cairo.h
> @@ -197,10 +197,6 @@
> * are needed and peripheral clocks for UART2 must be enabled in
> * function per_clocks_enable().
> */
> -#ifdef CONFIG_SPL_BUILD
> -#undef CONFIG_SERIAL3
> -#define CONFIG_SERIAL2
> -#endif
>
> /* Provide the MACH_TYPE value the vendor kernel requires */
> #define CONFIG_MACH_TYPE 3063
>
>
>
>
>
> cleanup_one_header() did a good job.
>
> Is there a particular case that the second path is needed?
>
Has moveconfig been updated to handle such cases in another way? I only
re-sent this because I found it while cleaning up some old branches. I
believe at the time I originally wrote it there were many more instances of
empty guards (most if which I submitted patches for).
I agree that if moveconfig already handles this there is no point in adding
a second pass (baring cases of double nesting).
There still may be benefit in running something as a one-off pass over all
files to remove existing empty guards but there's no need to store such a
script in the repository.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [U-Boot, v3] moveconfig: add a second pass for empty #if/#endif blocks
2019-01-30 7:23 [U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks Chris Packham
2019-01-30 8:55 ` Masahiro Yamada
@ 2019-02-10 13:08 ` Tom Rini
1 sibling, 0 replies; 5+ messages in thread
From: Tom Rini @ 2019-02-10 13:08 UTC (permalink / raw)
To: u-boot
On Wed, Jan 30, 2019 at 08:23:16PM +1300, Chris Packham wrote:
> Moveconfig already attempts to remove empty #if/#endif blocks when there
> is a matching CONFIG_ being moved. Add a second pass which covers files
> without a match.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190210/d0b4d457/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-10 13:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30 7:23 [U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks Chris Packham
2019-01-30 8:55 ` Masahiro Yamada
2019-01-31 6:25 ` Masahiro Yamada
2019-01-31 9:05 ` Chris Packham
2019-02-10 13:08 ` [U-Boot] [U-Boot, " Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.