Re: [PATCH] Check configuration files

[Paul Bolle]

On Fri, 2016-08-19 at 13:24 -0700, Miguel Flores Silverio wrote:
> Duble check generated configuration files in order to avoid
> discrepancies like this one:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1347454

What does this actually mean? Discrepancies (and mismatches, which you
use in the patch itself) are both rather generic terms.

(The main issue with having CONFIG_PWM_LPSS set to 'm' in one of the
.config fragments is that it is actually unneeded. See, the kconfig
tools will set it when other kconfig options select it. So why bother
including it in the .config fragments?

Is that the issue this patch addresses?)
> Signed-off-by: Miguel Flores Silverio
> PATCHv1
>
> - Check config files at compilation time
>
> * PATCHv2
>    - Check config files before and after oldconfig is aplied
>    - Save log of mismatches in logs directory in tree
>
> * PATCHv3
>    - save mismatches in .mismatches and exit compilation if
>      there any.
>    - Display error message and config options in .mismatches
>    - Fixed bug in awk statment
>    - Only save mismatches of enabled config options
>    - Delete temporary files outside function to avoid confusion
>    
>  kernel.spec | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/kernel.spec b/kernel.spec
> index 8d3587d..431a17c 100644
> --- a/kernel.spec
> +++ b/kernel.spec
> @@ -1231,9 +1231,21 @@ rm -f kernel-%{version}-*debug.config
>  
>  %define make make %{?cross_opts}
>  
> +# Check configuration files for any discrepancies
> +CheckConfigs(){
> +    awk 'NR==FNR{configs[$0]++;next;}
> +    NR!=FNR && !($0 in configs)' $1 $2 | egrep '^CONFIG_' > .mismatches

This awk one-liner looks like line noise to me. The comment
above CheckConfigs() doesn't tell me anything that the name of this
function already does.
> +    if [ -s .mismatches ]
> +    then
> echo "Error: Mismatches found in configuration files"

Errors should go to stderr, please.
> cat .mismatches
> exit 1

This terminates the build, doesn't it? But we should only terminate the
build when we're unsure how to continue. Because otherwise we might
break people's build for configuration issues they personally couldn't
care less about. For instance, I'm running an x86 laptop with
CONFIG_PWM not set ever since I run Fedora 24. Why should I care? I
have no idea.
> +    fi
> +}
>  # now run oldconfig over all the config files
>  for i in *.config
>  do
> +  cat $i > temp-$i
>    mv $i .config
>    Arch=`head -1 .config | cut -b 3-`
>    make ARCH=$Arch listnewconfig | grep -E '^CONFIG_' >.newoptions ||
> true
> @@ -1247,6 +1259,8 @@ do
>    make ARCH=$Arch oldnoconfig
>    echo "# $Arch" > configs/$i
>    cat .config >> configs/$i
> +  CheckConfigs configs/$i temp-$i
> +  rm temp-$i
>  done
>  # end of kernel config
>  %endif

Thanks,

Paul Bolle