[PATCH] RFC: CI: add test/usage_of_is_enabled_check.sh

[Simon Glass]

Hi Troy,

On Tue, 31 Jan 2023 at 12:57, Troy Kisky <troykiskyboundary at gmail.com> wrote:
>
> Add script usage_of_is_enabled_check to print any configs that
> use CONFIG_IS_ENABLED instead of IS_ENABLED and vice versa.
>
> Add usage_of_is_enabled_commit.sh to generate commits to fix the above
> issues.
>
> Signed-off-by: Troy Kisky <troykiskyboundary at gmail.com>
> ---
>  .azure-pipelines.yml               | 11 ++++++++
>  .gitlab-ci.yml                     |  5 ++++
>  test/usage_of_is_enabled_check.sh  | 41 ++++++++++++++++++++++++++++++
>  test/usage_of_is_enabled_commit.sh | 38 +++++++++++++++++++++++++++
>  4 files changed, 95 insertions(+)
>  create mode 100755 test/usage_of_is_enabled_check.sh
>  create mode 100755 test/usage_of_is_enabled_commit.sh

This looks reasonable to me. I would of course prefer to just fix all
these problems, since they make it impossible to move to split config.

But we need this sort of check in CI to stop it happening again.

>
> diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
> index 5673bb76afb..8e227512765 100644
> --- a/.azure-pipelines.yml
> +++ b/.azure-pipelines.yml
> @@ -67,6 +67,17 @@ stages:
>                    :^doc/ :^arch/arm/dts/ :^scripts/kconfig/lkc.h
>                    :^include/linux/kconfig.h :^tools/ && exit 1 || exit 0
>
> +  - job: check_usage_of_is_enabled
> +    displayName: 'Check usage of CONFIG_IS_ENABLED vs IS_ENABLED'
> +    pool:
> +      vmImage: $(ubuntu_vm)
> +    container:
> +      image: $(ci_runner_image)
> +      options: $(container_option)
> +    steps:
> +      # generate list of SPL configs
> +      - script: test/usage_of_is_enabled_check.sh
> +
>    - job: cppcheck
>      displayName: 'Static code analysis with cppcheck'
>      pool:
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index aaf9d25abfc..6bb8efef258 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -134,6 +134,11 @@ check for new CONFIG symbols outside Kconfig:
>          :^doc/ :^arch/arm/dts/ :^scripts/kconfig/lkc.h
>          :^include/linux/kconfig.h :^tools/ && exit 1 || exit 0
>
> +check usage of CONFIG_IS_ENABLED vs IS_ENABLED:
> +  stage: testsuites
> +  script:
> +    - ./test/usage_of_is_enabled_check.sh
> +
>  # QA jobs for code analytics
>  # static code analysis with cppcheck (we can add --enable=all later)
>  cppcheck:
> diff --git a/test/usage_of_is_enabled_check.sh b/test/usage_of_is_enabled_check.sh
> new file mode 100755
> index 00000000000..0bc9bff8bd1
> --- /dev/null
> +++ b/test/usage_of_is_enabled_check.sh
> @@ -0,0 +1,41 @@
> +#!/bin/bash
> +# generate list of CONFIGs that should use CONFIG_IS_ENABLED

Could you add a few comments about the steps in here?

> +{ { git grep 'obj-$(CONFIG_$(SPL_'|grep Makefile|sed -e "s/SPL_TPL_/SPL_/"| \
> +sed -n -r 's/obj\-\$\(CONFIG_\$\(SPL_\)([0-9a-zA-Z_]+)\)/\n\{\1\}\n/gp'| \
> +sed -n -r 's/\{([0-9a-zA-Z_]+)\}/\1/p'; } ;\
> +{ git grep -E 'config [ST]PL_'|grep Kconfig| \
> +sed -n -r "s/config [ST]PL_([0-9a-zA-Z_]+)/\n\{\1\}\n/p" |
> +sed -n -r 's/\{([0-9a-zA-Z_]+)\}/\1/p'; } ; \
> +git grep -E 'CONFIG_IS_ENABLED\(CMD_'|sed -n -e "s/\(CONFIG_IS_ENABLED(CMD_[0-9a-zA-Z_]*)\)/\n\1\n/gp"| \
> +sed -n -r "s/CONFIG_IS_ENABLED\((CMD_[0-9a-zA-Z_]+)\)/\1/p"; \
> +echo -e "\
> +BZIP2\n\
> +CONFIG_CLK\n\

CLK ?

> +DM_EVENT\n\
> +EFI_DEVICE_PATH_TO_TEXT\n\
> +EFI_LOADER\n\
> +ERRNO_STR\n\
> +GENERATE_SMBIOS_TABLE\n\
> +";\

What is the above list for? Can you add a comment?

> +} | sort -u >splcfg.tmp
> +
> +{
> +# generate list of CONFIGs that incorrectly use CONFIG_IS_ENABLED
> +git grep CONFIG_IS_ENABLED|sed -n -e "s/\(CONFIG_IS_ENABLED([0-9a-zA-Z_]*)\)/\n\1\n/gp"| \
> +sed -n -r "s/CONFIG_IS_ENABLED\(([0-9a-zA-Z_]+)\)/\1/p" |sort -u| comm -23 - splcfg.tmp ;
> +
> +# generate list of CONFIGs that incorrectly use IS_ENABLED
> +git grep -w IS_ENABLED|sed -n -e "s/\(IS_ENABLED(CONFIG_[0-9a-zA-Z_]*)\)/\n\1\n/gp"| \
> +sed -n -r "s/IS_ENABLED\(CONFIG_([0-9a-zA-Z_]+)\)/\1/p" |sort -u| join - splcfg.tmp;
> +} | grep -vw FOO;
> +if [ $? -eq 0 ] ; then
> +       echo "The above may have incorrect usage of IS_ENABLED/CONFIG_IS_ENABLED"
> +       echo "Run test/usage_of_is_enabled_commit.sh and squash with appropriate commit"
> +       ret=1;
> +else
> +       ret=0;
> +fi
> +
> +rm splcfg.tmp
> +exit ${ret}
> +
> diff --git a/test/usage_of_is_enabled_commit.sh b/test/usage_of_is_enabled_commit.sh
> new file mode 100755
> index 00000000000..f776d0c69ab
> --- /dev/null
> +++ b/test/usage_of_is_enabled_commit.sh
> @@ -0,0 +1,38 @@
> +#!/bin/bash
> +# generate list of CONFIGs that should use CONFIG_IS_ENABLED

