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.