[1/1] utils/test-pkg: Generate package config if it is not specified
diff mbox series

Message ID 1551443622-29498-1-git-send-email-email@gmail.com
State Accepted
Headers show
Series
  • [1/1] utils/test-pkg: Generate package config if it is not specified
Related show

Commit Message

Vadim Kochan March 1, 2019, 12:33 p.m. UTC
From: Vadim Kochan <vadim4j@gmail.com>

It is possible to generate one-line config for the package just by
normalize it to the form:

    BR2_PACKAGE_${pkg_replaced-to_and_uppercase}

it simplifes a bit of testing package where no additional config options
are needed.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 utils/test-pkg | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Arnout Vandecappelle March 4, 2019, 7:34 a.m. UTC | #1
On 01/03/2019 13:33, Vadim Kochan wrote:
> From: Vadim Kochan <vadim4j@gmail.com>
> 
> It is possible to generate one-line config for the package just by
> normalize it to the form:
> 
>     BR2_PACKAGE_${pkg_replaced-to_and_uppercase}
> 
> it simplifes a bit of testing package where no additional config options
> are needed.
> 
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

> ---
>  utils/test-pkg | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/test-pkg b/utils/test-pkg
> index 1995fa8..cce4679 100755
> --- a/utils/test-pkg
> +++ b/utils/test-pkg
> @@ -2,12 +2,20 @@
>  set -e
>  
>  TOOLCHAINS_CSV='support/config-fragments/autobuild/toolchain-configs.csv'
> +TEMP_CONF=""
> +
> +do_clean() {
> +    if [ ! -z "${TEMP_CONF}" ]; then
> +        rm -f "${TEMP_CONF}"

 A race-free way of cleaning up! Excellent!

> +    fi
> +}
>  
>  main() {
>      local o O opts
>      local cfg dir pkg random toolchains_dir toolchain all number mode
>      local ret nb nb_skip nb_fail nb_legal nb_tc build_dir
>      local -a toolchains
> +    local pkg_br_name
>  
>      o='hac:d:n:p:r:t:'
>      O='help,config-snippet:build-dir:package:,random:,toolchains-dir:'
> @@ -50,8 +58,15 @@ main() {
>              ;;
>          esac
>      done
> +
> +    trap do_clean INT TERM HUP EXIT
> +
>      if [ -z "${cfg}" ]; then
> -        printf "error: no config snippet specified\n" >&2; exit 1
> +        pkg_br_name="${pkg//-/_}"

 The script is already using bash, so this is OK.

 However, the UPPERCASE macro also converts . to _. Still, since we don't have
any package with . in it, it should be fine.

 Regards,
 Arnout

> +        pkg_br_name="BR2_PACKAGE_${pkg_br_name^^}"
> +        TEMP_CONF=$(mktemp /tmp/test-${pkg}-config.XXXXXX)
> +        echo "${pkg_br_name}=y" > ${TEMP_CONF}
> +        cfg="${TEMP_CONF}"
>      fi
>      if [ ! -e "${cfg}" ]; then
>          printf "error: %s: no such file\n" "${cfg}" >&2; exit 1
>
Thomas Petazzoni March 17, 2019, 9:27 a.m. UTC | #2
Hello Vadim,

On Fri,  1 Mar 2019 14:33:42 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:

> From: Vadim Kochan <vadim4j@gmail.com>
> 
> It is possible to generate one-line config for the package just by
> normalize it to the form:
> 
>     BR2_PACKAGE_${pkg_replaced-to_and_uppercase}
> 
> it simplifes a bit of testing package where no additional config options
> are needed.
> 
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
>  utils/test-pkg | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)

Applied to master, thanks. I was initially a bit skeptical because if
the package has a dependency, your solution does not work very well,
and users could believe that the package is going to be tested while it
won't. But in fact in that case all tests are going to be "SKIPPED"
because the BR2_PACKAGE_FOO=y line is not in the final .config. This
will clearly tell the user that much not testing has happened. So I'm
fine with this, and therefore I applied.

Thanks!

Thomas
Peter Korsgaard March 27, 2019, 9:43 a.m. UTC | #3
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > Hello Vadim,
 > On Fri,  1 Mar 2019 14:33:42 +0200
 > Vadim Kochan <vadim4j@gmail.com> wrote:

 >> From: Vadim Kochan <vadim4j@gmail.com>
 >> 
 >> It is possible to generate one-line config for the package just by
 >> normalize it to the form:
 >> 
 >> BR2_PACKAGE_${pkg_replaced-to_and_uppercase}
 >> 
 >> it simplifes a bit of testing package where no additional config options
 >> are needed.
 >> 
 >> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
 >> ---
 >> utils/test-pkg | 17 ++++++++++++++++-
 >> 1 file changed, 16 insertions(+), 1 deletion(-)

 > Applied to master, thanks. I was initially a bit skeptical because if
 > the package has a dependency, your solution does not work very well,
 > and users could believe that the package is going to be tested while it
 > won't. But in fact in that case all tests are going to be "SKIPPED"
 > because the BR2_PACKAGE_FOO=y line is not in the final .config. This
 > will clearly tell the user that much not testing has happened. So I'm
 > fine with this, and therefore I applied.

While this isn't a bugfix, it seems quite safe and can be useful for
users - So I've committed it to 2019.02.x as well, thanks.

Patch
diff mbox series

diff --git a/utils/test-pkg b/utils/test-pkg
index 1995fa8..cce4679 100755
--- a/utils/test-pkg
+++ b/utils/test-pkg
@@ -2,12 +2,20 @@ 
 set -e
 
 TOOLCHAINS_CSV='support/config-fragments/autobuild/toolchain-configs.csv'
+TEMP_CONF=""
+
+do_clean() {
+    if [ ! -z "${TEMP_CONF}" ]; then
+        rm -f "${TEMP_CONF}"
+    fi
+}
 
 main() {
     local o O opts
     local cfg dir pkg random toolchains_dir toolchain all number mode
     local ret nb nb_skip nb_fail nb_legal nb_tc build_dir
     local -a toolchains
+    local pkg_br_name
 
     o='hac:d:n:p:r:t:'
     O='help,config-snippet:build-dir:package:,random:,toolchains-dir:'
@@ -50,8 +58,15 @@  main() {
             ;;
         esac
     done
+
+    trap do_clean INT TERM HUP EXIT
+
     if [ -z "${cfg}" ]; then
-        printf "error: no config snippet specified\n" >&2; exit 1
+        pkg_br_name="${pkg//-/_}"
+        pkg_br_name="BR2_PACKAGE_${pkg_br_name^^}"
+        TEMP_CONF=$(mktemp /tmp/test-${pkg}-config.XXXXXX)
+        echo "${pkg_br_name}=y" > ${TEMP_CONF}
+        cfg="${TEMP_CONF}"
     fi
     if [ ! -e "${cfg}" ]; then
         printf "error: %s: no such file\n" "${cfg}" >&2; exit 1