Same thing, could use some comments

> +{ { git grep 'obj-$(CONFIG_$(SPL_'|grep Makefile|sed -e "s/SPL_TPL_/SPL_/"| \
> +sed -n -r 's/obj\-\$\(CONFIG_\$\(SPL_\)([0-9a-zA-Z_]+)\)/\n\{\1\}\n/gp'| \
> +sed -n -r 's/\{([0-9a-zA-Z_]+)\}/\1/p'; } ;\
> +{ git grep -E 'config [ST]PL_'|grep Kconfig| \
> +sed -n -r "s/config [ST]PL_([0-9a-zA-Z_]+)/\n\{\1\}\n/p" |
> +sed -n -r 's/\{([0-9a-zA-Z_]+)\}/\1/p'; } ; \
> +git grep -E 'CONFIG_IS_ENABLED\(CMD_'|sed -n -e "s/\(CONFIG_IS_ENABLED(CMD_[0-9a-zA-Z_]*)\)/\n\1\n/gp"| \
> +sed -n -r "s/CONFIG_IS_ENABLED\((CMD_[0-9a-zA-Z_]+)\)/\1/p"; \
> +echo -e "\
> +BZIP2\n\
> +CONFIG_CLK\n\
> +DM_EVENT\n\
> +EFI_DEVICE_PATH_TO_TEXT\n\
> +EFI_LOADER\n\
> +ERRNO_STR\n\
> +GENERATE_SMBIOS_TABLE\n\
> +";\
> +} | sort -u >splcfg.tmp
> +
> +
> +git grep CONFIG_IS_ENABLED|sed -n -e "s/\(CONFIG_IS_ENABLED([0-9a-zA-Z_]*)\)/\n\1\n/gp"| \
> +sed -n -r "s/CONFIG_IS_ENABLED\(([0-9a-zA-Z_]+)\)/\1/p" |sort -u|grep -vw FOO| \
> +comm -23 - splcfg.tmp|xargs -I {} \
> +sh -c "git grep -l 'CONFIG_IS_ENABLED({})' | \
> +xargs -IFile sh -c \"sed -i -e \\\"s/CONFIG_IS_ENABLED({})/IS_ENABLED(CONFIG_{})/g\\\" File\" ; \
> +git commit -a -m\"CONFIG_{}: change CONFIG_IS_ENABLED to IS_ENABLED\";"
> +
> +git grep -w IS_ENABLED|sed -n -e "s/\(IS_ENABLED(CONFIG_[0-9a-zA-Z_]*)\)/\n\1\n/gp"| \
> +sed -n -r "s/IS_ENABLED\(CONFIG_([0-9a-zA-Z_]+)\)/\1/p" |sort -u|grep -vw FOO| \
> +join - splcfg.tmp |xargs -I {} \
> +sh -c "git grep -l 'IS_ENABLED(CONFIG_{})' | \
> +xargs -IFile sh -c \"sed -i -e \\\"s/\([^_]\)IS_ENABLED(CONFIG_{})/\1CONFIG_IS_ENABLED({})/g\\\" File\" ; \
> +git commit -a -m\"CONFIG_{}: change IS_ENABLED to CONFIG_IS_ENABLED\";"
> +
> +rm splcfg.tmp
> +
> --
> 2.34.1
>

Regards,
SImon