[U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks
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.
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@lists.denx.de https://lists.denx.de/listinfo/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?
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.
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!
participants (3)
-
Chris Packham -
Masahiro Yamada -
Tom Rini